Re: [SeaBIOS] [PATCH v10 3/6] Support for BIOS interrupt handler

[Stefan Berger]

On 03/27/2015 03:58 AM, Xu, Quan wrote:
> > -----Original Message-----
> > From: Xu, Quan
> > Sent: Friday, March 27, 2015 10:01 AM
> > To: 'Stefan Berger'; Kevin O'Connor
> > Cc: [e-mail]; [e-mail]
> > Subject: RE: [PATCH v10 3/6] Support for BIOS interrupt handler
> > > -----Original Message-----
> > > From: Stefan Berger [mailto:[e-mail]]
> > > Sent: Thursday, March 26, 2015 7:04 PM
> > > To: Xu, Quan; Kevin O'Connor
> > > Cc: [e-mail]; [e-mail]
> > > Subject: Re: [PATCH v10 3/6] Support for BIOS interrupt handler
> > >
> > > On 03/26/2015 07:01 AM, Xu, Quan wrote:
> > > > > -----Original Message-----
> > > > > From: Stefan Berger [mailto:[e-mail]]
> > > > > Sent: Thursday, March 26, 2015 6:18 PM
> > > > > To: Kevin O'Connor; Xu, Quan
> > > > > Cc: [e-mail]; [e-mail]
> > > > > Subject: Re: [PATCH v10 3/6] Support for BIOS interrupt handler
> > > > >
> > > > > On 03/25/2015 06:42 PM, Kevin O'Connor wrote:
> > > > > > On Tue, Mar 24, 2015 at 11:10:03AM -0400, Stefan Berger wrote:
> > > > > > > On 03/23/2015 08:13 PM, Kevin O'Connor wrote:
> > > > > > >> Because of the mixed 16bit/32bit code in SeaBIOS, all assembler
> > > > > > >> must use size suffixes - so the above should be "roll" instead of "rol".
> > > > > > > Ok, fixed.
> > > > > > >
> > > > > > >> As before - both issues are minor and can be addressed after
> > > > > > >> merge (as long as there is agreement that the sha1.c file can be
> > > > > > >> licensed as LGPLv3).
> > > > > > > It can have that license. I can post v11 or you can modify it,
> > > > > > > either way is fine.
> > > > > > Thanks.  I pushed the first three patches into a test branch at:
> > > > > >
> > > > > > https://github.com/KevinOConnor/seabios/tree/tcg-testing
> > > > > >
> > > > > > I'd like to get confirmation that this works for the Xen
> > > > > > requirements before merging.
> > > > > > I don't use Xen. I hope that Quan will provide feedback.
> > > > >
> > > > > Stefan
> > > > > Sure, I am glad to help you test it :):) Try to
> > > > > https://github.com/KevinOConnor/seabios/tree/tcg-testing ??
> > > > > Yes.
> > >
> > > Stefan
> > > Just for check,
> > > I can NOT git clone https://github.com/KevinOConnor/seabios/tree/tcg-testing
> > > I can clone https://github.com/KevinOConnor/seabios and checkout * tcg-testing
> > > branch.
> > > Correct?
> >
> > Quan
> > Share the test result first. It is not working with Xen vTPM.
> > The log from vtpm mini-os:
>
> [.. ]
> tpm_testing.c:229: Debug: verify plain text
> tpm_testing.c:261: Info: Self-Test succeeded
> tpm_startup.c:43: Info: TPM_Startup(1)
> Tpmback:Info Frontend 0/0 connected
> tpm_cmd_handler.c:4217: Debug: tpm_handle_command(0)
> tpm_cmd_handler.c:3514: Debug: [TPM_TAG_RQU_COMMAND]
> tpm_cmd_handler.c:3537: Debug: [TPM_ORD_Startup]
> tpm_startup.c:43: Info: TPM_Startup(1)
> tpm_cmd_handler.c:4151: Info: TPM command failed: (0x26) The command was received in the wrong sequence
> relative to TPM_Init and a subsequent TPM_Startup.
> [..]
>
> In my opinion, we need this patch for Xen vTPM too.

Which patch is 'this patch'?
Does the above indicate that TPM_Startup(1) is sent twice to the Xen 
vTPM? Besides SeaBIOS, what else is sending a TPM_Startup()? If there is 
something else in Xen that sends a TPM_Startup() to the TPM, before 
SeaBIOS does, then the following code may apply as well:


This part is from src/tcgbios.c.

+    rc = build_and_send_cmd(TPM_ORD_Startup,
+                            Startup_ST_CLEAR, sizeof(Startup_ST_CLEAR),
+                            NULL, 10, &returnCode, 
TPM_DURATION_TYPE_SHORT);
+
+    dprintf(DEBUG_tcg, "Return code from TPM_Startup = 0x%08x\n",
+            returnCode);
+
+    if (CONFIG_COREBOOT) {

Here you could add a check whether you are running on Xen and ignore the 
error returned from TPM_Startup().

+        /* with other firmware on the system the TPM may already have been
+         * initialized
+         */
+        if (returnCode == TPM_INVALID_POSTINIT)
+            returnCode = 0;
+    }
+
+    if (rc || returnCode)
+        goto err_exit;



Regards,
      Stefan
> now Xen vTPM is working with two basic functions: TPM TCPA / SSDT and registers reset
> in my previous seabios patch.
>
> Quan
> >

[PATCH v10 3/6] Support for BIOS interrupt handler

[Xu, Quan]

