On 2/11/26 12:47, Vincent Stehlé wrote:
On Wed, Feb 11, 2026 at 10:56:30AM +0100, Heinrich Schuchardt wrote:
On 2/11/26 10:33, Vincent Stehlé wrote:
The size of the memory allocated for the EFI Conformance Profiles Table is
computed with `num_entries' always equal to zero, which is incorrect when
CONFIG_EFI_EBBR_2_1_CONFORMANCE is enabled.
This can be verified by allocating the ECPT memory with malloc() instead of
efi_allocate_pool(), building u-boot with sandbox_defconfig and
CONFIG_VALGRIND=y, and by finally running the following command:
valgrind --suppressions=scripts/u-boot.supp \
./u-boot -T -c 'efidebug tables'
Fix this by using an array of the supported profiles GUIDs instead, which
should also be easier to extend in the future.
Thank you Vincent for this patch.
Hi Heinrich,
Thanks for reviewing.
Maybe mention in the commit message that U-Boot should publish all supported
EBBR revisions.
Ok, will do.
Fixes: 6b92c1735205 ("efi: Create ECPT table")
Signed-off-by: Vincent Stehlé <[email protected]>
Cc: Heinrich Schuchardt <[email protected]>
Cc: Ilias Apalodimas <[email protected]>
Cc: Tom Rini <[email protected]>
Cc: Jose Marinho <[email protected]>
---
lib/efi_loader/efi_conformance.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/efi_loader/efi_conformance.c b/lib/efi_loader/efi_conformance.c
index 2bae93a94bd..9768b5ae824 100644
--- a/lib/efi_loader/efi_conformance.c
+++ b/lib/efi_loader/efi_conformance.c
@@ -13,8 +13,12 @@
#include <malloc.h>
static const efi_guid_t efi_ecpt_guid = EFI_CONFORMANCE_PROFILES_TABLE_GUID;
-static const efi_guid_t efi_ebbr_2_1_guid =
- EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID;
+
+static const efi_guid_t profiles[] = {
As profiles[] is not used outside efi_ecpt_register(), there is no need to
make it a static variable.
I am not sure why a "global" symbol would be better than a static one, but if
you like this solution we could make the profiles[] array extern.
Sorry to not be clear. I wanted to suggest a local symbol.
Best regards
Heinrich
Then I guess we should rename it with a much longer name to avoid namespace
"pollution":
const efi_guid_t efi_conformance_profiles[] = {
...
All the sizeof() and ARRAY_SIZE() would have to follow, too.
Alternatively, we could move the array declaration inside the
efi_ecpt_register() function completely:
efi_status_t efi_ecpt_register(void)
{
struct efi_conformance_profiles_table *ecpt;
efi_status_t ret;
size_t ecpt_size;
static const efi_guid_t profiles[] = {
#if CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE)
EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID,
#endif
};
...
This is still static here, but for storage duration not linkage. One advantage
is that the name would not have to change. I am not a big fan of the #if in
function scope, but if you prefer this solution this is fine as well.
Best regards,
Vincent.
Otherwise looks good to me.
Best regards
Heinrich
+#if CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE)
+ EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID,
+#endif
+};
/**
* efi_ecpt_register() - Install the ECPT system table.
@@ -23,12 +27,11 @@ static const efi_guid_t efi_ebbr_2_1_guid =
*/
efi_status_t efi_ecpt_register(void)
{
- u16 num_entries = 0;
struct efi_conformance_profiles_table *ecpt;
efi_status_t ret;
size_t ecpt_size;
- ecpt_size = num_entries * sizeof(efi_guid_t)
+ ecpt_size = sizeof(profiles)
+ sizeof(struct efi_conformance_profiles_table);
ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, ecpt_size,
(void **)&ecpt);
@@ -39,12 +42,9 @@ efi_status_t efi_ecpt_register(void)
return ret;
}
- if (CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE))
- guidcpy(&ecpt->conformance_profiles[num_entries++],
- &efi_ebbr_2_1_guid);
-
+ memcpy(ecpt->conformance_profiles, profiles, sizeof(profiles));
ecpt->version = EFI_CONFORMANCE_PROFILES_TABLE_VERSION;
- ecpt->number_of_profiles = num_entries;
+ ecpt->number_of_profiles = ARRAY_SIZE(profiles);
/* Install the ECPT in the system configuration table. */
ret = efi_install_configuration_table(&efi_ecpt_guid, (void *)ecpt);
