diff mbox series

[1/5] efi_loader: increase eventlog buffer size

Message ID 20210707133638.12630-2-masahisa.kojima@linaro.org
State Superseded
Headers show
Series add measurement support | expand

Commit Message

Masahisa Kojima July 7, 2021, 1:36 p.m. UTC
This is a preperation to add eventlog support
described in TCG PC Client PFP spec.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

---
 lib/efi_loader/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.17.1

Comments

Heinrich Schuchardt July 7, 2021, 1:47 p.m. UTC | #1
On 7/7/21 3:36 PM, Masahisa Kojima wrote:
> This is a preperation to add eventlog support

> described in TCG PC Client PFP spec.

>

> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> ---

>   lib/efi_loader/Kconfig | 2 +-

>   1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> index b2ab48a048..a87bf3cc98 100644

> --- a/lib/efi_loader/Kconfig

> +++ b/lib/efi_loader/Kconfig

> @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

>   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

>   	int "EFI_TCG2_PROTOCOL EventLog size"

>   	depends on EFI_TCG2_PROTOCOL

> -	default 4096

> +	default 16384


I found this text in EDK II:

Minimum length(in bytes) of the system preboot TCG event log area(LAML)
-----------------------------------------------------------------------

For PC Client Implementation spec up to and including 1.2 the minimum
log size is 64KB. (SecurityPkg/SecurityPkg.dec)

Why should ours be smaller?

Best regards

Heinrich

>   	help

>   		Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that

>   		this is going to be allocated twice. One for the eventlog it self

>
Masahisa Kojima July 8, 2021, 2:21 a.m. UTC | #2
On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>

>

>

> On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > This is a preperation to add eventlog support

> > described in TCG PC Client PFP spec.

> >

> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > ---

> >   lib/efi_loader/Kconfig | 2 +-

> >   1 file changed, 1 insertion(+), 1 deletion(-)

> >

> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > index b2ab48a048..a87bf3cc98 100644

> > --- a/lib/efi_loader/Kconfig

> > +++ b/lib/efi_loader/Kconfig

> > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> >       int "EFI_TCG2_PROTOCOL EventLog size"

> >       depends on EFI_TCG2_PROTOCOL

> > -     default 4096

> > +     default 16384

>

> I found this text in EDK II:

>

> Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> -----------------------------------------------------------------------

>

> For PC Client Implementation spec up to and including 1.2 the minimum

> log size is 64KB. (SecurityPkg/SecurityPkg.dec)


Thank you for your feedback.
I have not checked this.
TCG spec also says "The Log Area Minimum Length for the TCG event log
MUST be at least 64KB." in ACPI chapter.
I will update to set 64KB as default.

Thanks,
Masahisa Kojima

>

> Why should ours be smaller?

>

> Best regards

>

> Heinrich

>

> >       help

> >               Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that

> >               this is going to be allocated twice. One for the eventlog it self

> >
Simon Glass July 11, 2021, 12:01 a.m. UTC | #3
Hi Masahisa,

On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>

> On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> >

> >

> >

> > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > This is a preperation to add eventlog support

> > > described in TCG PC Client PFP spec.

> > >

> > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > ---

> > >   lib/efi_loader/Kconfig | 2 +-

> > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > >

> > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > index b2ab48a048..a87bf3cc98 100644

> > > --- a/lib/efi_loader/Kconfig

> > > +++ b/lib/efi_loader/Kconfig

> > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > >       depends on EFI_TCG2_PROTOCOL

> > > -     default 4096

> > > +     default 16384

> >

> > I found this text in EDK II:

> >

> > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > -----------------------------------------------------------------------

> >

> > For PC Client Implementation spec up to and including 1.2 the minimum

> > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

>

> Thank you for your feedback.

> I have not checked this.

> TCG spec also says "The Log Area Minimum Length for the TCG event log

> MUST be at least 64KB." in ACPI chapter.

> I will update to set 64KB as default.

>


Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we
put this in the bloblist? We want to avoid adding code in EFI which is
in U-Boot.


- Simon

> Thanks,

> Masahisa Kojima

>

> >

> > Why should ours be smaller?

> >

> > Best regards

> >

> > Heinrich

> >

> > >       help

> > >               Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that

> > >               this is going to be allocated twice. One for the eventlog it self

> > >
Masahisa Kojima July 12, 2021, 8:40 a.m. UTC | #4
Hi Simon,

On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:
>

> Hi Masahisa,

>

> On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> >

> > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > >

> > >

> > >

> > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > This is a preperation to add eventlog support

> > > > described in TCG PC Client PFP spec.

> > > >

> > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > ---

> > > >   lib/efi_loader/Kconfig | 2 +-

> > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > >

> > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > index b2ab48a048..a87bf3cc98 100644

> > > > --- a/lib/efi_loader/Kconfig

> > > > +++ b/lib/efi_loader/Kconfig

> > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > >       depends on EFI_TCG2_PROTOCOL

> > > > -     default 4096

> > > > +     default 16384

> > >

> > > I found this text in EDK II:

> > >

> > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > -----------------------------------------------------------------------

> > >

> > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> >

> > Thank you for your feedback.

> > I have not checked this.

> > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > MUST be at least 64KB." in ACPI chapter.

> > I will update to set 64KB as default.

> >

>

> Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> put this in the bloblist? We want to avoid adding code in EFI which is

> in U-Boot.


I think bloblist is used for data passing from SPL/TPL to u-boot.
Is your comment saying that the eventlog generated
in u-boot(done in efi_tcg2.c with this patch series) should be appended
into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

Thanks,
Masahisa Kojima

>

>

> - Simon

>

> > Thanks,

> > Masahisa Kojima

> >

> > >

> > > Why should ours be smaller?

> > >

> > > Best regards

> > >

> > > Heinrich

> > >

> > > >       help

> > > >               Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that

> > > >               this is going to be allocated twice. One for the eventlog it self

> > > >
Ilias Apalodimas July 12, 2021, 9:27 a.m. UTC | #5
On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>

> Hi Simon,

>

> On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> >

> > Hi Masahisa,

> >

> > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> > >

> > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > > >

> > > >

> > > >

> > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > > This is a preperation to add eventlog support

> > > > > described in TCG PC Client PFP spec.

> > > > >

> > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > > ---

> > > > >   lib/efi_loader/Kconfig | 2 +-

> > > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > > >

> > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > > index b2ab48a048..a87bf3cc98 100644

> > > > > --- a/lib/efi_loader/Kconfig

> > > > > +++ b/lib/efi_loader/Kconfig

> > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > > >       depends on EFI_TCG2_PROTOCOL

> > > > > -     default 4096

> > > > > +     default 16384

> > > >

> > > > I found this text in EDK II:

> > > >

> > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > > -----------------------------------------------------------------------

> > > >

> > > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> > >

> > > Thank you for your feedback.

> > > I have not checked this.

> > > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > > MUST be at least 64KB." in ACPI chapter.

> > > I will update to set 64KB as default.

> > >

> >

> > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> > put this in the bloblist? We want to avoid adding code in EFI which is

> > in U-Boot.

>

> I think bloblist is used for data passing from SPL/TPL to u-boot.

> Is your comment saying that the eventlog generated

> in u-boot(done in efi_tcg2.c with this patch series) should be appended

> into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

>


Even in that case the eventlog can't be appended.  The TCG eventlog
hould be copied into EFI memory, since the kernel expects to find it
there.
What we could do is copy the contents of that buffer to the eventlog.
Depending on what that buffer already has (e.g the starting header of
the eventlog), we might need to strip it from the efi_tcg.c code.

Thanks
/Ilias
> Thanks,

> Masahisa Kojima

>

> >

> >

> > - Simon

> >

> > > Thanks,

> > > Masahisa Kojima

> > >

> > > >

> > > > Why should ours be smaller?

> > > >

> > > > Best regards

> > > >

> > > > Heinrich

> > > >

> > > > >       help

> > > > >               Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that

> > > > >               this is going to be allocated twice. One for the eventlog it self

> > > > >
Simon Glass July 14, 2021, 2:50 p.m. UTC | #6
Hi Masahisa,

On Mon, 12 Jul 2021 at 02:40, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>

> Hi Simon,

>

> On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> >

> > Hi Masahisa,

> >

> > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> > >

> > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > > >

> > > >

> > > >

> > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > > This is a preperation to add eventlog support

> > > > > described in TCG PC Client PFP spec.

> > > > >

> > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > > ---

> > > > >   lib/efi_loader/Kconfig | 2 +-

> > > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > > >

> > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > > index b2ab48a048..a87bf3cc98 100644

> > > > > --- a/lib/efi_loader/Kconfig

> > > > > +++ b/lib/efi_loader/Kconfig

> > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > > >       depends on EFI_TCG2_PROTOCOL

> > > > > -     default 4096

> > > > > +     default 16384

> > > >

> > > > I found this text in EDK II:

> > > >

> > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > > -----------------------------------------------------------------------

> > > >

> > > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> > >

> > > Thank you for your feedback.

> > > I have not checked this.

> > > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > > MUST be at least 64KB." in ACPI chapter.

> > > I will update to set 64KB as default.

> > >

> >

> > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> > put this in the bloblist? We want to avoid adding code in EFI which is

> > in U-Boot.

>

> I think bloblist is used for data passing from SPL/TPL to u-boot.


It can also be used to place things in memory that end up accessed by
Linux, e.g. ACPI tables. So I think it fits.

> Is your comment saying that the eventlog generated

> in u-boot(done in efi_tcg2.c with this patch series) should be appended

> into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?


I suppose so, but this logic should be implemented in the TPM layer I
think, not just in EFI. Otherwise we end up with another parallel
implementation.

Regards,
Simon
Simon Glass July 14, 2021, 2:52 p.m. UTC | #7
Hi Ilias,

On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>

> On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima

> <masahisa.kojima@linaro.org> wrote:

> >

> > Hi Simon,

> >

> > On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> > >

> > > Hi Masahisa,

> > >

> > > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> > > >

> > > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > > > >

> > > > >

> > > > >

> > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > > > This is a preperation to add eventlog support

> > > > > > described in TCG PC Client PFP spec.

> > > > > >

> > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > > > ---

> > > > > >   lib/efi_loader/Kconfig | 2 +-

> > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > > > >

> > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > > > index b2ab48a048..a87bf3cc98 100644

> > > > > > --- a/lib/efi_loader/Kconfig

> > > > > > +++ b/lib/efi_loader/Kconfig

> > > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > > > >       depends on EFI_TCG2_PROTOCOL

> > > > > > -     default 4096

> > > > > > +     default 16384

> > > > >

> > > > > I found this text in EDK II:

> > > > >

> > > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > > > -----------------------------------------------------------------------

> > > > >

> > > > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> > > >

> > > > Thank you for your feedback.

> > > > I have not checked this.

> > > > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > > > MUST be at least 64KB." in ACPI chapter.

> > > > I will update to set 64KB as default.

> > > >

> > >

> > > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> > > put this in the bloblist? We want to avoid adding code in EFI which is

> > > in U-Boot.

> >

> > I think bloblist is used for data passing from SPL/TPL to u-boot.

> > Is your comment saying that the eventlog generated

> > in u-boot(done in efi_tcg2.c with this patch series) should be appended

> > into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

> >

>

> Even in that case the eventlog can't be appended.  The TCG eventlog

> hould be copied into EFI memory, since the kernel expects to find it

> there.


Typically bloblist is relocated by U-Boot. There are lots of tables
that must be passed to linux, including ACPI and SMBIOS. With bloblist
they can all be in one place.

> What we could do is copy the contents of that buffer to the eventlog.

> Depending on what that buffer already has (e.g the starting header of

> the eventlog), we might need to strip it from the efi_tcg.c code.


I'm not really sure, but the eventlog is not just EFI thing, right?
The code should be generic.

Regards,
Simon
Masahisa Kojima July 15, 2021, 5:09 a.m. UTC | #8
Hi Simon, Ilias,

On Wed, 14 Jul 2021 at 23:50, Simon Glass <sjg@chromium.org> wrote:
>

> Hi Masahisa,

>

> On Mon, 12 Jul 2021 at 02:40, Masahisa Kojima

> <masahisa.kojima@linaro.org> wrote:

> >

> > Hi Simon,

> >

> > On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> > >

> > > Hi Masahisa,

> > >

> > > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> > > >

> > > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > > > >

> > > > >

> > > > >

> > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > > > This is a preperation to add eventlog support

