Re: [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 12/23/21 15:57, Jose Marinho wrote:
> Hi Heinrich,
>
> Thank you for your reviews.
>
>> -----Original Message-----
>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Sent: 17 December 2021 17:27
>> To: Jose Marinho <Jose.Marinho@arm.com>; u-boot@lists.denx.de
>> Cc: ilias.apalodimas@linaro.org; sughosh.ganu@linaro.org;
>> takahiro.akashi@linaro.org; agraf@csgraf.de; nd <nd@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.marinho@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.

The firmware provider is the U-Boot project. If we do not know under
which circumstances we might add the EBBR 2.0 conformance GUID, I prefer
not to implement the table at all.

Best regards

Heinrich

>>
>>> +	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");
>>>
>