All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Conformance Profiles Table support in U-boot
@ 2021-12-17 12:55 Jose Marinho
  2021-12-17 12:55 ` [PATCH 1/3] efi: Create ECPT table Jose Marinho
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Jose Marinho @ 2021-12-17 12:55 UTC (permalink / raw)
  To: u-boot
  Cc: Jose Marinho, ilias.apalodimas, sughosh.ganu, xypron.glpk,
	takahiro.akashi, agraf, nd

The Conformance Profiles Table (ECPT) table will be included in the UEFI
specification 2.9+.
The ECPT table was introduced in UEFI following the code-first path. The
acceptance ticket can be viewed at:
	https://bugzilla.tianocore.org/show_bug.cgi?id=3591

This patch set implements the ECPT table in U-boot.

Jose Marinho (3):
  efi: Create ECPT table
  efi: ECPT add EBBRv2.0 conformance profile
  cmd: efi: efidebug print ECPT table

 cmd/efidebug.c                   | 45 +++++++++++++++++++
 include/efi_api.h                | 14 ++++++
 include/efi_loader.h             |  9 ++++
 lib/efi_loader/Kconfig           | 12 +++++
 lib/efi_loader/Makefile          |  1 +
 lib/efi_loader/efi_conformance.c | 75 ++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c       |  6 +++
 7 files changed, 162 insertions(+)
 create mode 100644 lib/efi_loader/efi_conformance.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] efi: Create ECPT table
  2021-12-17 12:55 [PATCH 0/3] Conformance Profiles Table support in U-boot Jose Marinho
@ 2021-12-17 12:55 ` Jose Marinho
  2021-12-17 17:19   ` Heinrich Schuchardt
  2021-12-17 12:55 ` [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile Jose Marinho
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Jose Marinho @ 2021-12-17 12:55 UTC (permalink / raw)
  To: u-boot
  Cc: Jose Marinho, ilias.apalodimas, sughosh.ganu, xypron.glpk,
	takahiro.akashi, agraf, nd

The ECPT table will be included in the UEFI specification 2.9+.
The ECPT table was introduced in UEFI following the code-first path. The
acceptance ticket can be viewed at:
	https://bugzilla.tianocore.org/show_bug.cgi?id=3591

The Conformance Profiles table is a UEFI configuration table that contains
GUID of the UEFI profiles that the UEFI implementation conforms with.

The ECPT table is created when CONFIG_EFI_ECPT=y.
The config is set by default.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>
---
 cmd/efidebug.c                   |  4 ++
 include/efi_api.h                | 10 +++++
 include/efi_loader.h             |  7 ++++
 lib/efi_loader/Kconfig           |  6 +++
 lib/efi_loader/Makefile          |  1 +
 lib/efi_loader/efi_conformance.c | 66 ++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c       |  6 +++
 7 files changed, 100 insertions(+)
 create mode 100644 lib/efi_loader/efi_conformance.c

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index a977ca9c72..a53a5029fa 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -619,6 +619,10 @@ static const struct {
 		"TCG2 Final Events Table",
 		EFI_TCG2_FINAL_EVENTS_TABLE_GUID,
 	},
+	{
+		"EFI Conformance Profiles Table",
+		EFI_CONFORMANCE_PROFILES_TABLE_GUID,
+	},
 };
 
 /**
diff --git a/include/efi_api.h b/include/efi_api.h
index 80109f012b..6fd4f04de3 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -226,6 +226,16 @@ enum efi_reset_type {
 	EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \
 		 0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)
 
+#define EFI_CONFORMANCE_PROFILES_TABLE_GUID \
+	EFI_GUID(0x36122546, 0xf7ef, 0x4c8f, 0xbd, 0x9b, \
+		 0xeb, 0x85, 0x25, 0xb5, 0x0c, 0x0b)
+
+struct efi_conformance_profiles_table {
+	u16 version;
+	u16 number_of_profiles;
+	efi_guid_t	conformance_profiles[];
+} __packed;
+
 struct efi_capsule_header {
 	efi_guid_t capsule_guid;
 	u32 header_size;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index d52e399841..d20ff396d0 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -976,6 +976,13 @@ efi_status_t efi_capsule_authenticate(const void *capsule,
  */
 efi_status_t efi_esrt_register(void);
 