> > > > > > described in TCG PC Client PFP spec.

> > > > > >

> > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > > > ---

> > > > > >   lib/efi_loader/Kconfig | 2 +-

> > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > > > >

> > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > > > index b2ab48a048..a87bf3cc98 100644

> > > > > > --- a/lib/efi_loader/Kconfig

> > > > > > +++ b/lib/efi_loader/Kconfig

> > > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > > > >       depends on EFI_TCG2_PROTOCOL

> > > > > > -     default 4096

> > > > > > +     default 16384

> > > > >

> > > > > I found this text in EDK II:

> > > > >

> > > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > > > -----------------------------------------------------------------------

> > > > >

> > > > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> > > >

> > > > Thank you for your feedback.

> > > > I have not checked this.

> > > > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > > > MUST be at least 64KB." in ACPI chapter.

> > > > I will update to set 64KB as default.

> > > >

> > >

> > > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> > > put this in the bloblist? We want to avoid adding code in EFI which is

> > > in U-Boot.

> >

> > I think bloblist is used for data passing from SPL/TPL to u-boot.

>

> It can also be used to place things in memory that end up accessed by

> Linux, e.g. ACPI tables. So I think it fits.


I understand bloblist can be used for eventlog, I will check further.

>

> > Is your comment saying that the eventlog generated

> > in u-boot(done in efi_tcg2.c with this patch series) should be appended

> > into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

>

> I suppose so, but this logic should be implemented in the TPM layer I

> think, not just in EFI. Otherwise we end up with another parallel

> implementation.


Current u-boot/lib/efi_loader/efi_tcg2.c includes the code
not directly related to EFI.
I would like to suggest to divide u-boot/lib/efi_loader/efi_tcg2.c
into two files.

 1)  u-boot/lib/efi_loader/efi_tcg2.c
  This file implements the EFI interfaces required in TCG EFI Protocol
Specification(https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/).

 2) u-boot/lib/tcg2.c(new file)
 This file implements the functionality required in TCG PC Client
Platform Firmware Profile
Specification(https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/).
 This file contains the common functions to extend eventlog and PCRs, etc.

In addition, this is different topic, I found some tpm related files
under u-boot/lib directory, I think it better to have new directory
like "tcg" and move tpm related files such as tpm_api.c, tpm-v2.c
and tpm-common.c  into lib/tcg(new directory).

Thanks,
Masahisa Kojima

>

> Regards,

> Simon
Ilias Apalodimas July 15, 2021, 6:20 a.m. UTC | #9
On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:
> Hi Ilias,

> 

> On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas

> <ilias.apalodimas@linaro.org> wrote:

> >

> > On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima

> > <masahisa.kojima@linaro.org> wrote:

> > >

> > > Hi Simon,

> > >

> > > On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> > > >

> > > > Hi Masahisa,

> > > >

> > > > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> > > > >

> > > > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > > > > >

> > > > > >

> > > > > >

> > > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > > > > This is a preperation to add eventlog support

> > > > > > > described in TCG PC Client PFP spec.

> > > > > > >

> > > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > > > > ---

> > > > > > >   lib/efi_loader/Kconfig | 2 +-

> > > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > > > > >

> > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > > > > index b2ab48a048..a87bf3cc98 100644

> > > > > > > --- a/lib/efi_loader/Kconfig

> > > > > > > +++ b/lib/efi_loader/Kconfig

> > > > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > > > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > > > > >       depends on EFI_TCG2_PROTOCOL

> > > > > > > -     default 4096

> > > > > > > +     default 16384

> > > > > >

> > > > > > I found this text in EDK II:

> > > > > >

> > > > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > > > > -----------------------------------------------------------------------

> > > > > >

> > > > > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> > > > >

> > > > > Thank you for your feedback.

> > > > > I have not checked this.

> > > > > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > > > > MUST be at least 64KB." in ACPI chapter.

> > > > > I will update to set 64KB as default.

> > > > >

> > > >

> > > > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> > > > put this in the bloblist? We want to avoid adding code in EFI which is

> > > > in U-Boot.

> > >

> > > I think bloblist is used for data passing from SPL/TPL to u-boot.

> > > Is your comment saying that the eventlog generated

> > > in u-boot(done in efi_tcg2.c with this patch series) should be appended

> > > into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

> > >

> >

> > Even in that case the eventlog can't be appended.  The TCG eventlog

> > hould be copied into EFI memory, since the kernel expects to find it

> > there.

> 

> Typically bloblist is relocated by U-Boot. There are lots of tables

> that must be passed to linux, including ACPI and SMBIOS. With bloblist

> they can all be in one place.



The eventlog must be allocated in EFI memory though. 

> 

> > What we could do is copy the contents of that buffer to the eventlog.

> > Depending on what that buffer already has (e.g the starting header of

> > the eventlog), we might need to strip it from the efi_tcg.c code.

> 

> I'm not really sure, but the eventlog is not just EFI thing, right?

> The code should be generic.


It's purely an EFI construct.  Specifically the entire spec, and even the  log
format for the eventlog are described in 
https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf


cheers
/Ilias
> 

> Regards,

> Simon
Ilias Apalodimas July 15, 2021, 6:46 a.m. UTC | #10
On Thu, Jul 15, 2021 at 02:09:57PM +0900, Masahisa Kojima wrote:
> Hi Simon, Ilias,

> 

> On Wed, 14 Jul 2021 at 23:50, Simon Glass <sjg@chromium.org> wrote:

> >

> > Hi Masahisa,

> >

> > On Mon, 12 Jul 2021 at 02:40, Masahisa Kojima

> > <masahisa.kojima@linaro.org> wrote:

> > >

> > > Hi Simon,

> > >

> > > On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> > > >

> > > > Hi Masahisa,

> > > >

> > > > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> > > > >

> > > > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > > > > >

> > > > > >

> > > > > >

> > > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > > > > This is a preperation to add eventlog support

> > > > > > > described in TCG PC Client PFP spec.

> > > > > > >

> > > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > > > > ---

> > > > > > >   lib/efi_loader/Kconfig | 2 +-

> > > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > > > > >

> > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > > > > index b2ab48a048..a87bf3cc98 100644

> > > > > > > --- a/lib/efi_loader/Kconfig

> > > > > > > +++ b/lib/efi_loader/Kconfig

> > > > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > > > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > > > > >       depends on EFI_TCG2_PROTOCOL

> > > > > > > -     default 4096

> > > > > > > +     default 16384

> > > > > >

> > > > > > I found this text in EDK II:

> > > > > >

> > > > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > > > > -----------------------------------------------------------------------

> > > > > >

> > > > > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> > > > >

> > > > > Thank you for your feedback.

> > > > > I have not checked this.

> > > > > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > > > > MUST be at least 64KB." in ACPI chapter.

> > > > > I will update to set 64KB as default.

> > > > >

> > > >

> > > > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> > > > put this in the bloblist? We want to avoid adding code in EFI which is

> > > > in U-Boot.

> > >

> > > I think bloblist is used for data passing from SPL/TPL to u-boot.

> >

> > It can also be used to place things in memory that end up accessed by

> > Linux, e.g. ACPI tables. So I think it fits.

> 

> I understand bloblist can be used for eventlog, I will check further.

> 

> >

> > > Is your comment saying that the eventlog generated

> > > in u-boot(done in efi_tcg2.c with this patch series) should be appended

> > > into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

> >

> > I suppose so, but this logic should be implemented in the TPM layer I

> > think, not just in EFI. Otherwise we end up with another parallel

> > implementation.

> 

> Current u-boot/lib/efi_loader/efi_tcg2.c includes the code

> not directly related to EFI.

> I would like to suggest to divide u-boot/lib/efi_loader/efi_tcg2.c

> into two files.

> 

>  1)  u-boot/lib/efi_loader/efi_tcg2.c

>   This file implements the EFI interfaces required in TCG EFI Protocol

> Specification(https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/).

> 


The only problem I see with the way the efi tcg2 eventlog is currently
created, is that it's all done during the efi init.  Ideally this should
happen earlier, especially if SPL or TF-A create their own eventlog.

The status with TF-A atm is that it only creates an eventlog which then
copies to secure and non-secure memory, but it doesnt extend the PCRs. 
We should pick that eventlog from u-boot (or better op-tee), extend the 
PCRs based on the information of the log and then use it as our basis and
start appending events there. 

>  2) u-boot/lib/tcg2.c(new file)

>  This file implements the functionality required in TCG PC Client

> Platform Firmware Profile

> Specification(https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/).

>  This file contains the common functions to extend eventlog and PCRs, etc.


Splitting up the pc client spec bits is probably a good idea. 
What do you have in mind? Moving tcg2_pcr_extend() and
tcg2_agile_log_append() as well, or just the pc client related wrappers?

> 

> In addition, this is different topic, I found some tpm related files

> under u-boot/lib directory, I think it better to have new directory

> like "tcg" and move tpm related files such as tpm_api.c, tpm-v2.c

> and tpm-common.c  into lib/tcg(new directory).


+1

Regards
/Ilias
> 

> Thanks,

> Masahisa Kojima

> 

> >

> > Regards,

> > Simon
Masahisa Kojima July 15, 2021, 7:50 a.m. UTC | #11
On Thu, 15 Jul 2021 at 15:46, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>

>

> On Thu, Jul 15, 2021 at 02:09:57PM +0900, Masahisa Kojima wrote:

> > Hi Simon, Ilias,

> >

> > On Wed, 14 Jul 2021 at 23:50, Simon Glass <sjg@chromium.org> wrote:

> > >

> > > Hi Masahisa,

> > >

> > > On Mon, 12 Jul 2021 at 02:40, Masahisa Kojima

> > > <masahisa.kojima@linaro.org> wrote:

> > > >

> > > > Hi Simon,

> > > >

> > > > On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> > > > >

> > > > > Hi Masahisa,

> > > > >

> > > > > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> > > > > >

> > > > > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > > > > > >

> > > > > > >

> > > > > > >

> > > > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > > > > > This is a preperation to add eventlog support

> > > > > > > > described in TCG PC Client PFP spec.

> > > > > > > >

> > > > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > > > > > ---

> > > > > > > >   lib/efi_loader/Kconfig | 2 +-

> > > > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > > > > > >

> > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > > > > > index b2ab48a048..a87bf3cc98 100644

> > > > > > > > --- a/lib/efi_loader/Kconfig

> > > > > > > > +++ b/lib/efi_loader/Kconfig

> > > > > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > > > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > > > > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > > > > > >       depends on EFI_TCG2_PROTOCOL

> > > > > > > > -     default 4096

> > > > > > > > +     default 16384

> > > > > > >

> > > > > > > I found this text in EDK II:

> > > > > > >

> > > > > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > > > > > -----------------------------------------------------------------------

> > > > > > >

> > > > > > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > > > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> > > > > >

> > > > > > Thank you for your feedback.

> > > > > > I have not checked this.

> > > > > > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > > > > > MUST be at least 64KB." in ACPI chapter.

> > > > > > I will update to set 64KB as default.

> > > > > >

> > > > >

> > > > > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> > > > > put this in the bloblist? We want to avoid adding code in EFI which is

> > > > > in U-Boot.

> > > >

> > > > I think bloblist is used for data passing from SPL/TPL to u-boot.

> > >

> > > It can also be used to place things in memory that end up accessed by

> > > Linux, e.g. ACPI tables. So I think it fits.

> >

> > I understand bloblist can be used for eventlog, I will check further.

> >

> > >

> > > > Is your comment saying that the eventlog generated

> > > > in u-boot(done in efi_tcg2.c with this patch series) should be appended

> > > > into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

> > >

> > > I suppose so, but this logic should be implemented in the TPM layer I

> > > think, not just in EFI. Otherwise we end up with another parallel

> > > implementation.

> >

> > Current u-boot/lib/efi_loader/efi_tcg2.c includes the code

> > not directly related to EFI.

> > I would like to suggest to divide u-boot/lib/efi_loader/efi_tcg2.c

> > into two files.

> >

> >  1)  u-boot/lib/efi_loader/efi_tcg2.c

> >   This file implements the EFI interfaces required in TCG EFI Protocol

> > Specification(https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/).

> >

>

> The only problem I see with the way the efi tcg2 eventlog is currently

> created, is that it's all done during the efi init.  Ideally this should

> happen earlier, especially if SPL or TF-A create their own eventlog.

>

> The status with TF-A atm is that it only creates an eventlog which then

