On 03/22/2015 10:10 AM, Kevin O'Connor wrote:
On Sun, Mar 22, 2015 at 09:55:47AM -0400, Stefan Berger wrote:
On 03/20/2015 09:15 PM, Kevin O'Connor wrote:
On Fri, Mar 20, 2015 at 02:00:37PM -0400, Stefan Berger wrote:
+u32 +tpm_s3_resume(void) +{
- u32 rc;
- u32 returnCode;
- if (!CONFIG_TCGBIOS)
return 0;
- if (!has_working_tpm())
return TCG_GENERAL_ERROR;
- timer_setup();
Why is the timer reinitialized here? There may be implications in touching that hardware during an S3 resume.
I am initializing the time because of the TPM driver's usage of msleep(). Your comment reminds me of a comment years ago from Keir Fraser from Xen. There, we ended up using I think a DRAM refresh timer bit that was emulated by QEMU and flipped every few ns.
I think msleep() should work in s3 even without the call to timer_setup(). The settings found during the first boot should still be applicable during s3 resume. Does it not work if the timer_setup() isn't invoked?
At least with Linux as OS, the timer_setup() call can be removed.
--- /dev/null +++ b/src/tcgbios.h @@ -0,0 +1,388 @@ +#ifndef TCGBIOS_H +#define TCGBIOS_H
+#include "types.h" +#include "bregs.h" /* struct bregs */
Why is "bregs.h" needed in the header - can it be included in the individual C files that need it?
This #include needs to only appear in the 3rd patch when the interrupt handler prototype appears. I move it there now.
If it's just for a function prototype the prefered approach is to forward declare the struct (ie, add the line "struct bregs;" to the header).
Ok, I will change it to that.
-Kevin