+/**
+ * efi_ecpt_register() - Install the ECPT system table.
+ *
+ * Return: status code
+ */
+efi_status_t efi_ecpt_register(void);
+
 /**
  * efi_esrt_populate() - Populates the ESRT entries from the FMP instances
  * present in the system.
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 700dc838dd..b2398976f4 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -367,4 +367,10 @@ config EFI_ESRT
 	help
 	  Enabling this option creates the ESRT UEFI system table.
 
+config EFI_ECPT
+	bool "Enable the UEFI ECPT generation"
+	default y
+	help
+	  Enabling this option created the ECPT UEFI table.
+
 endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index fd344cea29..9f5a0cebd1 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
 obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
 obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
 obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o
+obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
 
 EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
 $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
diff --git a/lib/efi_loader/efi_conformance.c b/lib/efi_loader/efi_conformance.c
new file mode 100644
index 0000000000..86c26d6b79
--- /dev/null
+++ b/lib/efi_loader/efi_conformance.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  EFI conformance profile table
+ *
+ *  Copyright (C) 2022 Arm Ltd.
+ */
+
+#include <common.h>
+#include <efi_loader.h>
+#include <log.h>
+#include <efi_api.h>
+#include <malloc.h>
+
+const efi_guid_t efi_ecpt_guid = EFI_CONFORMANCE_PROFILES_TABLE_GUID;
+
+#define EFI_CONFORMANCE_PROFILES_TABLE_VERSION 1
+
+/**
+ * efi_ecpt_register() - Install the ECPT system table.
+ *
+ * Return: status code
+ */
+efi_status_t efi_ecpt_register(void)
+{
+	int num_entries = 0;
+	struct efi_conformance_profiles_table *ecpt;
+	efi_status_t ret;
+	size_t ecpt_size = 0;
+
+	EFI_PRINT("ECPT table creation start\n");
+
+	ecpt_size = num_entries * sizeof(efi_guid_t)
+		+ sizeof(struct efi_conformance_profiles_table);
+	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, ecpt_size,
+				(void **)&ecpt);
+
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ECPT cannot allocate memory for %u entries (%zu bytes)\n",
+			  num_entries, ecpt_size);
+
+		return ret;
+	}
+
+	ecpt->version = EFI_CONFORMANCE_PROFILES_TABLE_VERSION;
+	ecpt->number_of_profiles = num_entries;
+
+	if (num_entries)
+		EFI_PRINT("ECPT check conformance profiles, not all entries populated in table\n");
+
+	/* Install the ECPT in the system configuration table. */
+	ret = efi_install_configuration_table(&efi_ecpt_guid, (void *)ecpt);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ECPT failed to install the ECPT in the system table\n");
+		goto error;
+	}
+
+	EFI_PRINT("ECPT table successfully created\n");
+
+	return ret;
+
+error:
+
+	ret = efi_free_pool(ecpt);
+
+	return ret;
+}
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 1aba71cd96..fa5ad13500 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -231,6 +231,12 @@ efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
+	if (IS_ENABLED(CONFIG_EFI_ECPT)) {
+		ret = efi_ecpt_register();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
 		ret = efi_esrt_register();
 		if (ret != EFI_SUCCESS)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile
  2021-12-17 12:55 [PATCH 0/3] Conformance Profiles Table support in U-boot Jose Marinho
  2021-12-17 12:55 ` [PATCH 1/3] efi: Create ECPT table Jose Marinho
@ 2021-12-17 12:55 ` Jose Marinho
  2021-12-17 17:26   ` Heinrich Schuchardt
  2021-12-17 12:55 ` [PATCH 3/3] cmd: efi: efidebug print ECPT table Jose Marinho
  2021-12-17 18:05 ` [PATCH 0/3] Conformance Profiles Table support in U-boot Heinrich Schuchardt
  3 siblings, 1 reply; 18+ messages in thread
From: Jose Marinho @ 2021-12-17 12:55 UTC (permalink / raw)
  To: u-boot
  Cc: Jose Marinho, ilias.apalodimas, sughosh.ganu, xypron.glpk,
	takahiro.akashi, agraf, nd

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] cmd: efi: efidebug print ECPT table
  2021-12-17 12:55 [PATCH 0/3] Conformance Profiles Table support in U-boot Jose Marinho
  2021-12-17 12:55 ` [PATCH 1/3] efi: Create ECPT table Jose Marinho
  2021-12-17 12:55 ` [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile Jose Marinho
@ 2021-12-17 12:55 ` Jose Marinho
  2021-12-17 13:29   ` Heinrich Schuchardt
  2021-12-17 18:07   ` Heinrich Schuchardt
  2021-12-17 18:05 ` [PATCH 0/3] Conformance Profiles Table support in U-boot Heinrich Schuchardt
  3 siblings, 2 replies; 18+ messages in thread
From: Jose Marinho @ 2021-12-17 12:55 UTC (permalink / raw)
  To: u-boot
  Cc: Jose Marinho, ilias.apalodimas, sughosh.ganu, xypron.glpk,
	takahiro.akashi, agraf, nd

Signed-off-by: Jose Marinho <jose.marinho@arm.com>
---
 cmd/efidebug.c       | 41 +++++++++++++++++++++++++++++++++++++++++
 include/efi_loader.h |  2 ++
 2 files changed, 43 insertions(+)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index a53a5029fa..c3246e1820 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -889,6 +889,38 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
 	return CMD_RET_SUCCESS;
 }
 
+#ifdef CONFIG_EFI_ECPT
+static int do_efi_ecpt(struct cmd_tbl *cmdtp, int flag,
+		       int argc, char * const argv[])
+{
+	struct efi_conformance_profiles_table *ecpt;
+
+	if (argc != 1)
+		return CMD_RET_USAGE;
+
+	for (int idx = 0; idx < systab.nr_tables; idx++)
+		if (!guidcmp(&efi_ecpt_guid, &systab.tables[idx].guid))
+			ecpt = (struct efi_system_resource_table *)systab.tables[idx].table;
+
+	if (!ecpt) {
+		log_info("ECPT: table not present\n");
+		return CMD_RET_SUCCESS;
+	}
+
+	const int num_profiles = ecpt->number_of_profiles;
+
+	printf("========================================\n");
+	printf("ECPT: version:%d\n", ecpt->version);
+	printf("ECPT: num profiles:%d\n", num_profiles);
+
+	for (int i = 0; i < num_profiles; i++)
+		printf("ECPT: profile %d = %pUL\n", i, &ecpt->conformance_profiles[i]);
+	printf("========================================\n");
+
+	return CMD_RET_SUCCESS;
+}
+#endif /* CONFIG_EFI_ECPT */
+
 /**
  * create_initrd_dp() - Create a special device for our Boot### option
  *
@@ -1681,6 +1713,11 @@ static struct cmd_tbl cmd_efidebug_sub[] = {
 			 "", ""),
 	U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
 			 "", ""),
+#ifdef CONFIG_EFI_ECPT
+	U_BOOT_CMD_MKENT(ecpt, CONFIG_SYS_MAXARGS, 1, do_efi_ecpt,
+			 "", ""),
+#endif
+
 };
 
 /**
@@ -1769,6 +1806,10 @@ static char efidebug_help_text[] =
 	"  - show UEFI memory map\n"
 	"efidebug tables\n"
 	"  - show UEFI configuration tables\n"
+#ifdef CONFIG_EFI_ECPT
+	"efidebug ecpt\n"
+	"  - show UEFI conformance profiles table\n"
+#endif
 #ifdef CONFIG_CMD_BOOTEFI_BOOTMGR
 	"efidebug test bootmgr\n"
 	"  - run simple bootmgr for test\n"
diff --git a/include/efi_loader.h b/include/efi_loader.h
index d20ff396d0..d60a340136 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -310,6 +310,8 @@ extern const efi_guid_t efi_guid_firmware_management_protocol;
 extern const efi_guid_t efi_esrt_guid;
 /* GUID of the SMBIOS table */
 extern const efi_guid_t smbios_guid;
+/* GUID for the ECPT */
+extern const efi_guid_t efi_ecpt_guid;
 
 extern char __efi_runtime_start[], __efi_runtime_stop[];
 extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] cmd: efi: efidebug print ECPT table
  2021-12-17 12:55 ` [PATCH 3/3] cmd: efi: efidebug print ECPT table Jose Marinho
@ 2021-12-17 13:29   ` Heinrich Schuchardt
  2021-12-17 18:07   ` Heinrich Schuchardt
  1 sibling, 0 replies; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-17 13:29 UTC (permalink / raw)
  To: Jose Marinho, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

Am 17. Dezember 2021 13:55:06 MEZ schrieb Jose Marinho <jose.marinho@arm.com>:
>Signed-off-by: Jose Marinho <jose.marinho@arm.com>

Please, provide a commit message.

Best regards

Heinrich

>---
> cmd/efidebug.c       | 41 +++++++++++++++++++++++++++++++++++++++++
> include/efi_loader.h |  2 ++
> 2 files changed, 43 insertions(+)
>
>diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>index a53a5029fa..c3246e1820 100644
>--- a/cmd/efidebug.c
>+++ b/cmd/efidebug.c
>@@ -889,6 +889,38 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
> 	return CMD_RET_SUCCESS;
> }
> 
>+#ifdef CONFIG_EFI_ECPT
>+static int do_efi_ecpt(struct cmd_tbl *cmdtp, int flag,
>+		       int argc, char * const argv[])
>+{
>+	struct efi_conformance_profiles_table *ecpt;
>+
>+	if (argc != 1)
>+		return CMD_RET_USAGE;
>+
>+	for (int idx = 0; idx < systab.nr_tables; idx++)
>+		if (!guidcmp(&efi_ecpt_guid, &systab.tables[idx].guid))
>+			ecpt = (struct efi_system_resource_table *)systab.tables[idx].table;
>+
>+	if (!ecpt) {
>+		log_info("ECPT: table not present\n");
>+		return CMD_RET_SUCCESS;
>+	}
>+
>+	const int num_profiles = ecpt->number_of_profiles;
>+
>+	printf("========================================\n");
>+	printf("ECPT: version:%d\n", ecpt->version);
>+	printf("ECPT: num profiles:%d\n", num_profiles);
>+
>+	for (int i = 0; i < num_profiles; i++)
>+		printf("ECPT: profile %d = %pUL\n", i, &ecpt->conformance_profiles[i]);
>+	printf("========================================\n");
>+
>+	return CMD_RET_SUCCESS;
>+}
>+#endif /* CONFIG_EFI_ECPT */
>+
> /**
>  * create_initrd_dp() - Create a special device for our Boot### option
>  *
>@@ -1681,6 +1713,11 @@ static struct cmd_tbl cmd_efidebug_sub[] = {
> 			 "", ""),
> 	U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
> 			 "", ""),
>+#ifdef CONFIG_EFI_ECPT
>+	U_BOOT_CMD_MKENT(ecpt, CONFIG_SYS_MAXARGS, 1, do_efi_ecpt,
>+			 "", ""),
>+#endif
>+
> };
> 
> /**
>@@ -1769,6 +1806,10 @@ static char efidebug_help_text[] =
> 	"  - show UEFI memory map\n"
> 	"efidebug tables\n"
> 	"  - show UEFI configuration tables\n"
>+#ifdef CONFIG_EFI_ECPT
>+	"efidebug ecpt\n"
>+	"  - show UEFI conformance profiles table\n"
>+#endif
> #ifdef CONFIG_CMD_BOOTEFI_BOOTMGR
> 	"efidebug test bootmgr\n"
> 	"  - run simple bootmgr for test\n"
>diff --git a/include/efi_loader.h b/include/efi_loader.h
>index d20ff396d0..d60a340136 100644
>--- a/include/efi_loader.h
>+++ b/include/efi_loader.h
>@@ -310,6 +310,8 @@ extern const efi_guid_t efi_guid_firmware_management_protocol;
> extern const efi_guid_t efi_esrt_guid;
> /* GUID of the SMBIOS table */
> extern const efi_guid_t smbios_guid;
>+/* GUID for the ECPT */
>+extern const efi_guid_t efi_ecpt_guid;
> 
> extern char __efi_runtime_start[], __efi_runtime_stop[];
> extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] efi: Create ECPT table
  2021-12-17 12:55 ` [PATCH 1/3] efi: Create ECPT table Jose Marinho
@ 2021-12-17 17:19   ` Heinrich Schuchardt
  2021-12-23 15:01     ` Jose Marinho
  0 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-17 17:19 UTC (permalink / raw)
  To: Jose Marinho, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

On 12/17/21 13:55, Jose Marinho wrote:
> The ECPT table will be included in the UEFI specification 2.9+.
> The ECPT table was introduced in UEFI following the code-first path. The
> acceptance ticket can be viewed at:
> 	https://bugzilla.tianocore.org/show_bug.cgi?id=3591
>
> The Conformance Profiles table is a UEFI configuration table that contains
> GUID of the UEFI profiles that the UEFI implementation conforms with.
>
> The ECPT table is created when CONFIG_EFI_ECPT=y.
> The config is set by default.
>
> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> ---
>   cmd/efidebug.c                   |  4 ++
>   include/efi_api.h                | 10 +++++
>   include/efi_loader.h             |  7 ++++
>   lib/efi_loader/Kconfig           |  6 +++
>   lib/efi_loader/Makefile          |  1 +
>   lib/efi_loader/efi_conformance.c | 66 ++++++++++++++++++++++++++++++++
>   lib/efi_loader/efi_setup.c       |  6 +++
>   7 files changed, 100 insertions(+)
>   create mode 100644 lib/efi_loader/efi_conformance.c
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index a977ca9c72..a53a5029fa 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -619,6 +619,10 @@ static const struct {
>   		"TCG2 Final Events Table",
>   		EFI_TCG2_FINAL_EVENTS_TABLE_GUID,
>   	},
> +	{
> +		"EFI Conformance Profiles Table",
> +		EFI_CONFORMANCE_PROFILES_TABLE_GUID,

Just as side note. Consider sending a patch for GRUB to amend:

grub-core/commands/efi/lsefisystab.c
include/grub/efi/api.h

> +	},
>   };
>
>   /**
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 80109f012b..6fd4f04de3 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -226,6 +226,16 @@ enum efi_reset_type {
>   	EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \
>   		 0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)
>
> +#define EFI_CONFORMANCE_PROFILES_TABLE_GUID \
> +	EFI_GUID(0x36122546, 0xf7ef, 0x4c8f, 0xbd, 0x9b, \
> +		 0xeb, 0x85, 0x25, 0xb5, 0x0c, 0x0b)
> +
> +struct efi_conformance_profiles_table {
> +	u16 version;
> +	u16 number_of_profiles;
> +	efi_guid_t	conformance_profiles[];
> +} __packed;
> +
>   struct efi_capsule_header {
>   	efi_guid_t capsule_guid;
>   	u32 header_size;
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index d52e399841..d20ff396d0 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -976,6 +976,13 @@ efi_status_t efi_capsule_authenticate(const void *capsule,
>    */
>   efi_status_t efi_esrt_register(void);
>
> +/**
> + * efi_ecpt_register() - Install the ECPT system table.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_ecpt_register(void);
> +
>   /**
>    * efi_esrt_populate() - Populates the ESRT entries from the FMP instances
>    * present in the system.
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 700dc838dd..b2398976f4 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -367,4 +367,10 @@ config EFI_ESRT
>   	help
>   	  Enabling this option creates the ESRT UEFI system table.
>
> +config EFI_ECPT
> +	bool "Enable the UEFI ECPT generation"
> +	default y
> +	help
> +	  Enabling this option created the ECPT UEFI table.
> +
>   endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index fd344cea29..9f5a0cebd1 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
>   obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
>   obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
>   obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o
> +obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
>
>   EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
>   $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
> diff --git a/lib/efi_loader/efi_conformance.c b/lib/efi_loader/efi_conformance.c
> new file mode 100644
> index 0000000000..86c26d6b79
> --- /dev/null
> +++ b/lib/efi_loader/efi_conformance.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *  EFI conformance profile table
> + *
> + *  Copyright (C) 2022 Arm Ltd.
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <log.h>
> +#include <efi_api.h>
> +#include <malloc.h>
> +
> +const efi_guid_t efi_ecpt_guid = EFI_CONFORMANCE_PROFILES_TABLE_GUID;
> +
> +#define EFI_CONFORMANCE_PROFILES_TABLE_VERSION 1
> +
> +/**
> + * efi_ecpt_register() - Install the ECPT system table.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_ecpt_register(void)
> +{
> +	int num_entries = 0;

Shouldn't we add EFI_CONFORMANCE_PROFILES_EBBR_2_0_GUID as entry?
What would be the point of adding an empty ECPT table?

> +	struct efi_conformance_profiles_table *ecpt;
> +	efi_status_t ret;
> +	size_t ecpt_size = 0;
> +
> +	EFI_PRINT("ECPT table creation start\n");

This function is not called via the EFI API. Please, use log_debug().

Best regards

Heinrich

> +
> +	ecpt_size = num_entries * sizeof(efi_guid_t)
> +		+ sizeof(struct efi_conformance_profiles_table);
> +	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, ecpt_size,
> +				(void **)&ecpt);
> +
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ECPT cannot allocate memory for %u entries (%zu bytes)\n",
> +			  num_entries, ecpt_size);
> +
> +		return ret;
> +	}
> +
> +	ecpt->version = EFI_CONFORMANCE_PROFILES_TABLE_VERSION;
> +	ecpt->number_of_profiles = num_entries;
> +
> +	if (num_entries)
> +		EFI_PRINT("ECPT check conformance profiles, not all entries populated in table\n");
> +
> +	/* Install the ECPT in the system configuration table. */
> +	ret = efi_install_configuration_table(&efi_ecpt_guid, (void *)ecpt);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ECPT failed to install the ECPT in the system table\n");
> +		goto error;
> +	}
> +
> +	EFI_PRINT("ECPT table successfully created\n");
> +
> +	return ret;
> +
> +error:
> +
> +	ret = efi_free_pool(ecpt);
> +
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 1aba71cd96..fa5ad13500 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -231,6 +231,12 @@ efi_status_t efi_init_obj_list(void)
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>
> +	if (IS_ENABLED(CONFIG_EFI_ECPT)) {
> +		ret = efi_ecpt_register();
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
>   	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
>   		ret = efi_esrt_register();
>   		if (ret != EFI_SUCCESS)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile
  2021-12-17 12:55 ` [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile Jose Marinho
@ 2021-12-17 17:26   ` Heinrich Schuchardt
  2021-12-23 14:57     ` Jose Marinho
  0 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-17 17:26 UTC (permalink / raw)
  To: Jose Marinho, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

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

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] Conformance Profiles Table support in U-boot
  2021-12-17 12:55 [PATCH 0/3] Conformance Profiles Table support in U-boot Jose Marinho
                   ` (2 preceding siblings ...)
  2021-12-17 12:55 ` [PATCH 3/3] cmd: efi: efidebug print ECPT table Jose Marinho