> copies to secure and non-secure memory, but it doesnt extend the PCRs.

> We should pick that eventlog from u-boot (or better op-tee), extend the

> PCRs based on the information of the log and then use it as our basis and

> start appending events there.

>

> >  2) u-boot/lib/tcg2.c(new file)

> >  This file implements the functionality required in TCG PC Client

> > Platform Firmware Profile

> > Specification(https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/).

> >  This file contains the common functions to extend eventlog and PCRs, etc.

>

> Splitting up the pc client spec bits is probably a good idea.

> What do you have in mind? Moving tcg2_pcr_extend() and

> tcg2_agile_log_append() as well, or just the pc client related wrappers?


Sorry but I was confused.
I checked spec again, there are many duplication in TCG EFI Protocol spec
and TCG PC Client PFP spec.
For example,  tdTCG_EfiSpecIdEvent structure is defined in both spec.
On second thought, it is difficult to split the pc client spec into new file,
so I would like to withdraw my suggestion earlier.

Thanks,
Masahisa Kojima


>

> >

> > In addition, this is different topic, I found some tpm related files

> > under u-boot/lib directory, I think it better to have new directory

> > like "tcg" and move tpm related files such as tpm_api.c, tpm-v2.c

> > and tpm-common.c  into lib/tcg(new directory).

>

> +1

>

> Regards

> /Ilias

> >

> > Thanks,

> > Masahisa Kojima

> >

> > >

> > > Regards,

> > > Simon
Simon Glass July 15, 2021, 12:57 p.m. UTC | #12
Hi Ilias,

On Thu, 15 Jul 2021 at 00:20, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>

> On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:

> > Hi Ilias,

> >

> > On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas

> > <ilias.apalodimas@linaro.org> wrote:

> > >

> > > On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima

> > > <masahisa.kojima@linaro.org> wrote:

> > > >

> > > > Hi Simon,

> > > >

> > > > On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> > > > >

> > > > > Hi Masahisa,

> > > > >

> > > > > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> > > > > >

> > > > > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> > > > > > >

> > > > > > >

> > > > > > >

> > > > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> > > > > > > > This is a preperation to add eventlog support

> > > > > > > > described in TCG PC Client PFP spec.

> > > > > > > >

> > > > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> > > > > > > > ---

> > > > > > > >   lib/efi_loader/Kconfig | 2 +-

> > > > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)

> > > > > > > >

> > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> > > > > > > > index b2ab48a048..a87bf3cc98 100644

> > > > > > > > --- a/lib/efi_loader/Kconfig

> > > > > > > > +++ b/lib/efi_loader/Kconfig

> > > > > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> > > > > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> > > > > > > >       int "EFI_TCG2_PROTOCOL EventLog size"

> > > > > > > >       depends on EFI_TCG2_PROTOCOL

> > > > > > > > -     default 4096

> > > > > > > > +     default 16384

> > > > > > >

> > > > > > > I found this text in EDK II:

> > > > > > >

> > > > > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> > > > > > > -----------------------------------------------------------------------

> > > > > > >

> > > > > > > For PC Client Implementation spec up to and including 1.2 the minimum

> > > > > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> > > > > >

> > > > > > Thank you for your feedback.

> > > > > > I have not checked this.

> > > > > > TCG spec also says "The Log Area Minimum Length for the TCG event log

> > > > > > MUST be at least 64KB." in ACPI chapter.

> > > > > > I will update to set 64KB as default.

> > > > > >

> > > > >

> > > > > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> > > > > put this in the bloblist? We want to avoid adding code in EFI which is

> > > > > in U-Boot.

> > > >

> > > > I think bloblist is used for data passing from SPL/TPL to u-boot.

> > > > Is your comment saying that the eventlog generated

> > > > in u-boot(done in efi_tcg2.c with this patch series) should be appended

> > > > into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

> > > >

> > >

> > > Even in that case the eventlog can't be appended.  The TCG eventlog

> > > hould be copied into EFI memory, since the kernel expects to find it

> > > there.

> >

> > Typically bloblist is relocated by U-Boot. There are lots of tables

> > that must be passed to linux, including ACPI and SMBIOS. With bloblist

> > they can all be in one place.

>

>

> The eventlog must be allocated in EFI memory though.


There is really only one memory in U-Boot. I feel that all stuff that
EFI passes on to linux should be in a single bloblist.

>

> >

> > > What we could do is copy the contents of that buffer to the eventlog.

> > > Depending on what that buffer already has (e.g the starting header of

> > > the eventlog), we might need to strip it from the efi_tcg.c code.

> >

> > I'm not really sure, but the eventlog is not just EFI thing, right?

> > The code should be generic.

>

> It's purely an EFI construct.  Specifically the entire spec, and even the  log

> format for the eventlog are described in

> https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf


For some reason I have seen this in ACPI, or something similar.
Perhaps I was getting confused.

We need to find ways to implement EFI things with generic code. I'm
not 100% sure about this particular thing, but since we already create
something similar with ACPI I think we should at least look at doing
it in one place.

Regards,
Simon
Heinrich Schuchardt July 15, 2021, 2:33 p.m. UTC | #13
On 7/15/21 2:57 PM, Simon Glass wrote:
> Hi Ilias,

>

> On Thu, 15 Jul 2021 at 00:20, Ilias Apalodimas

> <ilias.apalodimas@linaro.org> wrote:

>>

>> On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:

>>> Hi Ilias,

>>>

>>> On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas

>>> <ilias.apalodimas@linaro.org> wrote:

>>>>

>>>> On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima

>>>> <masahisa.kojima@linaro.org> wrote:

>>>>>

>>>>> Hi Simon,

>>>>>

>>>>> On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

>>>>>>

>>>>>> Hi Masahisa,

>>>>>>

>>>>>> On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

>>>>>>>

>>>>>>> On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

>>>>>>>>

>>>>>>>>

>>>>>>>>

>>>>>>>> On 7/7/21 3:36 PM, Masahisa Kojima wrote:

>>>>>>>>> This is a preperation to add eventlog support

>>>>>>>>> described in TCG PC Client PFP spec.

>>>>>>>>>

>>>>>>>>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

>>>>>>>>> ---

>>>>>>>>>    lib/efi_loader/Kconfig | 2 +-

>>>>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)