> -----Original Message-----
> From: Stefan Berger [mailto:[e-mail]]
> Sent: Friday, March 27, 2015 9:45 PM
> To: Xu, Quan; Kevin O'Connor
> Cc: [e-mail]; [e-mail];
> [e-mail]; Daniel De Graaf; [e-mail]
> Subject: Re: [PATCH v10 3/6] Support for BIOS interrupt handler
>
> On 03/27/2015 03:58 AM, Xu, Quan wrote:
> > > -----Original Message-----
> > > From: Xu, Quan
> > > Sent: Friday, March 27, 2015 10:01 AM
> > > To: 'Stefan Berger'; Kevin O'Connor
> > > Cc: [e-mail]; [e-mail]
> > > Subject: RE: [PATCH v10 3/6] Support for BIOS interrupt handler
> > > > -----Original Message-----
> > > > From: Stefan Berger [mailto:[e-mail]]
> > > > Sent: Thursday, March 26, 2015 7:04 PM
> > > > To: Xu, Quan; Kevin O'Connor
> > > > Cc: [e-mail]; [e-mail]
> > > > Subject: Re: [PATCH v10 3/6] Support for BIOS interrupt handler
> > > >
> > > > On 03/26/2015 07:01 AM, Xu, Quan wrote:
> > > > > > -----Original Message-----
> > > > > > From: Stefan Berger [mailto:[e-mail]]
> > > > > > Sent: Thursday, March 26, 2015 6:18 PM
> > > > > > To: Kevin O'Connor; Xu, Quan
> > > > > > Cc: [e-mail]; [e-mail]
> > > > > > Subject: Re: [PATCH v10 3/6] Support for BIOS interrupt handler
> > > > > >
> > > > > > On 03/25/2015 06:42 PM, Kevin O'Connor wrote:
> > > > > > > On Tue, Mar 24, 2015 at 11:10:03AM -0400, Stefan Berger wrote:
> > > > > > >> On 03/23/2015 08:13 PM, Kevin O'Connor wrote:
> > > > > > >>> Because of the mixed 16bit/32bit code in SeaBIOS, all assembler
> > > > > > >>> must use size suffixes - so the above should be "roll" instead of "rol".
> > > > > > >> Ok, fixed.
> > > > > > >>
> > > > > > >>> As before - both issues are minor and can be addressed after
> > > > > > >>> merge (as long as there is agreement that the sha1.c file can
> > > > > > >>> be licensed as LGPLv3).
> > > > > > >> It can have that license. I can post v11 or you can modify it,
> > > > > > >> either way is fine.
> > > > > > > Thanks.  I pushed the first three patches into a test branch at:
> > > > > > >
> > > > > > >      https://github.com/KevinOConnor/seabios/tree/tcg-testing
> > > > > > >
> > > > > > > I'd like to get confirmation that this works for the Xen
> > > > > > > requirements before merging.
> > > > > > I don't use Xen. I hope that Quan will provide feedback.
> > > > > >
> > > > > > Stefan
> > > > > > Sure, I am glad to help you test it :):) Try to
> > > > > > https://github.com/KevinOConnor/seabios/tree/tcg-testing ??
> > > > > > Yes.
> > > >
> > > > Stefan
> > > > Just for check,
> > > > I can NOT git clone
> > > > https://github.com/KevinOConnor/seabios/tree/tcg-testing
> > > > I can clone https://github.com/KevinOConnor/seabios and checkout *
> > > > tcg-testing branch.
> > > > Correct?
> > >
> > > Quan
> > > Share the test result first. It is not working with Xen vTPM.
> > > The log from vtpm mini-os:
> >
> > [.. ]
> > tpm_testing.c:229: Debug: verify plain text
> > tpm_testing.c:261: Info: Self-Test succeeded
> > tpm_startup.c:43: Info: TPM_Startup(1) Tpmback:Info Frontend 0/0
> > connected
> > tpm_cmd_handler.c:4217: Debug: tpm_handle_command(0)
> > tpm_cmd_handler.c:3514: Debug: [TPM_TAG_RQU_COMMAND]
> > tpm_cmd_handler.c:3537: Debug: [TPM_ORD_Startup]
> > tpm_startup.c:43: Info: TPM_Startup(1)
> > tpm_cmd_handler.c:4151: Info: TPM command failed: (0x26) The command
> > was received in the wrong sequence relative to TPM_Init and a subsequent
>
> TPM_Startup.
> > [..]
> >
> > In my opinion, we need this patch for Xen vTPM too.
>
> Which patch is 'this patch'?

This patch is '[PATCH v10 3/6] Support for BIOS interrupt handler'.
Is interrupt for tGrub? Correct me if I am wrong..
> Does the above indicate that TPM_Startup(1) is sent twice to the Xen vTPM?
> Besides SeaBIOS, what else is sending a TPM_Startup()? If there is something
> else in Xen that sends a TPM_Startup() to the TPM, before SeaBIOS does, then
> the following code may apply as well:
>
> This part is from src/tcgbios.c.
>
> +    rc = build_and_send_cmd(TPM_ORD_Startup,
> +                            Startup_ST_CLEAR, sizeof(Startup_ST_CLEAR),
> +                            NULL, 10, &returnCode,
> TPM_DURATION_TYPE_SHORT);
> +
> +    dprintf(DEBUG_tcg, "Return code from TPM_Startup = 0x%08x\n",
> +            returnCode);
> +
> +    if (CONFIG_COREBOOT) {
>
> Here you could add a check whether you are running on Xen and ignore the error
> returned from TPM_Startup().
>
> +        /* with other firmware on the system the TPM may already have been
> +         * initialized
> +         */
> +        if (returnCode == TPM_INVALID_POSTINIT)
> +            returnCode = 0;
> +    }
> +
> +    if (rc || returnCode)
> +        goto err_exit;
>
> Regards,
>       Stefan

I will go through all of these seabios patch, and try to make it compatible for Xen vTPM.



-Quan
> > now Xen vTPM is working with two basic functions: TPM TCPA / SSDT and
> > registers reset in my previous seabios patch.
> >
> > Quan
> > >