@ 2021-12-17 18:05 ` Heinrich Schuchardt
  2021-12-23 15:20   ` Jose Marinho
  3 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-17 18:05 UTC (permalink / raw)
  To: Jose Marinho
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd,
	Samer El-Haj-Mahmoud, Grant Likely, u-boot

On 12/17/21 13:55, Jose Marinho wrote:
> The Conformance Profiles Table (ECPT) table will be included in the UEFI
> specification 2.9+.

The change suggested in
https://bugzilla.tianocore.org/show_bug.cgi?id=3591
is a not well designed: How could the missing of a table ever be taken
as a sign of compliance?

How would an application make use of the table?
What information does it provide that is not better obtained from API calls?

As the table is not defined in UEFI 2.9 and no software uses it, why
should we implement it?

Best regards

Heinrich

> The ECPT table was introduced in UEFI following the code-first path. The
> acceptance ticket can be viewed at:
> 	https://bugzilla.tianocore.org/show_bug.cgi?id=3591
>
> This patch set implements the ECPT table in U-boot.
>
> Jose Marinho (3):
>    efi: Create ECPT table
>    efi: ECPT add EBBRv2.0 conformance profile
>    cmd: efi: efidebug print ECPT table
>
>   cmd/efidebug.c                   | 45 +++++++++++++++++++
>   include/efi_api.h                | 14 ++++++
>   include/efi_loader.h             |  9 ++++
>   lib/efi_loader/Kconfig           | 12 +++++
>   lib/efi_loader/Makefile          |  1 +
>   lib/efi_loader/efi_conformance.c | 75 ++++++++++++++++++++++++++++++++
>   lib/efi_loader/efi_setup.c       |  6 +++
>   7 files changed, 162 insertions(+)
>   create mode 100644 lib/efi_loader/efi_conformance.c
>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] cmd: efi: efidebug print ECPT table
  2021-12-17 12:55 ` [PATCH 3/3] cmd: efi: efidebug print ECPT table Jose Marinho
  2021-12-17 13:29   ` Heinrich Schuchardt