>>>>>>>>>

>>>>>>>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

>>>>>>>>> index b2ab48a048..a87bf3cc98 100644

>>>>>>>>> --- a/lib/efi_loader/Kconfig

>>>>>>>>> +++ b/lib/efi_loader/Kconfig

>>>>>>>>> @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

>>>>>>>>>    config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

>>>>>>>>>        int "EFI_TCG2_PROTOCOL EventLog size"

>>>>>>>>>        depends on EFI_TCG2_PROTOCOL

>>>>>>>>> -     default 4096

>>>>>>>>> +     default 16384

>>>>>>>>

>>>>>>>> I found this text in EDK II:

>>>>>>>>

>>>>>>>> Minimum length(in bytes) of the system preboot TCG event log area(LAML)

>>>>>>>> -----------------------------------------------------------------------

>>>>>>>>

>>>>>>>> For PC Client Implementation spec up to and including 1.2 the minimum

>>>>>>>> log size is 64KB. (SecurityPkg/SecurityPkg.dec)

>>>>>>>

>>>>>>> Thank you for your feedback.

>>>>>>> I have not checked this.

>>>>>>> TCG spec also says "The Log Area Minimum Length for the TCG event log

>>>>>>> MUST be at least 64KB." in ACPI chapter.

>>>>>>> I will update to set 64KB as default.

>>>>>>>

>>>>>>

>>>>>> Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

>>>>>> put this in the bloblist? We want to avoid adding code in EFI which is

>>>>>> in U-Boot.

>>>>>

>>>>> I think bloblist is used for data passing from SPL/TPL to u-boot.

>>>>> Is your comment saying that the eventlog generated

>>>>> in u-boot(done in efi_tcg2.c with this patch series) should be appended

>>>>> into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

>>>>>

>>>>

>>>> Even in that case the eventlog can't be appended.  The TCG eventlog

>>>> hould be copied into EFI memory, since the kernel expects to find it

>>>> there.

>>>

>>> Typically bloblist is relocated by U-Boot. There are lots of tables

>>> that must be passed to linux, including ACPI and SMBIOS. With bloblist

>>> they can all be in one place.

>>

>>

>> The eventlog must be allocated in EFI memory though.

>

> There is really only one memory in U-Boot. I feel that all stuff that

> EFI passes on to linux should be in a single bloblist.


We have should follow existing standards and not invent our own. LInux
is not the only OS booted via U-Boot.

Best regards

Heinrich

>

>>

>>>

>>>> What we could do is copy the contents of that buffer to the eventlog.

>>>> Depending on what that buffer already has (e.g the starting header of

>>>> the eventlog), we might need to strip it from the efi_tcg.c code.

>>>

>>> I'm not really sure, but the eventlog is not just EFI thing, right?

>>> The code should be generic.

>>

>> It's purely an EFI construct.  Specifically the entire spec, and even the  log

>> format for the eventlog are described in

>> https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf

>

> For some reason I have seen this in ACPI, or something similar.

> Perhaps I was getting confused.

>

> We need to find ways to implement EFI things with generic code. I'm

> not 100% sure about this particular thing, but since we already create

> something similar with ACPI I think we should at least look at doing

> it in one place.

>

> Regards,

> Simon

>
Simon Glass July 15, 2021, 3:18 p.m. UTC | #14
Hi Heinrich,

On Thu, 15 Jul 2021 at 08:38, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>

> On 7/15/21 2:57 PM, Simon Glass wrote:

> > Hi Ilias,

> >

> > On Thu, 15 Jul 2021 at 00:20, Ilias Apalodimas

> > <ilias.apalodimas@linaro.org> wrote:

> >>

> >> On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:

> >>> Hi Ilias,

> >>>

> >>> On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas

> >>> <ilias.apalodimas@linaro.org> wrote:

> >>>>

> >>>> On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima

> >>>> <masahisa.kojima@linaro.org> wrote:

> >>>>>

> >>>>> Hi Simon,

> >>>>>

> >>>>> On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> >>>>>>

> >>>>>> Hi Masahisa,

> >>>>>>

> >>>>>> On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> >>>>>>>

> >>>>>>> On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> >>>>>>>>

> >>>>>>>>

> >>>>>>>>

> >>>>>>>> On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> >>>>>>>>> This is a preperation to add eventlog support

> >>>>>>>>> described in TCG PC Client PFP spec.

> >>>>>>>>>

> >>>>>>>>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> >>>>>>>>> ---

> >>>>>>>>>    lib/efi_loader/Kconfig | 2 +-

> >>>>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)

> >>>>>>>>>

> >>>>>>>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> >>>>>>>>> index b2ab48a048..a87bf3cc98 100644

> >>>>>>>>> --- a/lib/efi_loader/Kconfig

> >>>>>>>>> +++ b/lib/efi_loader/Kconfig

> >>>>>>>>> @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> >>>>>>>>>    config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> >>>>>>>>>        int "EFI_TCG2_PROTOCOL EventLog size"

> >>>>>>>>>        depends on EFI_TCG2_PROTOCOL

> >>>>>>>>> -     default 4096

> >>>>>>>>> +     default 16384

> >>>>>>>>

> >>>>>>>> I found this text in EDK II:

> >>>>>>>>

> >>>>>>>> Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> >>>>>>>> -----------------------------------------------------------------------

> >>>>>>>>

> >>>>>>>> For PC Client Implementation spec up to and including 1.2 the minimum

> >>>>>>>> log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> >>>>>>>

> >>>>>>> Thank you for your feedback.

> >>>>>>> I have not checked this.

> >>>>>>> TCG spec also says "The Log Area Minimum Length for the TCG event log

> >>>>>>> MUST be at least 64KB." in ACPI chapter.

> >>>>>>> I will update to set 64KB as default.

> >>>>>>>

> >>>>>>

> >>>>>> Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> >>>>>> put this in the bloblist? We want to avoid adding code in EFI which is

> >>>>>> in U-Boot.

> >>>>>

> >>>>> I think bloblist is used for data passing from SPL/TPL to u-boot.

> >>>>> Is your comment saying that the eventlog generated

> >>>>> in u-boot(done in efi_tcg2.c with this patch series) should be appended

> >>>>> into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

> >>>>>

> >>>>

> >>>> Even in that case the eventlog can't be appended.  The TCG eventlog

