Hi Heinrich,
Thank you for your reviews.
> -----Original Message-----
> From: Heinrich Schuchardt <xypron.g...@gmx.de>
> Sent: 17 December 2021 17:27
> To: Jose Marinho <jose.mari...@arm.com>; u-boot@lists.denx.de
> Cc: ilias.apalodi...@linaro.org; sughosh.g...@linaro.org;
> takahiro.aka...@linaro.org; ag...@csgraf.de; nd <n...@arm.com>
> Subject: Re: [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile
>
> On 12/17/21 13:55, Jose Marinho wrote:
> > Display the EBBRv2.0 conformance in the ECPT table.
> >
> > The EBBRv2.0 conformance profile is set in the ECPT if
> > CONFIG_EFI_EBBR_2_0_CONFORMANCE=y.
> > The config defaults to 'n'.
> >
> >
> > Signed-off-by: Jose Marinho <jose.mari...@arm.com>
> > ---
> > include/efi_api.h | 4 ++++
> > lib/efi_loader/Kconfig | 6 ++++++
> > lib/efi_loader/efi_conformance.c | 9 +++++++++
> > 3 files changed, 19 insertions(+)
> >
> > diff --git a/include/efi_api.h b/include/efi_api.h index
> > 6fd4f04de3..49919caa35 100644
> > --- a/include/efi_api.h
> > +++ b/include/efi_api.h
> > @@ -230,6 +230,10 @@ enum efi_reset_type {
> > EFI_GUID(0x36122546, 0xf7ef, 0x4c8f, 0xbd, 0x9b, \
> > 0xeb, 0x85, 0x25, 0xb5, 0x0c, 0x0b)
> >
> > +#define EFI_CONFORMANCE_PROFILE_EBBR_2_0_GUID \
> > + EFI_GUID(0xcce33c35, 0x74ac, 0x4087, 0xbc, 0xe7, \
> > + 0x8b, 0x29, 0xb0, 0x2e, 0xeb, 0x27)
> > +
> > struct efi_conformance_profiles_table {
> > u16 version;
> > u16 number_of_profiles;
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index
> > b2398976f4..ab7476f68b 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -373,4 +373,10 @@ config EFI_ECPT
> > help
> > Enabling this option created the ECPT UEFI table.
> >
> > +config EFI_EBBR_2_0_CONFORMANCE
> > + bool "Add the EBBRv2.0 conformance entry to the ECPT table"
> > + depends on EFI_ECPT
>
> With this dependency the symbol EFI_EBBR_2_0_CONFORMANCE is
> superfluous.
>
> Can we add EFI_EBBR_2_0_CONFORMANCE unconditionally or are there
> relevant configuration flags in U-Boot that must be enabled to claim EBBR 2.0
> compliance? E.g.
>
> * CONFIG_CMD_BOOTEFI_BOOTMGR
> * CONFIG_EFI_GET_TIME
> * CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT
>
I've removed the "depends on" in PATCH v2.
Ideally the EFI_EBBR_2_0_CONFORMANCE should depend on all the CONFIGS required
by EBBR 2.0.
I'm not sure whether this is feasible, i.e. whether there is a set of CONFIGS_*
which when enabled make the implementation EBBRv2.0 compliant.
Also, as the u-boot code evolves, these dependencies would need to be carefully
maintained perhaps.
Perhaps the best choice is to let the FW provider to set EBBR_2_0_CONFORMANCE
in the platform config file once the FW has been deemed EBBRv2.0 compliant.
>
> > + default n
> > + help
> > + Enabling this option adds the EBBRv2.0 conformance entry to the
> ECPT UEFI table.
> > endif
> > diff --git a/lib/efi_loader/efi_conformance.c
> > b/lib/efi_loader/efi_conformance.c
> > index 86c26d6b79..b490ff3326 100644
> > --- a/lib/efi_loader/efi_conformance.c
> > +++ b/lib/efi_loader/efi_conformance.c
> > @@ -12,6 +12,7 @@
> > #include <malloc.h>
> >
> > const efi_guid_t efi_ecpt_guid =
> > EFI_CONFORMANCE_PROFILES_TABLE_GUID;
> > +const efi_guid_t efi_ebbr_2_0_guid =
> > +EFI_CONFORMANCE_PROFILE_EBBR_2_0_GUID;
> >
> > #define EFI_CONFORMANCE_PROFILES_TABLE_VERSION 1
> >
> > @@ -29,6 +30,9 @@ efi_status_t efi_ecpt_register(void)
> >
> > EFI_PRINT("ECPT table creation start\n");
> >
> > + if (IS_ENABLED(CONFIG_EFI_EBBR_2_0_CONFORMANCE))
> > + num_entries++;
> > +
> > ecpt_size = num_entries * sizeof(efi_guid_t)
> > + sizeof(struct efi_conformance_profiles_table);
> > ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, ecpt_size, @@ -
> 44,6
> > +48,11 @@ efi_status_t efi_ecpt_register(void)
> > ecpt->version = EFI_CONFORMANCE_PROFILES_TABLE_VERSION;
> > ecpt->number_of_profiles = num_entries;
> >
> > + if (IS_ENABLED(CONFIG_EFI_EBBR_2_0_CONFORMANCE)) {
> > + num_entries--;
> > + guidcpy(&ecpt->conformance_profiles[num_entries],
> &efi_ecpt_guid);
> > + }
> > +
> > if (num_entries)
> > EFI_PRINT("ECPT check conformance profiles, not all entries
> > populated in table\n");
> >