@ 2021-12-17 18:07   ` Heinrich Schuchardt
  2021-12-23 14:59     ` Jose Marinho
  1 sibling, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-17 18:07 UTC (permalink / raw)
  To: Jose Marinho, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

On 12/17/21 13:55, Jose Marinho wrote:
> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> ---
>   cmd/efidebug.c       | 41 +++++++++++++++++++++++++++++++++++++++++
>   include/efi_loader.h |  2 ++
>   2 files changed, 43 insertions(+)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index a53a5029fa..c3246e1820 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -889,6 +889,38 @@ static int do_efi_show_tables(struct cmd_tbl *cmdtp, int flag,
>   	return CMD_RET_SUCCESS;
>   }
>
> +#ifdef CONFIG_EFI_ECPT
> +static int do_efi_ecpt(struct cmd_tbl *cmdtp, int flag,
> +		       int argc, char * const argv[])
> +{
> +	struct efi_conformance_profiles_table *ecpt;
> +
> +	if (argc != 1)
> +		return CMD_RET_USAGE;
> +
> +	for (int idx = 0; idx < systab.nr_tables; idx++)
> +		if (!guidcmp(&efi_ecpt_guid, &systab.tables[idx].guid))
> +			ecpt = (struct efi_system_resource_table *)systab.tables[idx].table;
> +
> +	if (!ecpt) {
> +		log_info("ECPT: table not present\n");
> +		return CMD_RET_SUCCESS;
> +	}
> +
> +	const int num_profiles = ecpt->number_of_profiles;
> +
> +	printf("========================================\n");
> +	printf("ECPT: version:%d\n", ecpt->version);
> +	printf("ECPT: num profiles:%d\n", num_profiles);
> +
> +	for (int i = 0; i < num_profiles; i++)
> +		printf("ECPT: profile %d = %pUL\n", i, &ecpt->conformance_profiles[i]);
> +	printf("========================================\n");
> +
> +	return CMD_RET_SUCCESS;
> +}
> +#endif /* CONFIG_EFI_ECPT */
> +
>   /**
>    * create_initrd_dp() - Create a special device for our Boot### option
>    *
> @@ -1681,6 +1713,11 @@ static struct cmd_tbl cmd_efidebug_sub[] = {
>   			 "", ""),
>   	U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
>   			 "", ""),
> +#ifdef CONFIG_EFI_ECPT
> +	U_BOOT_CMD_MKENT(ecpt, CONFIG_SYS_MAXARGS, 1, do_efi_ecpt,
> +			 "", ""),
> +#endif
> +
>   };
>
>   /**
> @@ -1769,6 +1806,10 @@ static char efidebug_help_text[] =
>   	"  - show UEFI memory map\n"
>   	"efidebug tables\n"
>   	"  - show UEFI configuration tables\n"
> +#ifdef CONFIG_EFI_ECPT
> +	"efidebug ecpt\n"
> +	"  - show UEFI conformance profiles table\n"
> +#endif
>   #ifdef CONFIG_CMD_BOOTEFI_BOOTMGR
>   	"efidebug test bootmgr\n"
>   	"  - run simple bootmgr for test\n"
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index d20ff396d0..d60a340136 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -310,6 +310,8 @@ extern const efi_guid_t efi_guid_firmware_management_protocol;
>   extern const efi_guid_t efi_esrt_guid;
>   /* GUID of the SMBIOS table */
>   extern const efi_guid_t smbios_guid;
> +/* GUID for the ECPT */
> +extern const efi_guid_t efi_ecpt_guid;
>
>   extern char __efi_runtime_start[], __efi_runtime_stop[];
>   extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];