> >>>> hould be copied into EFI memory, since the kernel expects to find it

> >>>> there.

> >>>

> >>> Typically bloblist is relocated by U-Boot. There are lots of tables

> >>> that must be passed to linux, including ACPI and SMBIOS. With bloblist

> >>> they can all be in one place.

> >>

> >>

> >> The eventlog must be allocated in EFI memory though.

> >

> > There is really only one memory in U-Boot. I feel that all stuff that

> > EFI passes on to linux should be in a single bloblist.

>

> We have should follow existing standards and not invent our own. LInux

> is not the only OS booted via U-Boot.


Perhaps we can talk about it in the next call. My point is not about
avoiding standards!

What I am saying is that if we put things in a bloblist, and make that
available to Linux (or other OS) via EFI, things should work, but
non-EFI people are happy too. See the ACPI stuff for example - we put
all of those bits in a bloblist, which is really just a contiguous
area of memory. It is more convenient for U-Boot than allocating
memory willy nilly. Plus the 'bloblist' command lets you see what is
there.

Anyway I really don't understand all of this well enough to say what
we should do. I am just passing on hints. There is way too much
'separate' EFI code in U-Boot at present and we need to work to reduce
that and hopefully not add more.

[..]

Regards,
Simon
Heinrich Schuchardt July 15, 2021, 3:29 p.m. UTC | #15
On 15.07.21 17:18, Simon Glass wrote:
> Hi Heinrich,

>

> On Thu, 15 Jul 2021 at 08:38, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

>>

>> On 7/15/21 2:57 PM, Simon Glass wrote:

>>> Hi Ilias,

>>>

>>> On Thu, 15 Jul 2021 at 00:20, Ilias Apalodimas

>>> <ilias.apalodimas@linaro.org> wrote:

>>>>

>>>> On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:

>>>>> Hi Ilias,

>>>>>

>>>>> On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas

>>>>> <ilias.apalodimas@linaro.org> wrote:

>>>>>>

>>>>>> On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima

>>>>>> <masahisa.kojima@linaro.org> wrote:

>>>>>>>

>>>>>>> Hi Simon,

>>>>>>>

>>>>>>> On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

>>>>>>>>

>>>>>>>> Hi Masahisa,

>>>>>>>>

>>>>>>>> On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

>>>>>>>>>

>>>>>>>>> On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>> On 7/7/21 3:36 PM, Masahisa Kojima wrote:

>>>>>>>>>>> This is a preperation to add eventlog support

>>>>>>>>>>> described in TCG PC Client PFP spec.

>>>>>>>>>>>

>>>>>>>>>>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

>>>>>>>>>>> ---

>>>>>>>>>>>     lib/efi_loader/Kconfig | 2 +-

>>>>>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)

>>>>>>>>>>>

>>>>>>>>>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

>>>>>>>>>>> index b2ab48a048..a87bf3cc98 100644

>>>>>>>>>>> --- a/lib/efi_loader/Kconfig

>>>>>>>>>>> +++ b/lib/efi_loader/Kconfig

>>>>>>>>>>> @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

>>>>>>>>>>>     config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

>>>>>>>>>>>         int "EFI_TCG2_PROTOCOL EventLog size"

>>>>>>>>>>>         depends on EFI_TCG2_PROTOCOL

>>>>>>>>>>> -     default 4096

>>>>>>>>>>> +     default 16384

>>>>>>>>>>

>>>>>>>>>> I found this text in EDK II:

>>>>>>>>>>

>>>>>>>>>> Minimum length(in bytes) of the system preboot TCG event log area(LAML)

>>>>>>>>>> -----------------------------------------------------------------------

>>>>>>>>>>

>>>>>>>>>> For PC Client Implementation spec up to and including 1.2 the minimum

>>>>>>>>>> log size is 64KB. (SecurityPkg/SecurityPkg.dec)

>>>>>>>>>

>>>>>>>>> Thank you for your feedback.

>>>>>>>>> I have not checked this.

>>>>>>>>> TCG spec also says "The Log Area Minimum Length for the TCG event log

>>>>>>>>> MUST be at least 64KB." in ACPI chapter.

>>>>>>>>> I will update to set 64KB as default.

>>>>>>>>>

>>>>>>>>

>>>>>>>> Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

>>>>>>>> put this in the bloblist? We want to avoid adding code in EFI which is

>>>>>>>> in U-Boot.

>>>>>>>

>>>>>>> I think bloblist is used for data passing from SPL/TPL to u-boot.

>>>>>>> Is your comment saying that the eventlog generated

>>>>>>> in u-boot(done in efi_tcg2.c with this patch series) should be appended

>>>>>>> into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

>>>>>>>

>>>>>>

>>>>>> Even in that case the eventlog can't be appended.  The TCG eventlog

>>>>>> hould be copied into EFI memory, since the kernel expects to find it

>>>>>> there.

>>>>>

>>>>> Typically bloblist is relocated by U-Boot. There are lots of tables

>>>>> that must be passed to linux, including ACPI and SMBIOS. With bloblist

>>>>> they can all be in one place.

>>>>

>>>>

>>>> The eventlog must be allocated in EFI memory though.

>>>

>>> There is really only one memory in U-Boot. I feel that all stuff that

>>> EFI passes on to linux should be in a single bloblist.

>>

>> We have should follow existing standards and not invent our own. LInux

>> is not the only OS booted via U-Boot.

>

> Perhaps we can talk about it in the next call. My point is not about

> avoiding standards!

>

> What I am saying is that if we put things in a bloblist, and make that

> available to Linux (or other OS) via EFI, things should work, but


Which operating would be aware of your bloblist? Windows, BSD, Haiku?

We want U-Boot to be interchangable with other UEFI firmware like EDK
II. This will only work if we program against the same specs and don't
invent new interfaces.

Best regards

Heinrich

> non-EFI people are happy too. See the ACPI stuff for example - we put

> all of those bits in a bloblist, which is really just a contiguous

> area of memory. It is more convenient for U-Boot than allocating

> memory willy nilly. Plus the 'bloblist' command lets you see what is

> there.

>

> Anyway I really don't understand all of this well enough to say what

> we should do. I am just passing on hints. There is way too much

> 'separate' EFI code in U-Boot at present and we need to work to reduce

> that and hopefully not add more.

>