Our interest is to keep the U-Boot binary size small. I see no need to
print the ECPT table.

What make more sense is a unit test that checks the consistency of the
table.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile
  2021-12-17 17:26   ` Heinrich Schuchardt
@ 2021-12-23 14:57     ` Jose Marinho
  2021-12-23 18:31       ` Heinrich Schuchardt
  0 siblings, 1 reply; 18+ messages in thread
From: Jose Marinho @ 2021-12-23 14:57 UTC (permalink / raw)
  To: Heinrich Schuchardt, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 3/3] cmd: efi: efidebug print ECPT table
  2021-12-17 18:07   ` Heinrich Schuchardt
@ 2021-12-23 14:59     ` Jose Marinho
  0 siblings, 0 replies; 18+ messages in thread
From: Jose Marinho @ 2021-12-23 14:59 UTC (permalink / raw)
  To: Heinrich Schuchardt, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

Hi Heinrich,

I dropped the ECPT print in efidebug in PATCH v2. Additionally I've introduced an efi selftet.

Regards,

Jose

> -----Original Message-----
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Sent: 17 December 2021 18:07
> 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 3/3] cmd: efi: efidebug print ECPT table
> 
> On 12/17/21 13:55, Jose Marinho wrote:
> > Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> > ---
> >   cmd/efidebug.c       | 41
> +++++++++++++++++++++++++++++++++++++++++
> >   include/efi_loader.h |  2 ++
> >   2 files changed, 43 insertions(+)
> >
> > diff --git a/cmd/efidebug.c b/cmd/efidebug.c index
> > a53a5029fa..c3246e1820 100644
> > --- a/cmd/efidebug.c
> > +++ b/cmd/efidebug.c
> > @@ -889,6 +889,38 @@ static int do_efi_show_tables(struct cmd_tbl
> *cmdtp, int flag,
> >   	return CMD_RET_SUCCESS;
> >   }
> >
> > +#ifdef CONFIG_EFI_ECPT
> > +static int do_efi_ecpt(struct cmd_tbl *cmdtp, int flag,
> > +		       int argc, char * const argv[]) {
> > +	struct efi_conformance_profiles_table *ecpt;
> > +
> > +	if (argc != 1)
> > +		return CMD_RET_USAGE;
> > +
> > +	for (int idx = 0; idx < systab.nr_tables; idx++)
> > +		if (!guidcmp(&efi_ecpt_guid, &systab.tables[idx].guid))
> > +			ecpt = (struct efi_system_resource_table
> > +*)systab.tables[idx].table;
> > +
> > +	if (!ecpt) {
> > +		log_info("ECPT: table not present\n");
> > +		return CMD_RET_SUCCESS;
> > +	}
> > +
> > +	const int num_profiles = ecpt->number_of_profiles;
> > +
> > +	printf("========================================\n");
> > +	printf("ECPT: version:%d\n", ecpt->version);
> > +	printf("ECPT: num profiles:%d\n", num_profiles);
> > +
> > +	for (int i = 0; i < num_profiles; i++)
> > +		printf("ECPT: profile %d = %pUL\n", i, &ecpt-
> >conformance_profiles[i]);
> > +	printf("========================================\n");
> > +
> > +	return CMD_RET_SUCCESS;
> > +}
> > +#endif /* CONFIG_EFI_ECPT */
> > +
> >   /**
> >    * create_initrd_dp() - Create a special device for our Boot### option
> >    *
> > @@ -1681,6 +1713,11 @@ static struct cmd_tbl cmd_efidebug_sub[] = {
> >   			 "", ""),
> >   	U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1,
> do_efi_query_info,
> >   			 "", ""),
> > +#ifdef CONFIG_EFI_ECPT
> > +	U_BOOT_CMD_MKENT(ecpt, CONFIG_SYS_MAXARGS, 1,
> do_efi_ecpt,
> > +			 "", ""),
> > +#endif
> > +
> >   };
> >
> >   /**
> > @@ -1769,6 +1806,10 @@ static char efidebug_help_text[] =
> >   	"  - show UEFI memory map\n"
> >   	"efidebug tables\n"
> >   	"  - show UEFI configuration tables\n"
> > +#ifdef CONFIG_EFI_ECPT
> > +	"efidebug ecpt\n"
> > +	"  - show UEFI conformance profiles table\n"
> > +#endif
> >   #ifdef CONFIG_CMD_BOOTEFI_BOOTMGR
> >   	"efidebug test bootmgr\n"
> >   	"  - run simple bootmgr for test\n"
> > diff --git a/include/efi_loader.h b/include/efi_loader.h index
> > d20ff396d0..d60a340136 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -310,6 +310,8 @@ extern const efi_guid_t
> efi_guid_firmware_management_protocol;
> >   extern const efi_guid_t efi_esrt_guid;
> >   /* GUID of the SMBIOS table */
> >   extern const efi_guid_t smbios_guid;
> > +/* GUID for the ECPT */
> > +extern const efi_guid_t efi_ecpt_guid;
> >
> >   extern char __efi_runtime_start[], __efi_runtime_stop[];
> >   extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];
> 
> Our interest is to keep the U-Boot binary size small. I see no need to print the
> ECPT table.
> 
> What make more sense is a unit test that checks the consistency of the table.
> 
> Best regards
> 
> Heinrich

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 1/3] efi: Create ECPT table
  2021-12-17 17:19   ` Heinrich Schuchardt
@ 2021-12-23 15:01     ` Jose Marinho
  0 siblings, 0 replies; 18+ messages in thread
From: Jose Marinho @ 2021-12-23 15:01 UTC (permalink / raw)
  To: Heinrich Schuchardt, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

Hi Heinrich,

> -----Original Message-----
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Sent: 17 December 2021 17:20
> 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 1/3] efi: Create ECPT table
> 
> On 12/17/21 13:55, Jose Marinho wrote:
> > The ECPT table will be included in the UEFI specification 2.9+.
> > The ECPT table was introduced in UEFI following the code-first path.
> > The acceptance ticket can be viewed at:
> > 	https://bugzilla.tianocore.org/show_bug.cgi?id=3591
> >
> > The Conformance Profiles table is a UEFI configuration table that
> > contains GUID of the UEFI profiles that the UEFI implementation conforms
> with.
> >
> > The ECPT table is created when CONFIG_EFI_ECPT=y.
> > The config is set by default.
> >
> > Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> > ---
> >   cmd/efidebug.c                   |  4 ++
> >   include/efi_api.h                | 10 +++++
> >   include/efi_loader.h             |  7 ++++
> >   lib/efi_loader/Kconfig           |  6 +++
> >   lib/efi_loader/Makefile          |  1 +
> >   lib/efi_loader/efi_conformance.c | 66
> ++++++++++++++++++++++++++++++++
> >   lib/efi_loader/efi_setup.c       |  6 +++
> >   7 files changed, 100 insertions(+)
> >   create mode 100644 lib/efi_loader/efi_conformance.c
> >
> > diff --git a/cmd/efidebug.c b/cmd/efidebug.c index
> > a977ca9c72..a53a5029fa 100644
> > --- a/cmd/efidebug.c
> > +++ b/cmd/efidebug.c
> > @@ -619,6 +619,10 @@ static const struct {
> >   		"TCG2 Final Events Table",
> >   		EFI_TCG2_FINAL_EVENTS_TABLE_GUID,
> >   	},
> > +	{
> > +		"EFI Conformance Profiles Table",
> > +		EFI_CONFORMANCE_PROFILES_TABLE_GUID,
> 
> Just as side note. Consider sending a patch for GRUB to amend:
> 
> grub-core/commands/efi/lsefisystab.c
> include/grub/efi/api.h
> 
> > +	},
> >   };
> >
> >   /**
> > diff --git a/include/efi_api.h b/include/efi_api.h index
> > 80109f012b..6fd4f04de3 100644
> > --- a/include/efi_api.h
> > +++ b/include/efi_api.h
> > @@ -226,6 +226,16 @@ enum efi_reset_type {
> >   	EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \
> >   		 0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)
> >
> > +#define EFI_CONFORMANCE_PROFILES_TABLE_GUID \
> > +	EFI_GUID(0x36122546, 0xf7ef, 0x4c8f, 0xbd, 0x9b, \
> > +		 0xeb, 0x85, 0x25, 0xb5, 0x0c, 0x0b)
> > +
> > +struct efi_conformance_profiles_table {
> > +	u16 version;
> > +	u16 number_of_profiles;
> > +	efi_guid_t	conformance_profiles[];
> > +} __packed;
> > +
> >   struct efi_capsule_header {
> >   	efi_guid_t capsule_guid;
> >   	u32 header_size;
> > diff --git a/include/efi_loader.h b/include/efi_loader.h index
> > d52e399841..d20ff396d0 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -976,6 +976,13 @@ efi_status_t efi_capsule_authenticate(const void
> *capsule,
> >    */
> >   efi_status_t efi_esrt_register(void);
> >
> > +/**
> > + * efi_ecpt_register() - Install the ECPT system table.
> > + *
> > + * Return: status code
> > + */
> > +efi_status_t efi_ecpt_register(void);
> > +
> >   /**
> >    * efi_esrt_populate() - Populates the ESRT entries from the FMP
> instances
> >    * present in the system.
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index
> > 700dc838dd..b2398976f4 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -367,4 +367,10 @@ config EFI_ESRT
> >   	help
> >   	  Enabling this option creates the ESRT UEFI system table.
> >
> > +config EFI_ECPT
> > +	bool "Enable the UEFI ECPT generation"
> > +	default y
> > +	help
> > +	  Enabling this option created the ECPT UEFI table.
> > +
> >   endif
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index
> > fd344cea29..9f5a0cebd1 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -64,6 +64,7 @@ obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> >   obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
> >   obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> >   obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o
> > +obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
> >
> >   EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> >   $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE) diff --git
> > a/lib/efi_loader/efi_conformance.c b/lib/efi_loader/efi_conformance.c
> > new file mode 100644
> > index 0000000000..86c26d6b79
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_conformance.c
> > @@ -0,0 +1,66 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + *  EFI conformance profile table
> > + *
> > + *  Copyright (C) 2022 Arm Ltd.
> > + */
> > +
> > +#include <common.h>
> > +#include <efi_loader.h>
> > +#include <log.h>
> > +#include <efi_api.h>
> > +#include <malloc.h>
> > +
> > +const efi_guid_t efi_ecpt_guid =
> EFI_CONFORMANCE_PROFILES_TABLE_GUID;
> > +
> > +#define EFI_CONFORMANCE_PROFILES_TABLE_VERSION 1
> > +
> > +/**
> > + * efi_ecpt_register() - Install the ECPT system table.
> > + *
> > + * Return: status code
> > + */
> > +efi_status_t efi_ecpt_register(void)
> > +{
> > +	int num_entries = 0;
> 
> Shouldn't we add EFI_CONFORMANCE_PROFILES_EBBR_2_0_GUID as entry?
> What would be the point of adding an empty ECPT table?[Jose] 

The EEBRv2.0 conformance profile is introduced in the following commit of this patch set.
Should we squash the two commits?

> 
> > +	struct efi_conformance_profiles_table *ecpt;
> > +	efi_status_t ret;
> > +	size_t ecpt_size = 0;
> > +
> > +	EFI_PRINT("ECPT table creation start\n");
> 
> This function is not called via the EFI API. Please, use log_debug().
> 
Done

Regards,

Jose

> Best regards
> 
> Heinrich
> 
> > +
> > +	ecpt_size = num_entries * sizeof(efi_guid_t)
> > +		+ sizeof(struct efi_conformance_profiles_table);
> > +	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, ecpt_size,
> > +				(void **)&ecpt);
> > +
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT("ECPT cannot allocate memory for %u entries
> (%zu bytes)\n",
> > +			  num_entries, ecpt_size);
> > +
> > +		return ret;
> > +	}
> > +
> > +	ecpt->version = EFI_CONFORMANCE_PROFILES_TABLE_VERSION;
> > +	ecpt->number_of_profiles = num_entries;
> > +
> > +	if (num_entries)
> > +		EFI_PRINT("ECPT check conformance profiles, not all entries
> > +populated in table\n");
> > +
> > +	/* Install the ECPT in the system configuration table. */
> > +	ret = efi_install_configuration_table(&efi_ecpt_guid, (void *)ecpt);
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT("ECPT failed to install the ECPT in the system
> table\n");
> > +		goto error;
> > +	}
> > +
> > +	EFI_PRINT("ECPT table successfully created\n");
> > +
> > +	return ret;
> > +
> > +error:
> > +
> > +	ret = efi_free_pool(ecpt);
> > +
> > +	return ret;
> > +}
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 1aba71cd96..fa5ad13500 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -231,6 +231,12 @@ efi_status_t efi_init_obj_list(void)
> >   	if (ret != EFI_SUCCESS)
> >   		goto out;
> >
> > +	if (IS_ENABLED(CONFIG_EFI_ECPT)) {
> > +		ret = efi_ecpt_register();
> > +		if (ret != EFI_SUCCESS)
> > +			goto out;
> > +	}
> > +
> >   	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> >   		ret = efi_esrt_register();
> >   		if (ret != EFI_SUCCESS)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 0/3] Conformance Profiles Table support in U-boot
  2021-12-17 18:05 ` [PATCH 0/3] Conformance Profiles Table support in U-boot Heinrich Schuchardt