> [..]

>

> Regards,

> Simon

>
Simon Glass July 15, 2021, 4:09 p.m. UTC | #16
Hi Heinrich,

On Thu, 15 Jul 2021 at 09:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>

>

>

> On 15.07.21 17:18, Simon Glass wrote:

> > Hi Heinrich,

> >

> > On Thu, 15 Jul 2021 at 08:38, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> >>

> >> On 7/15/21 2:57 PM, Simon Glass wrote:

> >>> Hi Ilias,

> >>>

> >>> On Thu, 15 Jul 2021 at 00:20, Ilias Apalodimas

> >>> <ilias.apalodimas@linaro.org> wrote:

> >>>>

> >>>> On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:

> >>>>> Hi Ilias,

> >>>>>

> >>>>> On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas

> >>>>> <ilias.apalodimas@linaro.org> wrote:

> >>>>>>

> >>>>>> On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima

> >>>>>> <masahisa.kojima@linaro.org> wrote:

> >>>>>>>

> >>>>>>> Hi Simon,

> >>>>>>>

> >>>>>>> On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:

> >>>>>>>>

> >>>>>>>> Hi Masahisa,

> >>>>>>>>

> >>>>>>>> On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:

> >>>>>>>>>

> >>>>>>>>> On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> >>>>>>>>>>

> >>>>>>>>>>

> >>>>>>>>>>

> >>>>>>>>>> On 7/7/21 3:36 PM, Masahisa Kojima wrote:

> >>>>>>>>>>> This is a preperation to add eventlog support

> >>>>>>>>>>> described in TCG PC Client PFP spec.

> >>>>>>>>>>>

> >>>>>>>>>>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

> >>>>>>>>>>> ---

> >>>>>>>>>>>     lib/efi_loader/Kconfig | 2 +-

> >>>>>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)

> >>>>>>>>>>>

> >>>>>>>>>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig

> >>>>>>>>>>> index b2ab48a048..a87bf3cc98 100644

> >>>>>>>>>>> --- a/lib/efi_loader/Kconfig

> >>>>>>>>>>> +++ b/lib/efi_loader/Kconfig

> >>>>>>>>>>> @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL

> >>>>>>>>>>>     config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE

> >>>>>>>>>>>         int "EFI_TCG2_PROTOCOL EventLog size"

> >>>>>>>>>>>         depends on EFI_TCG2_PROTOCOL

> >>>>>>>>>>> -     default 4096

> >>>>>>>>>>> +     default 16384

> >>>>>>>>>>

> >>>>>>>>>> I found this text in EDK II:

> >>>>>>>>>>

> >>>>>>>>>> Minimum length(in bytes) of the system preboot TCG event log area(LAML)

> >>>>>>>>>> -----------------------------------------------------------------------

> >>>>>>>>>>

> >>>>>>>>>> For PC Client Implementation spec up to and including 1.2 the minimum

> >>>>>>>>>> log size is 64KB. (SecurityPkg/SecurityPkg.dec)

> >>>>>>>>>

> >>>>>>>>> Thank you for your feedback.

> >>>>>>>>> I have not checked this.

> >>>>>>>>> TCG spec also says "The Log Area Minimum Length for the TCG event log

> >>>>>>>>> MUST be at least 64KB." in ACPI chapter.

> >>>>>>>>> I will update to set 64KB as default.

> >>>>>>>>>

> >>>>>>>>

> >>>>>>>> Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we

> >>>>>>>> put this in the bloblist? We want to avoid adding code in EFI which is

> >>>>>>>> in U-Boot.

> >>>>>>>

> >>>>>>> I think bloblist is used for data passing from SPL/TPL to u-boot.

> >>>>>>> Is your comment saying that the eventlog generated

> >>>>>>> in u-boot(done in efi_tcg2.c with this patch series) should be appended

> >>>>>>> into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

> >>>>>>>

> >>>>>>

> >>>>>> Even in that case the eventlog can't be appended.  The TCG eventlog

> >>>>>> hould be copied into EFI memory, since the kernel expects to find it

> >>>>>> there.

> >>>>>

> >>>>> Typically bloblist is relocated by U-Boot. There are lots of tables

> >>>>> that must be passed to linux, including ACPI and SMBIOS. With bloblist

> >>>>> they can all be in one place.

> >>>>

> >>>>

> >>>> The eventlog must be allocated in EFI memory though.

> >>>

> >>> There is really only one memory in U-Boot. I feel that all stuff that

> >>> EFI passes on to linux should be in a single bloblist.

> >>

> >> We have should follow existing standards and not invent our own. LInux

> >> is not the only OS booted via U-Boot.

> >

> > Perhaps we can talk about it in the next call. My point is not about

> > avoiding standards!

> >

> > What I am saying is that if we put things in a bloblist, and make that

> > available to Linux (or other OS) via EFI, things should work, but

>

> Which operating would be aware of your bloblist? Windows, BSD, Haiku?


None, it is not necessary. The bloblist is a U-Boot construct, a
container for blobs. We can pass a pointer to the blob through the EFI
tables, as we do with ACPI.

>

> We want U-Boot to be interchangable with other UEFI firmware like EDK

> II. This will only work if we program against the same specs and don't

> invent new interfaces.


This is not a new interface. Let's chat about it in a contributor call.

Regards,
Simon

>

> Best regards

>

> Heinrich

>

> > non-EFI people are happy too. See the ACPI stuff for example - we put

> > all of those bits in a bloblist, which is really just a contiguous

> > area of memory. It is more convenient for U-Boot than allocating

> > memory willy nilly. Plus the 'bloblist' command lets you see what is

> > there.

> >

> > Anyway I really don't understand all of this well enough to say what

> > we should do. I am just passing on hints. There is way too much

> > 'separate' EFI code in U-Boot at present and we need to work to reduce

> > that and hopefully not add more.

> >

> > [..]

> >

> > Regards,

> > Simon

> >
diff mbox series

Patch

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index b2ab48a048..a87bf3cc98 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -327,7 +327,7 @@  config EFI_TCG2_PROTOCOL
 config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE
 	int "EFI_TCG2_PROTOCOL EventLog size"
 	depends on EFI_TCG2_PROTOCOL
-	default 4096
+	default 16384
 	help
 		Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that
 		this is going to be allocated twice. One for the eventlog it self