@ 2021-12-23 15:20   ` Jose Marinho
  2021-12-23 18:27     ` Heinrich Schuchardt
  0 siblings, 1 reply; 18+ messages in thread
From: Jose Marinho @ 2021-12-23 15:20 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd,
	Samer El-Haj-Mahmoud, Grant Likely, u-boot

Hi Heinrich,

> 
> The change suggested in
> https://bugzilla.tianocore.org/show_bug.cgi?id=3591
> is a not well designed: How could the missing of a table ever be taken as a
> sign of compliance?
> 
Below is my interpretation of intent behind the ECPT changes:
The UEFI spec specifies a set of requirements for UEFI compliance in Section 2.6.
Any complete UEFI implementation must adhere to the requirements in Section 2.6.

The Conformance Profiles allow for "partial" UEFI implementations, which implement a subset of the full UEFI requirements.
The conformance with a particular profile should be explicitly advertised (via the conformance profiles table). If not, then full compliance with the Section 2.6 requirements is assumed.

> How would an application make use of the table?
> What information does it provide that is not better obtained from API calls?
> 
With the ECPT, an application can easily detect the UEFI profile that's implemented and hence adopt a model of execution that suits that profile.
Alternatively the application could probe the different RT services, UEFI variables, etc to "detect" a profile. The table allows for a more straightforward or simpler detection. 

> As the table is not defined in UEFI 2.9 and no software uses it, why should we
> implement it?
The code first UEFI ECR was accepted. Once a UEFI ECR is accepted it becomes part of the UEFI specification.

Regards,

Jose

> Best regards
> 
> Heinrich
> 
> > The ECPT table was introduced in UEFI following the code-first path.
> > The acceptance ticket can be viewed at:
> > 	https://bugzilla.tianocore.org/show_bug.cgi?id=3591
> >
> > This patch set implements the ECPT table in U-boot.
> >
> > Jose Marinho (3):
> >    efi: Create ECPT table
> >    efi: ECPT add EBBRv2.0 conformance profile
> >    cmd: efi: efidebug print ECPT table
> >
> >   cmd/efidebug.c                   | 45 +++++++++++++++++++
> >   include/efi_api.h                | 14 ++++++
> >   include/efi_loader.h             |  9 ++++
> >   lib/efi_loader/Kconfig           | 12 +++++
> >   lib/efi_loader/Makefile          |  1 +
> >   lib/efi_loader/efi_conformance.c | 75
> ++++++++++++++++++++++++++++++++
> >   lib/efi_loader/efi_setup.c       |  6 +++
> >   7 files changed, 162 insertions(+)
> >   create mode 100644 lib/efi_loader/efi_conformance.c
> >


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] Conformance Profiles Table support in U-boot
  2021-12-23 15:20   ` Jose Marinho
@ 2021-12-23 18:27     ` Heinrich Schuchardt
  0 siblings, 0 replies; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-23 18:27 UTC (permalink / raw)
  To: Jose Marinho
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd,
	Samer El-Haj-Mahmoud, Grant Likely, u-boot

On 12/23/21 16:20, Jose Marinho wrote:
> Hi Heinrich,
>
>>
>> The change suggested in
>> https://bugzilla.tianocore.org/show_bug.cgi?id=3591
>> is a not well designed: How could the missing of a table ever be taken as a
>> sign of compliance?
>>
> Below is my interpretation of intent behind the ECPT changes:
> The UEFI spec specifies a set of requirements for UEFI compliance in Section 2.6.
> Any complete UEFI implementation must adhere to the requirements in Section 2.6.
>
> The Conformance Profiles allow for "partial" UEFI implementations, which implement a subset of the full UEFI requirements.
> The conformance with a particular profile should be explicitly advertised (via the conformance profiles table). If not, then full compliance with the Section 2.6 requirements is assumed.
>
>> How would an application make use of the table?
>> What information does it provide that is not better obtained from API calls?
>>
> With the ECPT, an application can easily detect the UEFI profile that's implemented and hence adopt a model of execution that suits that profile.
> Alternatively the application could probe the different RT services, UEFI variables, etc to "detect" a profile. The table allows for a more straightforward or simpler detection.

U-Boot in many cases will not implement any of the profiles in full.
E.g. GetTime() might be missing if there is no RTC. The table will not
be very helpful as it will be empty in these cases. Why should we blow
up the U-Boot code size with this - most of the time - useless table?

>
>> As the table is not defined in UEFI 2.9 and no software uses it, why should we
>> implement it?
> The code first UEFI ECR was accepted. Once a UEFI ECR is accepted it becomes part of the UEFI specification.

Code first? https://bugzilla.tianocore.org/show_bug.cgi?id=3591 does not
indicate any implementation of code using or providing the table.

The way the new table was defined does not make sense. It is assumed
that if the table is missing the UEFI implementation is compliant to the
complete UEFI spec. This aspect needs to be fixed. It is not too late to
do so.

This is the paragraph that needs to change:

"The absence of this table shall indicate that the platform
implementation is conformant with the UEFI specification requirements,
as defined in section 2.6. This is equivalent to publishing this
configuration table with the EFI_CONFORMANCE_PROFILES_UEFI_SPEC_GUID
conformance profile."

Best regards

Heinrich

>
> Regards,
>
> Jose
>
>> Best regards
>>
>> Heinrich
>>
>>> The ECPT table was introduced in UEFI following the code-first path.
>>> The acceptance ticket can be viewed at:
>>> 	https://bugzilla.tianocore.org/show_bug.cgi?id=3591
>>>
>>> This patch set implements the ECPT table in U-boot.
>>>
>>> Jose Marinho (3):
>>>     efi: Create ECPT table
>>>     efi: ECPT add EBBRv2.0 conformance profile
>>>     cmd: efi: efidebug print ECPT table
>>>
>>>    cmd/efidebug.c                   | 45 +++++++++++++++++++
>>>    include/efi_api.h                | 14 ++++++
>>>    include/efi_loader.h             |  9 ++++
>>>    lib/efi_loader/Kconfig           | 12 +++++
>>>    lib/efi_loader/Makefile          |  1 +
>>>    lib/efi_loader/efi_conformance.c | 75
>> ++++++++++++++++++++++++++++++++
>>>    lib/efi_loader/efi_setup.c       |  6 +++
>>>    7 files changed, 162 insertions(+)
>>>    create mode 100644 lib/efi_loader/efi_conformance.c
>>>
>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile
  2021-12-23 14:57     ` Jose Marinho
@ 2021-12-23 18:31       ` Heinrich Schuchardt
  2021-12-31 14:36         ` Jose Marinho
  0 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-23 18:31 UTC (permalink / raw)
  To: Jose Marinho, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile
  2021-12-23 18:31       ` Heinrich Schuchardt
@ 2021-12-31 14:36         ` Jose Marinho
  2022-01-01 23:56           ` Heinrich Schuchardt
  0 siblings, 1 reply; 18+ messages in thread
From: Jose Marinho @ 2021-12-31 14:36 UTC (permalink / raw)
  To: Heinrich Schuchardt, u-boot
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd

Hi Heinrich,

> -----Original Message-----
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Sent: 23 December 2021 18:32
> 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/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.
> 
The EBBR 2.0 conformance GUID can be an entry in the ECPT when the FW is EBBR v2.0 compliant.
The FW compliance with EBBRv2.0 can be determined by running the EBBR ACS (obtainable from https://github.com/ARM-software/bbr-acs).
Should we state this in the commit message, or perhaps as a comment in Config definition? In any case, the GUID inclusion criteria in the ECPT is not ambiguous. 

If we were to determine the EBBR2.0 GUID inclusion as a function of other u-boot configs, we'd potentially unnecessarily complicate the ECPT implementation in u-boot and also generate a maintenance burden as the codebase evolves. 

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile
  2021-12-31 14:36         ` Jose Marinho
@ 2022-01-01 23:56           ` Heinrich Schuchardt
  2022-08-29 23:58             ` Heinrich Schuchardt
  0 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2022-01-01 23:56 UTC (permalink / raw)
  To: Jose Marinho
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd, u-boot



On 12/31/21 15:36, Jose Marinho wrote:
> Hi Heinrich,
>
>> -----Original Message-----
>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Sent: 23 December 2021 18:32
>> 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/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.
>>
> The EBBR 2.0 conformance GUID can be an entry in the ECPT when the FW is EBBR v2.0 compliant.
> The FW compliance with EBBRv2.0 can be determined by running the EBBR ACS (obtainable from https://github.com/ARM-software/bbr-acs).
> Should we state this in the commit message, or perhaps as a comment in Config definition? In any case, the GUID inclusion criteria in the ECPT is not ambiguous.
>
> If we were to determine the EBBR2.0 GUID inclusion as a function of other u-boot configs, we'd potentially unnecessarily complicate the ECPT implementation in u-boot and also generate a maintenance burden as the codebase evolves.

As the implementation of the EFI_HII_DATABASE_PROTOCOL in U-Boot does
not pass the SCT U-Boot does not comply to EBBR 2.0.

I see no use case for the ECPT table.

Best regards

Heinrich

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile
  2022-01-01 23:56           ` Heinrich Schuchardt
@ 2022-08-29 23:58             ` Heinrich Schuchardt
  0 siblings, 0 replies; 18+ messages in thread
From: Heinrich Schuchardt @ 2022-08-29 23:58 UTC (permalink / raw)
  To: Jose Marinho
  Cc: ilias.apalodimas, sughosh.ganu, takahiro.akashi, agraf, nd, u-boot

On 1/2/22 00:56, Heinrich Schuchardt wrote:
>
>
> On 12/31/21 15:36, Jose Marinho wrote:
>> Hi Heinrich,
>>
>>> -----Original Message-----
>>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> Sent: 23 December 2021 18:32
>>> 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/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.
>>>
>> The EBBR 2.0 conformance GUID can be an entry in the ECPT when the FW
>> is EBBR v2.0 compliant.
>> The FW compliance with EBBRv2.0 can be determined by running the EBBR
>> ACS (obtainable from https://github.com/ARM-software/bbr-acs).
>> Should we state this in the commit message, or perhaps as a comment in
>> Config definition? In any case, the GUID inclusion criteria in the
>> ECPT is not ambiguous.
>>
>> If we were to determine the EBBR2.0 GUID inclusion as a function of
>> other u-boot configs, we'd potentially unnecessarily complicate the
>> ECPT implementation in u-boot and also generate a maintenance burden
>> as the codebase evolves.
>
> As the implementation of the EFI_HII_DATABASE_PROTOCOL in U-Boot does
> not pass the SCT U-Boot does not comply to EBBR 2.0.
>
> I see no use case for the ECPT table.

UEFI 2.10 now has defined the EFI_CONFORMANCE_PROFILE_TABLE. But it does
not provide a GUID for EBBR. EBBR does not define one either. I have
created https://github.com/ARM-software/ebbr/issues/95. Once the EBBR
community agrees on a GUID we can add the table.

Best regards

Heinrich

>
> Best regards
>
> Heinrich
>
>>
>>> 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");
>>>>>>
>>>>
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-08-29 23:58 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-17 12:55 [PATCH 0/3] Conformance Profiles Table support in U-boot Jose Marinho
2021-12-17 12:55 ` [PATCH 1/3] efi: Create ECPT table Jose Marinho
2021-12-17 17:19   ` Heinrich Schuchardt
2021-12-23 15:01     ` Jose Marinho
2021-12-17 12:55 ` [PATCH 2/3] efi: ECPT add EBBRv2.0 conformance profile Jose Marinho
2021-12-17 17:26   ` Heinrich Schuchardt
2021-12-23 14:57     ` Jose Marinho
2021-12-23 18:31       ` Heinrich Schuchardt
2021-12-31 14:36         ` Jose Marinho
2022-01-01 23:56           ` Heinrich Schuchardt
2022-08-29 23:58             ` Heinrich Schuchardt
2021-12-17 12:55 ` [PATCH 3/3] cmd: efi: efidebug print ECPT table Jose Marinho
2021-12-17 13:29   ` Heinrich Schuchardt
2021-12-17 18:07   ` Heinrich Schuchardt
2021-12-23 14:59     ` Jose Marinho
2021-12-17 18:05 ` [PATCH 0/3] Conformance Profiles Table support in U-boot Heinrich Schuchardt
2021-12-23 15:20   ` Jose Marinho
2021-12-23 18:27     ` Heinrich Schuchardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.