[PATCH 0/2] Add ESRT and test ESRT creation

[PATCH 0/2] Add ESRT and test ESRT creation

[Jose Marinho]

The following 2 commits add the ESRT and provide a test of the
functionality.

The first commit adds the ESRT as defined in the UEFI 2.8 specification.
An empty ESRT is created during the execution of the efi_init_obj_list().
The ESRT is updated when:
  1) a FMP protocol is installed in the system: this will add the
corresponding entries to the ESRT.
  2) a capsule is installed via UpdateCapsule: this should update
entries already present in the ESRT.

This implementation of the ESRT creation takes input from FMP only.
It is assumed that the FMP will maintain the following values across
reboot:
 - LastAttemptVersion.
 - LastAttemptStatus.


The second commit enables testing the ESRT creation in the sandbox
platfrom. The test executes from the u-boot shell with "ut lib".

Note: I've removed the RFC tag that was previously used to post this
patch.

Patch v1:
- reworked the ESRT creation code, allowing table to resize as
FMPs are installed.
- registered a callback for the FMP protocol install.
- Created a unit test running on the sandbox platform.

rfc: initial patch submission

CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
CC: Sughosh Ganu <sughosh.ganu@linaro.org>
CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
CC: Andre Przywara <andre.przywara@arm.com>
CC: Alexander Graf <agraf@csgraf.de>
CC: nd at arm.com

Jose Marinho (2):
  efi: Add ESRT to the EFI system table
  efi: ESRT cration unit test

 cmd/efidebug.c                |   4 +
 include/efi_api.h             |  21 ++
 include/efi_loader.h          |  19 ++
 lib/efi_loader/Kconfig        |   7 +
 lib/efi_loader/Makefile       |   1 +
 lib/efi_loader/efi_boottime.c |   2 -
 lib/efi_loader/efi_capsule.c  |   7 +
 lib/efi_loader/efi_esrt.c     | 439 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c    |   6 +
 test/lib/Makefile             |   1 +
 test/lib/efi_esrt.c           | 188 +++++++++++++++
 11 files changed, 693 insertions(+), 2 deletions(-)
 create mode 100644 lib/efi_loader/efi_esrt.c
 create mode 100644 test/lib/efi_esrt.c

-- 
2.17.1

[PATCH 1/2] efi: Add ESRT to the EFI system table

[Jose Marinho]

The ESRT is initialised during efi_init_objlist after
efi_initialize_system_table().

The ESRT is initially created with size for 50 FW image entries.
The ESRT is resized when it runs out of space. Every resize adds 50
additional entries.
The ESRT is populated from information provided by FMP instances only.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>

CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
CC: Sughosh Ganu <sughosh.ganu@linaro.org>
CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
CC: Andre Przywara <andre.przywara@arm.com>
CC: Alexander Graf <agraf@csgraf.de>
CC: nd at arm.com

---
 cmd/efidebug.c                |   4 +
 include/efi_api.h             |  21 ++
 include/efi_loader.h          |  19 ++
 lib/efi_loader/Kconfig        |   7 +
 lib/efi_loader/Makefile       |   1 +
 lib/efi_loader/efi_boottime.c |   2 -
 lib/efi_loader/efi_capsule.c  |   7 +
 lib/efi_loader/efi_esrt.c     | 439 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c    |   6 +
 9 files changed, 504 insertions(+), 2 deletions(-)
 create mode 100644 lib/efi_loader/efi_esrt.c

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 83bc2196a5..4160dde1cf 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -458,6 +458,10 @@ static const struct {
 		"Block IO",
 		EFI_BLOCK_IO_PROTOCOL_GUID,
 	},
+	{
+		"EFI System Resource Table",
+		EFI_SYSTEM_RESOURCE_TABLE_GUID,
+	},
 	{
 		"Simple File System",
 		EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
diff --git a/include/efi_api.h b/include/efi_api.h
index 48e48a6263..7eb15bd44c 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -1722,6 +1722,23 @@ struct efi_load_file_protocol {
 					 void *buffer);
 };
 
+struct efi_system_resource_entry {
+	efi_guid_t fw_class;
+	uint32_t fw_type;
+	uint32_t fw_version;
+	uint32_t lowest_supported_fw_version;
+	uint32_t capsule_flags;
+	uint32_t last_attempt_version;
+	uint32_t last_attempt_status;
+} __packed;
+
+struct efi_system_resource_table {
+	uint32_t fw_resource_count;
+	uint32_t fw_resource_count_max;
+	uint64_t fw_resource_version;
+	struct efi_system_resource_entry entries[];
+} __packed;
+
 /* Boot manager load options */
 #define LOAD_OPTION_ACTIVE		0x00000001
 #define LOAD_OPTION_FORCE_RECONNECT	0x00000002
@@ -1740,6 +1757,10 @@ struct efi_load_file_protocol {
 #define ESRT_FW_TYPE_DEVICEFIRMWARE	0x00000002
 #define ESRT_FW_TYPE_UEFIDRIVER		0x00000003
 
+#define EFI_SYSTEM_RESOURCE_TABLE_GUID\
+	EFI_GUID(0xb122a263, 0x3661, 0x4f68,\
+		0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80 )
+
 /* Last Attempt Status Values */
 #define LAST_ATTEMPT_STATUS_SUCCESS			0x00000000
 #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL		0x00000001
diff --git a/include/efi_loader.h b/include/efi_loader.h
index f470bbd636..c85c540041 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -884,4 +884,23 @@ static inline efi_status_t efi_launch_capsules(void)
 
 #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
 
+/*
+ * Install the ESRT system table.
+ *
+ * @return	status code
+ */
+efi_status_t efi_esrt_register(void);
+
+/**
+ * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
+ * images in the FMP.
+ *
+ * @fmp:        the fmp from which fw images are added to the ESRT
+ *
+ * Return:
+ * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
+ * - Error status otherwise
+ */
+efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp);
+
 #endif /* _EFI_LOADER_H */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e729f727df..12b29180a0 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -347,4 +347,11 @@ config EFI_SECURE_BOOT
 	  it is signed with a trusted key. To do that, you need to install,
 	  at least, PK, KEK and db.
 
+config EFI_ESRT
+	bool "Enable the UEFI ESRT generation"
+	depends on EFI_LOADER
+	default y
+	help
+	  Enabling this option creates the ESRT UEFI system table.
+
 endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 10b42e8847..dec791b310 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -52,6 +52,7 @@ obj-y += efi_variable.o
 obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
 endif
 obj-y += efi_watchdog.o
+obj-y += efi_esrt.o
 obj-$(CONFIG_LCD) += efi_gop.o
 obj-$(CONFIG_DM_VIDEO) += efi_gop.o
 obj-$(CONFIG_PARTITIONS) += efi_disk.o
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index ce658a8e73..9b0b15571a 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1148,8 +1148,6 @@ efi_status_t efi_add_protocol(const efi_handle_t handle,
 		}
 	}
 
-	if (!guidcmp(&efi_guid_device_path, protocol))
-		EFI_PRINT("installed device path '%pD'\n", protocol_interface);
 	return EFI_SUCCESS;
 }
 
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 0d5a7b63ec..60703bcdaa 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -404,6 +404,13 @@ static efi_status_t efi_capsule_update_firmware(
 			efi_free_pool(abort_reason);
 			goto out;
 		}
+
+#ifdef CONFIG_EFI_ESRT
+		/* Update the ESRT entries corresponding to fmp. */
+		ret = efi_esrt_add_from_fmp(fmp);
+		if (ret != EFI_SUCCESS)
+			log_warning("EFI Capsule: failed to populate ESRT entry\n");
+#endif
 	}
 
 out:
diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
new file mode 100644
index 0000000000..34133346ca
--- /dev/null
+++ b/lib/efi_loader/efi_esrt.c
@@ -0,0 +1,439 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  EFI application ESRT tables support
+ *
+ *  Copyright (C) 2021 Arm Ltd.
+ */
+
+#include <common.h>
+#include <efi_loader.h>
+#include <log.h>
+#include <efi_api.h>
+#include <malloc.h>
+
+static efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
+
+struct efi_system_resource_table *esrt;
+
+#define EFI_ESRT_MIN_RESIZE_ENTIRES 50
+#define EFI_ESRT_VERSION 1
+
+/**
+ * efi_esrt_image_info_to_entry() - copy the information present in a fw image
+ * descriptor to a ESRT entry.
+ *
+ * @img_info:     the source image info descriptor
+ * @entry:        pointer to the ESRT entry to be filled
+ * @desc_version: the version of the elements in img_info
+ * @image_type:   the image type value to be set in the ESRT entry
+ * @flags:        the capsule flags value to be set in the ESRT entry
+ *
+ */
+void
+efi_esrt_image_info_to_entry(struct efi_firmware_image_descriptor *img_info,
+			     struct efi_system_resource_entry *entry,
+			     u32 desc_version, u32 image_type, u32 flags)
+{
+	guidcpy(&entry->fw_class, &img_info->image_type_id);
+	entry->fw_version = img_info->version;
+
+	entry->fw_type = image_type;
+	entry->capsule_flags = flags;
+
+	/*
+	 * The field lowest_supported_image_version is only present
+	 * on image info structure of version 2 or greater.
+	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
+	 */
+	if (desc_version >= 2) {
+		entry->lowest_supported_fw_version =
+			img_info->lowest_supported_image_version;
+	} else {
+		entry->lowest_supported_fw_version = 0;
+	}
+
+	/*
+	 * The fields last_attempt_version and last_attempt_status
+	 * are only present on image info structure of version 3 or
+	 * greater.
+	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
+	 */
+	if (desc_version >= 3) {
+		entry->last_attempt_version =
+			img_info->last_attempt_version;
+
+		entry->last_attempt_status =
+			img_info->last_attempt_status;
+	} else {
+		entry->last_attempt_version = 0;
+		entry->last_attempt_status = EFI_SUCCESS;
+	}
+}
+
+/**
+ * efi_esrt_entries_to_size() - Obtain the bytes used by an ESRT
+ * datastructure with @num_entries.
+ *
+ * @num_entries: the ESRT which we obtain the filled size in bytes.
+ *
+ * Return: the number of bytes an ESRT with @num_entries occupies in memory.
+ */
+static
+inline u32 efi_esrt_entries_to_size(u32 num_entries)
+{
+	u32 esrt_size = sizeof(struct efi_system_resource_table) +
+		num_entries * sizeof(struct efi_system_resource_entry);
+
+	return esrt_size;
+}
+
+/**
+ * efi_esrt_allocate_install() - Allocates @num_entries for the ESRT and
+ * performs basic ESRT initialization.
+ *
+ * @bt	       : pointer to the boottime services structure.
+ * @num_entries: the number of entries that the ESRT will hold.
+ *
+ * Return:
+ * - pointer to the ESRT if successful.
+ * - NULL otherwise.
+ */
+static
+struct efi_system_resource_table *efi_esrt_allocate_install(struct efi_boot_services *bt,
+							    u32 num_entries)
+{
+	efi_status_t ret;
+	struct efi_system_resource_table *new_esrt;
+	u32 size = efi_esrt_entries_to_size(num_entries);
+
+	/* Allocated pages must be on the lower 32bit address space. */
+	new_esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
+
+	/* Reserve num_pages for ESRT */
+	ret = EFI_CALL(bt->allocate_pool(EFI_RUNTIME_SERVICES_DATA,
+					 size,
+					 (void **)&new_esrt));
+
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT Failed to allocate memory for ESRT with %d entries (%d bytes)\n",
+			  num_entries, efi_esrt_entries_to_size(num_entries));
+
+		return NULL;
+	}
+
+	new_esrt->fw_resource_count_max = num_entries;
+	new_esrt->fw_resource_version = EFI_ESRT_VERSION;
+
+	/* Install the ESRT in the system configuration table. */
+	ret = EFI_CALL(bt->install_configuration_table(&esrt_guid, (void *)new_esrt));
+	if (ret != EFI_SUCCESS)
+		EFI_PRINT("ESRT failed to install the ESRT\n");
+
+	return new_esrt;
+}
+
+/**
+ * efi_esrt_resize() - Increments the ESRT to hold an addition @inc_entries FW
+ * image entries.
+ *
+ * @inc_entries: the additional number of FW image entries to add to the ESRT.
+ *
+ * Return:
+ * - EFI_SUCCESS if ESRT correctly resized.
+ * - Error code otherwise.
+ */
+static efi_status_t efi_esrt_resize(u32 inc_entries)
+{
+	struct efi_boot_services *bt = systab.boottime;
+	struct efi_system_resource_table *old_esrt = esrt;
+	struct efi_system_resource_table *new_esrt;
+
+	u32 old_max_entries = old_esrt->fw_resource_count_max;
+	u32 old_filled_size;
+	efi_status_t ret;
+
+	if (!bt) {
+		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
+		return EFI_NOT_READY;
+	}
+
+	/* Allocate and install the bigger ESRT. */
+	new_esrt = efi_esrt_allocate_install(bt, old_max_entries + inc_entries);
+	if (!esrt) {
+		EFI_PRINT("ESRT failed to allocate and install resized ESRT\n");
+		esrt = old_esrt;
+		return EFI_OUT_OF_RESOURCES;
+	}
+	esrt = new_esrt;
+
+	old_filled_size = efi_esrt_entries_to_size(old_esrt->fw_resource_count_max);
+
+	/* Copy the old ESRT entries onto the new table. */
+	memcpy(new_esrt->entries, old_esrt->entries, old_filled_size
+		- sizeof(struct efi_system_resource_table));
+
+	new_esrt->fw_resource_count =  old_esrt->fw_resource_count;
+
+	ret = EFI_CALL(bt->free_pool(old_esrt));
+	if (ret != EFI_SUCCESS) {
+		/*
+		 * This should never happen.
+		 *
+		 * if ret!= EFI_SUCCESS then old_esrt is invalid.
+		 */
+		panic("EFI ESRT: could not deallocate old ESRT at %p\n", old_esrt);
+	}
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * esrt_find_entry() - Obtain the ESRT entry for the image with GUID
+ * @img_fw_class.
+ *
+ * If the img_fw_class is not yet present in the ESRT, this function
+ * reserves the tail element of the current ESRT as the entry for that fw_class.
+ * The number of elements in the ESRT is updated in that case.
+ *
+ * @img_fw_class: the GUID of the FW image which ESRT entry we want to obtain.
+ *
+ * Return:
+ *  - a pointer to the ESRT entry for the image with GUID img_fw_class,
+ *  - NULL if:
+ *   - there is no more space in the ESRT,
+ *   - ESRT is not initialized,
+ *   - boot services are not present.
+ */
+static
+struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
+{
+	u32 filled_entries;
+	u32 max_entries;
+	struct efi_system_resource_entry *entry;
+
+	if (!esrt) {
+		EFI_PRINT("ESRT access before initialized\n");
+		return NULL;
+	}
+
+	filled_entries = esrt->fw_resource_count;
+	entry = esrt->entries;
+
+	/* Check if the image with img_fw_class is already in the ESRT. */
+	for (u32 idx = 0; idx < filled_entries; idx++) {
+		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
+			EFI_PRINT("ESRT found entry for image %pUl@index %d\n",
+				  img_fw_class, idx);
+			return &entry[idx];
+		}
+	}
+
+	max_entries = esrt->fw_resource_count_max;
+	/* Since the image with img_fw_class is not present in the ESRT, check
+	 * if ESRT is full before appending the new entry to it. */
+	if (filled_entries == max_entries) {
+		efi_status_t ret;
+
+		/* ESRT is full, attempt to extend the ESRT entries. */
+		ret = efi_esrt_resize(EFI_ESRT_MIN_RESIZE_ENTIRES);
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT full, failed to resize\n");
+			return NULL;
+		}
+
+		entry = esrt->entries;
+		EFI_PRINT("ESRT table resized\n");
+	}
+
+	/*
+	 * This is a new entry for a fw image, increment the element
+	 * number in the table and set the fw_class field.
+	 */
+	esrt->fw_resource_count++;
+	entry[filled_entries].fw_class = *img_fw_class;
+	EFI_PRINT("ESRT allocated new entry for image %pUl at index %d\n",
+		  img_fw_class, filled_entries);
+
+	return &entry[filled_entries];
+}
+
+/**
+ * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
+ * images in the FMP.
+ *
+ * @fmp: the FMP instance from which FW images are added to the ESRT
+ *
+ * Return:
+ * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
+ * - Error status otherwise
+ */
+efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp)
+{
+	struct efi_boot_services *bt = systab.boottime;
+	struct efi_system_resource_entry *entry = NULL;
+	size_t info_size = 0;
+	struct efi_firmware_image_descriptor *img_info = NULL;
+	u32 desc_version;
+	u8 desc_count;
+	size_t desc_size;
+	efi_status_t ret = EFI_SUCCESS;
+
+	if (!bt) {
+		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
+		return EFI_NOT_READY;
+	}
+
+	/*
+	 * TODO: set the field image_type depending on the FW image type
+	 * defined in a platform basis.
+	 */
+	u32 image_type = ESRT_FW_TYPE_UNKNOWN;
+
+	/* TODO: set the capsule flags as a function of the FW image type. */
+	u32 flags = 0;
+
+	ret = fmp->get_image_info(fmp, &info_size, img_info,
+			&desc_version, &desc_count,
+			&desc_size, NULL, NULL);
+
+	if (ret != EFI_BUFFER_TOO_SMALL) {
+		/*
+		 * An input of info_size=0 should always lead
+		 * fmp->get_image_info to return BUFFER_TO_SMALL.
+		 */
+		EFI_PRINT("Erroneous FMP implementation\n");
+		return EFI_INVALID_PARAMETER;
+	}
+
+	ret = EFI_CALL(bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
+					 (void **)&img_info));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to allocate memory for image info.\n");
+		return ret;
+	}
+
+	ret = fmp->get_image_info(fmp, &info_size, img_info,
+			&desc_version, &desc_count,
+			&desc_size, NULL, NULL);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to obtain the FMP image info\n");
+		goto out;
+	}
+
+	/*
+	 * Iterate over all the FW images in the FMP.
+	 */
+	for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
+		struct efi_firmware_image_descriptor *cur_img_info =
+			(struct efi_firmware_image_descriptor *)
+			((uintptr_t)img_info + desc_idx * desc_size);
+
+		/*
+		 * Obtain the ESRT entry for the FW image with fw_class
+		 * equal to cur_img_info->image_type_id.
+		 */
+		entry = esrt_find_entry(&cur_img_info->image_type_id);
+
+		if (entry) {
+			efi_esrt_image_info_to_entry(cur_img_info, entry,
+						     desc_version, image_type,
+						     flags);
+		} else {
+			EFI_PRINT("ESRT failed to add entry for %pUl\n",
+				  &cur_img_info->image_type_id);
+			continue;
+		}
+	}
+
+out:
+	EFI_CALL(bt->free_pool(img_info));
+	return EFI_SUCCESS;
+}
+
+static void EFIAPI efi_esrt_new_fmp_notify(struct efi_event *event,
+					   void *context)
+{
+	efi_status_t ret;
+	efi_handle_t handle = NULL;
+	struct efi_firmware_management_protocol *fmp;
+	struct efi_boot_services *bt = systab.boottime;
+	size_t handle_buff_size = sizeof(handle);
+
+	if (!bt) {
+		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
+		return;
+	}
+
+	/* Obtain a single handle of the FMP type. */
+	ret = EFI_CALL(bt->locate_handle(BY_PROTOCOL,
+					 &efi_guid_firmware_management_protocol,
+					 NULL,
+					 &handle_buff_size,
+					 &handle
+					 ));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT cannot find FMP handle\n");
+		return;
+	}
+
+	/* Translate the received handle to a FMP object. */
+	ret = EFI_CALL(bt->handle_protocol(handle,
+					   &efi_guid_firmware_management_protocol,
+					   (void **)&fmp));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT(" ESRT cannot obtain FMP\n");
+		return;
+	}
+
+	/* Add all the FW images managed by FMP into the ESRT. */
+	ret = efi_esrt_add_from_fmp(fmp);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to populate ESRT entry\n");
+		return;
+	}
+}
+
+/**
+ * efi_esrt_register() - Install the ESRT system table.
+ *
+ * Return: status code
+ */
+efi_status_t efi_esrt_register(void)
+{
+	struct efi_boot_services *bt = systab.boottime;
+	struct efi_event *ev = NULL;
+	void *registration;
+	u32 num_entries = EFI_ESRT_MIN_RESIZE_ENTIRES;
+	efi_status_t ret;
+
+	if (!bt) {
+		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
+		return EFI_NOT_READY;
+	}
+
+	EFI_PRINT("ESRT creation start\n");
+
+	esrt = efi_esrt_allocate_install(bt, num_entries);
+	if (!esrt) {
+		EFI_PRINT("ESRT failed to initialize ESRT\n");
+		return EFI_NOT_READY;
+	}
+
+	ret = EFI_CALL(bt->create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+					efi_esrt_new_fmp_notify, NULL, &ev));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to create event\n");
+		return ret;
+	}
+
+	ret = EFI_CALL(bt->register_protocol_notify(&efi_guid_firmware_management_protocol,
+						    ev, &registration));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to register FMP callback\n");
+		return ret;
+	}
+
+	EFI_PRINT("ESRT table created\n");
+
+	return ret;
+}
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 5800cbf6d4..0183abd09e 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;
 
+#ifdef CONFIG_EFI_ESRT
+	ret = efi_esrt_register();
+	if (ret != EFI_SUCCESS)
+		goto out;
+#endif
+
 	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
 		ret = efi_tcg2_register();
 		if (ret != EFI_SUCCESS)
-- 
2.17.1

[PATCH 1/2] efi: Add ESRT to the EFI system table

[Heinrich Schuchardt]

On 2/8/21 1:52 PM, Jose Marinho wrote:
> The ESRT is initialised during efi_init_objlist after
> efi_initialize_system_table().
>
> The ESRT is initially created with size for 50 FW image entries.
> The ESRT is resized when it runs out of space. Every resize adds 50
> additional entries.
> The ESRT is populated from information provided by FMP instances only.
>
> Signed-off-by: Jose Marinho<jose.marinho@arm.com>


<josem> one limitation is, if any FMP instance is installed and then
uninstalled during the same boottime flow, the ESRT entries will not be
removed
<josem> this limitation is because I cannot find a proper UEFI way to
hook to a FMP protocol uninstall.
<xypron> register an event with RegisterProtocolNotify().
<xypron> EFI_CALL(efi_register_protocol_notify(...)) after exporting the
function in /include/efi_loader
<xypron> The event can be created with efi_create_event().

[PATCH 1/2] efi: Add ESRT to the EFI system table

[Heinrich Schuchardt]

On 2/8/21 11:18 PM, Heinrich Schuchardt wrote:
> On 2/8/21 1:52 PM, Jose Marinho wrote:
>> The ESRT is initialised during efi_init_objlist after
>> efi_initialize_system_table().
>>
>> The ESRT is initially created with size for 50 FW image entries.
>> The ESRT is resized when it runs out of space. Every resize adds 50
>> additional entries.
>> The ESRT is populated from information provided by FMP instances only.
>>
>> Signed-off-by: Jose Marinho<jose.marinho@arm.com>
>
>
> <josem> one limitation is, if any FMP instance is installed and then
> uninstalled during the same boottime flow, the ESRT entries will not be
> removed
> <josem> this limitation is because I cannot find a proper UEFI way to
> hook to a FMP protocol uninstall.
> <xypron> register an event with RegisterProtocolNotify().
> <xypron> EFI_CALL(efi_register_protocol_notify(...)) after exporting the
> function in /include/efi_loader
> <xypron> The event can be created with efi_create_event().


RegisterProtocolNotifiy() only tells you that a protocol is newly
installed, or reinstalled.

If you open the protocol with EFI_OPEN_PROTOCOL_BY_DRIVER, than
EFI_BOOT_SERVICES.UninstallProtocolInterface() will call
EFI_BOOT_SERVICES.DisconnectController() for the driver which will end
up in calling the Stop() function of the driver binding protocol.

Instead of using RegisterProtocolNotifiy() it would be better if the FMP
driver would call ConnectController() after installing its protocol.

This project is gone to show you all delights of EFI driver development.

Best regards

Heinrich

[PATCH 1/2] efi: Add ESRT to the EFI system table

[Heinrich Schuchardt]

On 08.02.21 13:52, Jose Marinho wrote:
> The ESRT is initialised during efi_init_objlist after
> efi_initialize_system_table().
>
> The ESRT is initially created with size for 50 FW image entries.
> The ESRT is resized when it runs out of space. Every resize adds 50
> additional entries.
> The ESRT is populated from information provided by FMP instances only.
>
> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
>
> CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
>
> ---
>  cmd/efidebug.c                |   4 +
>  include/efi_api.h             |  21 ++
>  include/efi_loader.h          |  19 ++
>  lib/efi_loader/Kconfig        |   7 +
>  lib/efi_loader/Makefile       |   1 +
>  lib/efi_loader/efi_boottime.c |   2 -
>  lib/efi_loader/efi_capsule.c  |   7 +
>  lib/efi_loader/efi_esrt.c     | 439 ++++++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_setup.c    |   6 +
>  9 files changed, 504 insertions(+), 2 deletions(-)
>  create mode 100644 lib/efi_loader/efi_esrt.c
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index 83bc2196a5..4160dde1cf 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -458,6 +458,10 @@ static const struct {
>  		"Block IO",
>  		EFI_BLOCK_IO_PROTOCOL_GUID,
>  	},
> +	{
> +		"EFI System Resource Table",
> +		EFI_SYSTEM_RESOURCE_TABLE_GUID,
> +	},
>  	{
>  		"Simple File System",
>  		EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 48e48a6263..7eb15bd44c 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -1722,6 +1722,23 @@ struct efi_load_file_protocol {
>  					 void *buffer);
>  };
>
> +struct efi_system_resource_entry {
> +	efi_guid_t fw_class;
> +	uint32_t fw_type;
> +	uint32_t fw_version;
> +	uint32_t lowest_supported_fw_version;
> +	uint32_t capsule_flags;
> +	uint32_t last_attempt_version;
> +	uint32_t last_attempt_status;
> +} __packed;
> +
> +struct efi_system_resource_table {
> +	uint32_t fw_resource_count;
> +	uint32_t fw_resource_count_max;
> +	uint64_t fw_resource_version;
> +	struct efi_system_resource_entry entries[];
> +} __packed;
> +
>  /* Boot manager load options */
>  #define LOAD_OPTION_ACTIVE		0x00000001
>  #define LOAD_OPTION_FORCE_RECONNECT	0x00000002
> @@ -1740,6 +1757,10 @@ struct efi_load_file_protocol {
>  #define ESRT_FW_TYPE_DEVICEFIRMWARE	0x00000002
>  #define ESRT_FW_TYPE_UEFIDRIVER		0x00000003
>
> +#define EFI_SYSTEM_RESOURCE_TABLE_GUID\
> +	EFI_GUID(0xb122a263, 0x3661, 0x4f68,\
> +		0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80 )

Please, run scripts/checkpatch.pl on your patches before submitting.

ERROR: space prohibited before that close parenthesis ')'

> +
>  /* Last Attempt Status Values */
>  #define LAST_ATTEMPT_STATUS_SUCCESS			0x00000000
>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL		0x00000001
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index f470bbd636..c85c540041 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -884,4 +884,23 @@ static inline efi_status_t efi_launch_capsules(void)
>
>  #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
>
> +/*
> + * Install the ESRT system table.
> + *
> + * @return	status code
> + */
> +efi_status_t efi_esrt_register(void);
> +
> +/**
> + * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
> + * images in the FMP.
> + *
> + * @fmp:        the fmp from which fw images are added to the ESRT
> + *
> + * Return:
> + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> + * - Error status otherwise
> + */
> +efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp);
> +
>  #endif /* _EFI_LOADER_H */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e729f727df..12b29180a0 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -347,4 +347,11 @@ config EFI_SECURE_BOOT
>  	  it is signed with a trusted key. To do that, you need to install,
>  	  at least, PK, KEK and db.
>
> +config EFI_ESRT
> +	bool "Enable the UEFI ESRT generation"
> +	depends on EFI_LOADER

This line is after "if EFI_LOADER".

I assume EFI_ESRT should depend on CONFIG_EFI_HAVE_CAPSULE_SUPPORT.

> +	default y
> +	help
> +	  Enabling this option creates the ESRT UEFI system table.
> +
>  endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 10b42e8847..dec791b310 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -52,6 +52,7 @@ obj-y += efi_variable.o
>  obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
>  endif
>  obj-y += efi_watchdog.o
> +obj-y += efi_esrt.o
>  obj-$(CONFIG_LCD) += efi_gop.o
>  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>  obj-$(CONFIG_PARTITIONS) += efi_disk.o
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index ce658a8e73..9b0b15571a 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1148,8 +1148,6 @@ efi_status_t efi_add_protocol(const efi_handle_t handle,
>  		}
>  	}
>
> -	if (!guidcmp(&efi_guid_device_path, protocol))
> -		EFI_PRINT("installed device path '%pD'\n", protocol_interface);

This change is unrelated. Why would you want to remove the debug output?

>  	return EFI_SUCCESS;
>  }
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 0d5a7b63ec..60703bcdaa 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -404,6 +404,13 @@ static efi_status_t efi_capsule_update_firmware(
>  			efi_free_pool(abort_reason);
>  			goto out;
>  		}
> +
> +#ifdef CONFIG_EFI_ESRT

Please, use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'
where possible.

> +		/* Update the ESRT entries corresponding to fmp. */
> +		ret = efi_esrt_add_from_fmp(fmp);

Why don't you reuse efi_esrt_new_fmp_notify()?

> +		if (ret != EFI_SUCCESS)
> +			log_warning("EFI Capsule: failed to populate ESRT entry\n");

I think the warning can be moved to inside efi_esrt_add_from_fmp().

> +#endif

This would not catch an update via an FMP driver loaded via the bootefi
command. Shouldn't the call be add the end of efi_update_capsule()?

>  	}
>
>  out:
> diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
> new file mode 100644
> index 0000000000..34133346ca
> --- /dev/null
> +++ b/lib/efi_loader/efi_esrt.c
> @@ -0,0 +1,439 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *  EFI application ESRT tables support
> + *
> + *  Copyright (C) 2021 Arm Ltd.
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <log.h>
> +#include <efi_api.h>
> +#include <malloc.h>
> +
> +static efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
> +
> +struct efi_system_resource_table *esrt;
> +
> +#define EFI_ESRT_MIN_RESIZE_ENTIRES 50
> +#define EFI_ESRT_VERSION 1
> +
> +/**
> + * efi_esrt_image_info_to_entry() - copy the information present in a fw image
> + * descriptor to a ESRT entry.
> + *
> + * @img_info:     the source image info descriptor
> + * @entry:        pointer to the ESRT entry to be filled
> + * @desc_version: the version of the elements in img_info
> + * @image_type:   the image type value to be set in the ESRT entry
> + * @flags:        the capsule flags value to be set in the ESRT entry
> + *
> + */
> +void
> +efi_esrt_image_info_to_entry(struct efi_firmware_image_descriptor *img_info,
> +			     struct efi_system_resource_entry *entry,
> +			     u32 desc_version, u32 image_type, u32 flags)
> +{
> +	guidcpy(&entry->fw_class, &img_info->image_type_id);
> +	entry->fw_version = img_info->version;
> +
> +	entry->fw_type = image_type;
> +	entry->capsule_flags = flags;
> +
> +	/*
> +	 * The field lowest_supported_image_version is only present
> +	 * on image info structure of version 2 or greater.
> +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
> +	 */
> +	if (desc_version >= 2) {
> +		entry->lowest_supported_fw_version =
> +			img_info->lowest_supported_image_version;
> +	} else {
> +		entry->lowest_supported_fw_version = 0;
> +	}
> +
> +	/*
> +	 * The fields last_attempt_version and last_attempt_status
> +	 * are only present on image info structure of version 3 or
> +	 * greater.
> +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
> +	 */
> +	if (desc_version >= 3) {
> +		entry->last_attempt_version =
> +			img_info->last_attempt_version;
> +
> +		entry->last_attempt_status =
> +			img_info->last_attempt_status;
> +	} else {
> +		entry->last_attempt_version = 0;
> +		entry->last_attempt_status = EFI_SUCCESS;

This should be LAST_ATTEMPT_STATUS_SUCCESS.

> +	}
> +}
> +
> +/**
> + * efi_esrt_entries_to_size() - Obtain the bytes used by an ESRT
> + * datastructure with @num_entries.
> + *
> + * @num_entries: the ESRT which we obtain the filled size in bytes.
> + *
> + * Return: the number of bytes an ESRT with @num_entries occupies in memory.
> + */
> +static
> +inline u32 efi_esrt_entries_to_size(u32 num_entries)
> +{
> +	u32 esrt_size = sizeof(struct efi_system_resource_table) +
> +		num_entries * sizeof(struct efi_system_resource_entry);
> +
> +	return esrt_size;
> +}
> +
> +/**
> + * efi_esrt_allocate_install() - Allocates @num_entries for the ESRT and
> + * performs basic ESRT initialization.
> + *
> + * @bt	       : pointer to the boottime services structure.
> + * @num_entries: the number of entries that the ESRT will hold.
> + *
> + * Return:
> + * - pointer to the ESRT if successful.
> + * - NULL otherwise.
> + */
> +static
> +struct efi_system_resource_table *efi_esrt_allocate_install(struct efi_boot_services *bt,
> +							    u32 num_entries)
> +{
> +	efi_status_t ret;
> +	struct efi_system_resource_table *new_esrt;
> +	u32 size = efi_esrt_entries_to_size(num_entries);
> +
> +	/* Allocated pages must be on the lower 32bit address space. */
> +	new_esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
> +
> +	/* Reserve num_pages for ESRT */
> +	ret = EFI_CALL(bt->allocate_pool(EFI_RUNTIME_SERVICES_DATA,
> +					 size,
> +					 (void **)&new_esrt));
> +
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT Failed to allocate memory for ESRT with %d entries (%d bytes)\n",
> +			  num_entries, efi_esrt_entries_to_size(num_entries));
> +
> +		return NULL;
> +	}
> +
> +	new_esrt->fw_resource_count_max = num_entries;
> +	new_esrt->fw_resource_version = EFI_ESRT_VERSION;
> +
> +	/* Install the ESRT in the system configuration table. */
> +	ret = EFI_CALL(bt->install_configuration_table(&esrt_guid, (void *)new_esrt));
> +	if (ret != EFI_SUCCESS)
> +		EFI_PRINT("ESRT failed to install the ESRT\n");
> +
> +	return new_esrt;
> +}
> +
> +/**
> + * efi_esrt_resize() - Increments the ESRT to hold an addition @inc_entries FW
> + * image entries.
> + *
> + * @inc_entries: the additional number of FW image entries to add to the ESRT.
> + *
> + * Return:
> + * - EFI_SUCCESS if ESRT correctly resized.
> + * - Error code otherwise.
> + */
> +static efi_status_t efi_esrt_resize(u32 inc_entries)
> +{
> +	struct efi_boot_services *bt = systab.boottime;
> +	struct efi_system_resource_table *old_esrt = esrt;
> +	struct efi_system_resource_table *new_esrt;

This seems overly complex. Just dispose of the existing table and create
a new one if either an FMP protocol is installed or CapsuleUpdate()  occurs.

> +
> +	u32 old_max_entries = old_esrt->fw_resource_count_max;
> +	u32 old_filled_size;
> +	efi_status_t ret;
> +
> +	if (!bt) {
> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> +		return EFI_NOT_READY;
> +	}
> +
> +	/* Allocate and install the bigger ESRT. */
> +	new_esrt = efi_esrt_allocate_install(bt, old_max_entries + inc_entries);
> +	if (!esrt) {
> +		EFI_PRINT("ESRT failed to allocate and install resized ESRT\n");
> +		esrt = old_esrt;
> +		return EFI_OUT_OF_RESOURCES;
> +	}
> +	esrt = new_esrt;
> +
> +	old_filled_size = efi_esrt_entries_to_size(old_esrt->fw_resource_count_max);
> +
> +	/* Copy the old ESRT entries onto the new table. */
> +	memcpy(new_esrt->entries, old_esrt->entries, old_filled_size
> +		- sizeof(struct efi_system_resource_table));
> +
> +	new_esrt->fw_resource_count =  old_esrt->fw_resource_count;
> +
> +	ret = EFI_CALL(bt->free_pool(old_esrt));
> +	if (ret != EFI_SUCCESS) {
> +		/*
> +		 * This should never happen.
> +		 *
> +		 * if ret!= EFI_SUCCESS then old_esrt is invalid.
> +		 */
> +		panic("EFI ESRT: could not deallocate old ESRT at %p\n", old_esrt);
> +	}
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * esrt_find_entry() - Obtain the ESRT entry for the image with GUID
> + * @img_fw_class.
> + *
> + * If the img_fw_class is not yet present in the ESRT, this function
> + * reserves the tail element of the current ESRT as the entry for that fw_class.
> + * The number of elements in the ESRT is updated in that case.
> + *
> + * @img_fw_class: the GUID of the FW image which ESRT entry we want to obtain.
> + *
> + * Return:
> + *  - a pointer to the ESRT entry for the image with GUID img_fw_class,
> + *  - NULL if:
> + *   - there is no more space in the ESRT,
> + *   - ESRT is not initialized,
> + *   - boot services are not present.
> + */
> +static
> +struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
> +{
> +	u32 filled_entries;
> +	u32 max_entries;
> +	struct efi_system_resource_entry *entry;
> +
> +	if (!esrt) {
> +		EFI_PRINT("ESRT access before initialized\n");
> +		return NULL;
> +	}
> +
> +	filled_entries = esrt->fw_resource_count;
> +	entry = esrt->entries;
> +
> +	/* Check if the image with img_fw_class is already in the ESRT. */
> +	for (u32 idx = 0; idx < filled_entries; idx++) {
> +		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
> +			EFI_PRINT("ESRT found entry for image %pUl at index %d\n",
> +				  img_fw_class, idx);
> +			return &entry[idx];
> +		}
> +	}
> +
> +	max_entries = esrt->fw_resource_count_max;
> +	/* Since the image with img_fw_class is not present in the ESRT, check
> +	 * if ESRT is full before appending the new entry to it. */
> +	if (filled_entries == max_entries) {
> +		efi_status_t ret;
> +
> +		/* ESRT is full, attempt to extend the ESRT entries. */
> +		ret = efi_esrt_resize(EFI_ESRT_MIN_RESIZE_ENTIRES);
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT full, failed to resize\n");
> +			return NULL;
> +		}
> +
> +		entry = esrt->entries;
> +		EFI_PRINT("ESRT table resized\n");
> +	}
> +
> +	/*
> +	 * This is a new entry for a fw image, increment the element
> +	 * number in the table and set the fw_class field.
> +	 */
> +	esrt->fw_resource_count++;
> +	entry[filled_entries].fw_class = *img_fw_class;
> +	EFI_PRINT("ESRT allocated new entry for image %pUl at index %d\n",
> +		  img_fw_class, filled_entries);
> +
> +	return &entry[filled_entries];
> +}
> +
> +/**
> + * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
> + * images in the FMP.
> + *
> + * @fmp: the FMP instance from which FW images are added to the ESRT
> + *
> + * Return:
> + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> + * - Error status otherwise
> + */
> +efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp)
> +{
> +	struct efi_boot_services *bt = systab.boottime;
> +	struct efi_system_resource_entry *entry = NULL;
> +	size_t info_size = 0;
> +	struct efi_firmware_image_descriptor *img_info = NULL;
> +	u32 desc_version;
> +	u8 desc_count;
> +	size_t desc_size;
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	if (!bt) {
> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> +		return EFI_NOT_READY;
> +	}
> +
> +	/*
> +	 * TODO: set the field image_type depending on the FW image type
> +	 * defined in a platform basis.
> +	 */
> +	u32 image_type = ESRT_FW_TYPE_UNKNOWN;
> +
> +	/* TODO: set the capsule flags as a function of the FW image type. */
> +	u32 flags = 0;
> +
> +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> +			&desc_version, &desc_count,
> +			&desc_size, NULL, NULL);
> +
> +	if (ret != EFI_BUFFER_TOO_SMALL) {
> +		/*
> +		 * An input of info_size=0 should always lead
> +		 * fmp->get_image_info to return BUFFER_TO_SMALL.
> +		 */
> +		EFI_PRINT("Erroneous FMP implementation\n");
> +		return EFI_INVALID_PARAMETER;
> +	}
> +
> +	ret = EFI_CALL(bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
> +					 (void **)&img_info));
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to allocate memory for image info.\n");
> +		return ret;
> +	}
> +
> +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> +			&desc_version, &desc_count,
> +			&desc_size, NULL, NULL);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to obtain the FMP image info\n");
> +		goto out;
> +	}
> +
> +	/*
> +	 * Iterate over all the FW images in the FMP.
> +	 */
> +	for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
> +		struct efi_firmware_image_descriptor *cur_img_info =
> +			(struct efi_firmware_image_descriptor *)
> +			((uintptr_t)img_info + desc_idx * desc_size);
> +
> +		/*
> +		 * Obtain the ESRT entry for the FW image with fw_class
> +		 * equal to cur_img_info->image_type_id.
> +		 */
> +		entry = esrt_find_entry(&cur_img_info->image_type_id);
> +
> +		if (entry) {
> +			efi_esrt_image_info_to_entry(cur_img_info, entry,
> +						     desc_version, image_type,
> +						     flags);
> +		} else {
> +			EFI_PRINT("ESRT failed to add entry for %pUl\n",
> +				  &cur_img_info->image_type_id);
> +			continue;
> +		}
> +	}
> +
> +out:
> +	EFI_CALL(bt->free_pool(img_info));
> +	return EFI_SUCCESS;
> +}
> +
> +static void EFIAPI efi_esrt_new_fmp_notify(struct efi_event *event,
> +					   void *context)
> +{
> +	efi_status_t ret;
> +	efi_handle_t handle = NULL;
> +	struct efi_firmware_management_protocol *fmp;
> +	struct efi_boot_services *bt = systab.boottime;
> +	size_t handle_buff_size = sizeof(handle);
> +
> +	if (!bt) {
> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> +		return;
> +	}
> +
> +	/* Obtain a single handle of the FMP type. */
> +	ret = EFI_CALL(bt->locate_handle(BY_PROTOCOL,
> +					 &efi_guid_firmware_management_protocol,
> +					 NULL,
> +					 &handle_buff_size,
> +					 &handle
> +					 ));
> +	if (ret != EFI_SUCCESS) {

There may be many handles for the FMP protocol. I would expect a handler
for EFI_BUFFER_TOO_SMALL here.

It is easier to use LocateHandleBuffer() to get all FMP protocols.

As said is wourd prefer replacing the complete ESRT when
efi_esrt_new_fmp_notify() is called instead of incremental changes.

Best regards

Heinrich

> +		EFI_PRINT("ESRT cannot find FMP handle\n");
> +		return;
> +	}
> +
> +	/* Translate the received handle to a FMP object. */
> +	ret = EFI_CALL(bt->handle_protocol(handle,
> +					   &efi_guid_firmware_management_protocol,
> +					   (void **)&fmp));
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT(" ESRT cannot obtain FMP\n");
> +		return;
> +	}
> +
> +	/* Add all the FW images managed by FMP into the ESRT. */
> +	ret = efi_esrt_add_from_fmp(fmp);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to populate ESRT entry\n");
> +		return;
> +	}
> +}
> +
> +/**
> + * efi_esrt_register() - Install the ESRT system table.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_esrt_register(void)
> +{
> +	struct efi_boot_services *bt = systab.boottime;
> +	struct efi_event *ev = NULL;
> +	void *registration;
> +	u32 num_entries = EFI_ESRT_MIN_RESIZE_ENTIRES;
> +	efi_status_t ret;
> +
> +	if (!bt) {
> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> +		return EFI_NOT_READY;
> +	}
> +
> +	EFI_PRINT("ESRT creation start\n");
> +
> +	esrt = efi_esrt_allocate_install(bt, num_entries);
> +	if (!esrt) {
> +		EFI_PRINT("ESRT failed to initialize ESRT\n");
> +		return EFI_NOT_READY;
> +	}
> +
> +	ret = EFI_CALL(bt->create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +					efi_esrt_new_fmp_notify, NULL, &ev));
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to create event\n");
> +		return ret;
> +	}
> +
> +	ret = EFI_CALL(bt->register_protocol_notify(&efi_guid_firmware_management_protocol,
> +						    ev, &registration));
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to register FMP callback\n");
> +		return ret;
> +	}
> +
> +	EFI_PRINT("ESRT table created\n");
> +
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 5800cbf6d4..0183abd09e 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;
>
> +#ifdef CONFIG_EFI_ESRT
> +	ret = efi_esrt_register();

Please, add a comment that this function must be called before
efi_launch_capsules().

Your code does not work for CONFIG_EFI_CAPSULE_ON_DISK_EARLY=y because
efi_launch_capsules() is called before efi_init_obj_list().

@Sughosh

I think EFI_CAPSULE_ON_DISK_EARLY has to be corrected first.
arch_efi_load_capsule_drivers() tries to install protocols on the root
handle before the root handle has been created.

Best regards

Heinrich

> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +#endif
> +
>  	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
>  		ret = efi_tcg2_register();
>  		if (ret != EFI_SUCCESS)
>

[PATCH 1/2] efi: Add ESRT to the EFI system table

[Heinrich Schuchardt]

On 16.02.21 10:06, Heinrich Schuchardt wrote:
> On 08.02.21 13:52, Jose Marinho wrote:
>> The ESRT is initialised during efi_init_objlist after
>> efi_initialize_system_table().
>>
>> The ESRT is initially created with size for 50 FW image entries.
>> The ESRT is resized when it runs out of space. Every resize adds 50
>> additional entries.
>> The ESRT is populated from information provided by FMP instances only.
>>
>> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
>>
>> CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
>> CC: Sughosh Ganu <sughosh.ganu@linaro.org>
>> CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> CC: Andre Przywara <andre.przywara@arm.com>
>> CC: Alexander Graf <agraf@csgraf.de>
>> CC: nd at arm.com
>>
>> ---
>>  cmd/efidebug.c                |   4 +
>>  include/efi_api.h             |  21 ++
>>  include/efi_loader.h          |  19 ++
>>  lib/efi_loader/Kconfig        |   7 +
>>  lib/efi_loader/Makefile       |   1 +
>>  lib/efi_loader/efi_boottime.c |   2 -
>>  lib/efi_loader/efi_capsule.c  |   7 +
>>  lib/efi_loader/efi_esrt.c     | 439 ++++++++++++++++++++++++++++++++++
>>  lib/efi_loader/efi_setup.c    |   6 +
>>  9 files changed, 504 insertions(+), 2 deletions(-)
>>  create mode 100644 lib/efi_loader/efi_esrt.c
>>
>> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>> index 83bc2196a5..4160dde1cf 100644
>> --- a/cmd/efidebug.c
>> +++ b/cmd/efidebug.c
>> @@ -458,6 +458,10 @@ static const struct {
>>  		"Block IO",
>>  		EFI_BLOCK_IO_PROTOCOL_GUID,
>>  	},
>> +	{
>> +		"EFI System Resource Table",
>> +		EFI_SYSTEM_RESOURCE_TABLE_GUID,
>> +	},
>>  	{
>>  		"Simple File System",
>>  		EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index 48e48a6263..7eb15bd44c 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -1722,6 +1722,23 @@ struct efi_load_file_protocol {
>>  					 void *buffer);
>>  };
>>
>> +struct efi_system_resource_entry {
>> +	efi_guid_t fw_class;
>> +	uint32_t fw_type;
>> +	uint32_t fw_version;
>> +	uint32_t lowest_supported_fw_version;
>> +	uint32_t capsule_flags;
>> +	uint32_t last_attempt_version;
>> +	uint32_t last_attempt_status;
>> +} __packed;
>> +
>> +struct efi_system_resource_table {
>> +	uint32_t fw_resource_count;
>> +	uint32_t fw_resource_count_max;
>> +	uint64_t fw_resource_version;
>> +	struct efi_system_resource_entry entries[];
>> +} __packed;
>> +
>>  /* Boot manager load options */
>>  #define LOAD_OPTION_ACTIVE		0x00000001
>>  #define LOAD_OPTION_FORCE_RECONNECT	0x00000002
>> @@ -1740,6 +1757,10 @@ struct efi_load_file_protocol {
>>  #define ESRT_FW_TYPE_DEVICEFIRMWARE	0x00000002
>>  #define ESRT_FW_TYPE_UEFIDRIVER		0x00000003
>>
>> +#define EFI_SYSTEM_RESOURCE_TABLE_GUID\
>> +	EFI_GUID(0xb122a263, 0x3661, 0x4f68,\
>> +		0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80 )
>
> Please, run scripts/checkpatch.pl on your patches before submitting.
>
> ERROR: space prohibited before that close parenthesis ')'
>
>> +
>>  /* Last Attempt Status Values */
>>  #define LAST_ATTEMPT_STATUS_SUCCESS			0x00000000
>>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL		0x00000001
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index f470bbd636..c85c540041 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -884,4 +884,23 @@ static inline efi_status_t efi_launch_capsules(void)
>>
>>  #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
>>
>> +/*
>> + * Install the ESRT system table.
>> + *
>> + * @return	status code
>> + */
>> +efi_status_t efi_esrt_register(void);
>> +
>> +/**
>> + * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
>> + * images in the FMP.
>> + *
>> + * @fmp:        the fmp from which fw images are added to the ESRT
>> + *
>> + * Return:
>> + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
>> + * - Error status otherwise
>> + */
>> +efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp);
>> +
>>  #endif /* _EFI_LOADER_H */
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index e729f727df..12b29180a0 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -347,4 +347,11 @@ config EFI_SECURE_BOOT
>>  	  it is signed with a trusted key. To do that, you need to install,
>>  	  at least, PK, KEK and db.
>>
>> +config EFI_ESRT
>> +	bool "Enable the UEFI ESRT generation"
>> +	depends on EFI_LOADER
>
> This line is after "if EFI_LOADER".
>
> I assume EFI_ESRT should depend on CONFIG_EFI_HAVE_CAPSULE_SUPPORT.
>
>> +	default y
>> +	help
>> +	  Enabling this option creates the ESRT UEFI system table.
>> +
>>  endif
>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>> index 10b42e8847..dec791b310 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -52,6 +52,7 @@ obj-y += efi_variable.o
>>  obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
>>  endif
>>  obj-y += efi_watchdog.o
>> +obj-y += efi_esrt.o
>>  obj-$(CONFIG_LCD) += efi_gop.o
>>  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>>  obj-$(CONFIG_PARTITIONS) += efi_disk.o
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index ce658a8e73..9b0b15571a 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -1148,8 +1148,6 @@ efi_status_t efi_add_protocol(const efi_handle_t handle,
>>  		}
>>  	}
>>
>> -	if (!guidcmp(&efi_guid_device_path, protocol))
>> -		EFI_PRINT("installed device path '%pD'\n", protocol_interface);
>
> This change is unrelated. Why would you want to remove the debug output?
>
>>  	return EFI_SUCCESS;
>>  }
>>
>> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
>> index 0d5a7b63ec..60703bcdaa 100644
>> --- a/lib/efi_loader/efi_capsule.c
>> +++ b/lib/efi_loader/efi_capsule.c
>> @@ -404,6 +404,13 @@ static efi_status_t efi_capsule_update_firmware(
>>  			efi_free_pool(abort_reason);
>>  			goto out;
>>  		}
>> +
>> +#ifdef CONFIG_EFI_ESRT
>
> Please, use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'
> where possible.
>
>> +		/* Update the ESRT entries corresponding to fmp. */
>> +		ret = efi_esrt_add_from_fmp(fmp);
>
> Why don't you reuse efi_esrt_new_fmp_notify()?
>
>> +		if (ret != EFI_SUCCESS)
>> +			log_warning("EFI Capsule: failed to populate ESRT entry\n");
>
> I think the warning can be moved to inside efi_esrt_add_from_fmp().
>
>> +#endif
>
> This would not catch an update via an FMP driver loaded via the bootefi
> command. Shouldn't the call be add the end of efi_update_capsule()?
>
>>  	}
>>
>>  out:
>> diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
>> new file mode 100644
>> index 0000000000..34133346ca
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_esrt.c
>> @@ -0,0 +1,439 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + *  EFI application ESRT tables support
>> + *
>> + *  Copyright (C) 2021 Arm Ltd.
>> + */
>> +
>> +#include <common.h>
>> +#include <efi_loader.h>
>> +#include <log.h>
>> +#include <efi_api.h>
>> +#include <malloc.h>
>> +
>> +static efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
>> +
>> +struct efi_system_resource_table *esrt;
>> +
>> +#define EFI_ESRT_MIN_RESIZE_ENTIRES 50
>> +#define EFI_ESRT_VERSION 1
>> +
>> +/**
>> + * efi_esrt_image_info_to_entry() - copy the information present in a fw image
>> + * descriptor to a ESRT entry.
>> + *
>> + * @img_info:     the source image info descriptor
>> + * @entry:        pointer to the ESRT entry to be filled
>> + * @desc_version: the version of the elements in img_info
>> + * @image_type:   the image type value to be set in the ESRT entry
>> + * @flags:        the capsule flags value to be set in the ESRT entry
>> + *
>> + */
>> +void
>> +efi_esrt_image_info_to_entry(struct efi_firmware_image_descriptor *img_info,
>> +			     struct efi_system_resource_entry *entry,
>> +			     u32 desc_version, u32 image_type, u32 flags)
>> +{
>> +	guidcpy(&entry->fw_class, &img_info->image_type_id);
>> +	entry->fw_version = img_info->version;
>> +
>> +	entry->fw_type = image_type;
>> +	entry->capsule_flags = flags;
>> +
>> +	/*
>> +	 * The field lowest_supported_image_version is only present
>> +	 * on image info structure of version 2 or greater.
>> +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
>> +	 */
>> +	if (desc_version >= 2) {
>> +		entry->lowest_supported_fw_version =
>> +			img_info->lowest_supported_image_version;
>> +	} else {
>> +		entry->lowest_supported_fw_version = 0;
>> +	}
>> +
>> +	/*
>> +	 * The fields last_attempt_version and last_attempt_status
>> +	 * are only present on image info structure of version 3 or
>> +	 * greater.
>> +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
>> +	 */
>> +	if (desc_version >= 3) {
>> +		entry->last_attempt_version =
>> +			img_info->last_attempt_version;
>> +
>> +		entry->last_attempt_status =
>> +			img_info->last_attempt_status;
>> +	} else {
>> +		entry->last_attempt_version = 0;
>> +		entry->last_attempt_status = EFI_SUCCESS;
>
> This should be LAST_ATTEMPT_STATUS_SUCCESS.
>
>> +	}
>> +}
>> +
>> +/**
>> + * efi_esrt_entries_to_size() - Obtain the bytes used by an ESRT
>> + * datastructure with @num_entries.
>> + *
>> + * @num_entries: the ESRT which we obtain the filled size in bytes.
>> + *
>> + * Return: the number of bytes an ESRT with @num_entries occupies in memory.
>> + */
>> +static
>> +inline u32 efi_esrt_entries_to_size(u32 num_entries)
>> +{
>> +	u32 esrt_size = sizeof(struct efi_system_resource_table) +
>> +		num_entries * sizeof(struct efi_system_resource_entry);
>> +
>> +	return esrt_size;
>> +}
>> +
>> +/**
>> + * efi_esrt_allocate_install() - Allocates @num_entries for the ESRT and
>> + * performs basic ESRT initialization.
>> + *
>> + * @bt	       : pointer to the boottime services structure.
>> + * @num_entries: the number of entries that the ESRT will hold.
>> + *
>> + * Return:
>> + * - pointer to the ESRT if successful.
>> + * - NULL otherwise.
>> + */
>> +static
>> +struct efi_system_resource_table *efi_esrt_allocate_install(struct efi_boot_services *bt,
>> +							    u32 num_entries)
>> +{
>> +	efi_status_t ret;
>> +	struct efi_system_resource_table *new_esrt;
>> +	u32 size = efi_esrt_entries_to_size(num_entries);
>> +
>> +	/* Allocated pages must be on the lower 32bit address space. */
>> +	new_esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
>> +
>> +	/* Reserve num_pages for ESRT */
>> +	ret = EFI_CALL(bt->allocate_pool(EFI_RUNTIME_SERVICES_DATA,
>> +					 size,
>> +					 (void **)&new_esrt));
>> +
>> +	if (ret != EFI_SUCCESS) {
>> +		EFI_PRINT("ESRT Failed to allocate memory for ESRT with %d entries (%d bytes)\n",
>> +			  num_entries, efi_esrt_entries_to_size(num_entries));
>> +
>> +		return NULL;
>> +	}
>> +
>> +	new_esrt->fw_resource_count_max = num_entries;
>> +	new_esrt->fw_resource_version = EFI_ESRT_VERSION;
>> +
>> +	/* Install the ESRT in the system configuration table. */
>> +	ret = EFI_CALL(bt->install_configuration_table(&esrt_guid, (void *)new_esrt));
>> +	if (ret != EFI_SUCCESS)
>> +		EFI_PRINT("ESRT failed to install the ESRT\n");
>> +
>> +	return new_esrt;
>> +}
>> +
>> +/**
>> + * efi_esrt_resize() - Increments the ESRT to hold an addition @inc_entries FW
>> + * image entries.
>> + *
>> + * @inc_entries: the additional number of FW image entries to add to the ESRT.
>> + *
>> + * Return:
>> + * - EFI_SUCCESS if ESRT correctly resized.
>> + * - Error code otherwise.
>> + */
>> +static efi_status_t efi_esrt_resize(u32 inc_entries)
>> +{
>> +	struct efi_boot_services *bt = systab.boottime;
>> +	struct efi_system_resource_table *old_esrt = esrt;
>> +	struct efi_system_resource_table *new_esrt;
>
> This seems overly complex. Just dispose of the existing table and create
> a new one if either an FMP protocol is installed or CapsuleUpdate()  occurs.
>
>> +
>> +	u32 old_max_entries = old_esrt->fw_resource_count_max;
>> +	u32 old_filled_size;
>> +	efi_status_t ret;
>> +
>> +	if (!bt) {
>> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
>> +		return EFI_NOT_READY;
>> +	}
>> +
>> +	/* Allocate and install the bigger ESRT. */
>> +	new_esrt = efi_esrt_allocate_install(bt, old_max_entries + inc_entries);
>> +	if (!esrt) {
>> +		EFI_PRINT("ESRT failed to allocate and install resized ESRT\n");
>> +		esrt = old_esrt;
>> +		return EFI_OUT_OF_RESOURCES;
>> +	}
>> +	esrt = new_esrt;
>> +
>> +	old_filled_size = efi_esrt_entries_to_size(old_esrt->fw_resource_count_max);
>> +
>> +	/* Copy the old ESRT entries onto the new table. */
>> +	memcpy(new_esrt->entries, old_esrt->entries, old_filled_size
>> +		- sizeof(struct efi_system_resource_table));
>> +
>> +	new_esrt->fw_resource_count =  old_esrt->fw_resource_count;
>> +
>> +	ret = EFI_CALL(bt->free_pool(old_esrt));
>> +	if (ret != EFI_SUCCESS) {
>> +		/*
>> +		 * This should never happen.
>> +		 *
>> +		 * if ret!= EFI_SUCCESS then old_esrt is invalid.
>> +		 */
>> +		panic("EFI ESRT: could not deallocate old ESRT at %p\n", old_esrt);
>> +	}
>> +
>> +	return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> + * esrt_find_entry() - Obtain the ESRT entry for the image with GUID
>> + * @img_fw_class.
>> + *
>> + * If the img_fw_class is not yet present in the ESRT, this function
>> + * reserves the tail element of the current ESRT as the entry for that fw_class.
>> + * The number of elements in the ESRT is updated in that case.
>> + *
>> + * @img_fw_class: the GUID of the FW image which ESRT entry we want to obtain.
>> + *
>> + * Return:
>> + *  - a pointer to the ESRT entry for the image with GUID img_fw_class,
>> + *  - NULL if:
>> + *   - there is no more space in the ESRT,
>> + *   - ESRT is not initialized,
>> + *   - boot services are not present.
>> + */
>> +static
>> +struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
>> +{
>> +	u32 filled_entries;
>> +	u32 max_entries;
>> +	struct efi_system_resource_entry *entry;
>> +
>> +	if (!esrt) {
>> +		EFI_PRINT("ESRT access before initialized\n");
>> +		return NULL;
>> +	}
>> +
>> +	filled_entries = esrt->fw_resource_count;
>> +	entry = esrt->entries;
>> +
>> +	/* Check if the image with img_fw_class is already in the ESRT. */
>> +	for (u32 idx = 0; idx < filled_entries; idx++) {
>> +		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
>> +			EFI_PRINT("ESRT found entry for image %pUl at index %d\n",
>> +				  img_fw_class, idx);
>> +			return &entry[idx];
>> +		}
>> +	}
>> +
>> +	max_entries = esrt->fw_resource_count_max;
>> +	/* Since the image with img_fw_class is not present in the ESRT, check
>> +	 * if ESRT is full before appending the new entry to it. */
>> +	if (filled_entries == max_entries) {
>> +		efi_status_t ret;
>> +
>> +		/* ESRT is full, attempt to extend the ESRT entries. */
>> +		ret = efi_esrt_resize(EFI_ESRT_MIN_RESIZE_ENTIRES);
>> +		if (ret != EFI_SUCCESS) {
>> +			EFI_PRINT("ESRT full, failed to resize\n");
>> +			return NULL;
>> +		}
>> +
>> +		entry = esrt->entries;
>> +		EFI_PRINT("ESRT table resized\n");
>> +	}
>> +
>> +	/*
>> +	 * This is a new entry for a fw image, increment the element
>> +	 * number in the table and set the fw_class field.
>> +	 */
>> +	esrt->fw_resource_count++;
>> +	entry[filled_entries].fw_class = *img_fw_class;
>> +	EFI_PRINT("ESRT allocated new entry for image %pUl at index %d\n",
>> +		  img_fw_class, filled_entries);
>> +
>> +	return &entry[filled_entries];
>> +}
>> +
>> +/**
>> + * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
>> + * images in the FMP.
>> + *
>> + * @fmp: the FMP instance from which FW images are added to the ESRT
>> + *
>> + * Return:
>> + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
>> + * - Error status otherwise
>> + */
>> +efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp)
>> +{
>> +	struct efi_boot_services *bt = systab.boottime;
>> +	struct efi_system_resource_entry *entry = NULL;
>> +	size_t info_size = 0;
>> +	struct efi_firmware_image_descriptor *img_info = NULL;
>> +	u32 desc_version;
>> +	u8 desc_count;
>> +	size_t desc_size;
>> +	efi_status_t ret = EFI_SUCCESS;
>> +
>> +	if (!bt) {
>> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
>> +		return EFI_NOT_READY;
>> +	}
>> +
>> +	/*
>> +	 * TODO: set the field image_type depending on the FW image type
>> +	 * defined in a platform basis.
>> +	 */
>> +	u32 image_type = ESRT_FW_TYPE_UNKNOWN;
>> +
>> +	/* TODO: set the capsule flags as a function of the FW image type. */
>> +	u32 flags = 0;
>> +
>> +	ret = fmp->get_image_info(fmp, &info_size, img_info,
>> +			&desc_version, &desc_count,
>> +			&desc_size, NULL, NULL);
>> +
>> +	if (ret != EFI_BUFFER_TOO_SMALL) {
>> +		/*
>> +		 * An input of info_size=0 should always lead
>> +		 * fmp->get_image_info to return BUFFER_TO_SMALL.
>> +		 */
>> +		EFI_PRINT("Erroneous FMP implementation\n");
>> +		return EFI_INVALID_PARAMETER;
>> +	}
>> +
>> +	ret = EFI_CALL(bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
>> +					 (void **)&img_info));
>> +	if (ret != EFI_SUCCESS) {
>> +		EFI_PRINT("ESRT failed to allocate memory for image info.\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = fmp->get_image_info(fmp, &info_size, img_info,
>> +			&desc_version, &desc_count,
>> +			&desc_size, NULL, NULL);
>> +	if (ret != EFI_SUCCESS) {
>> +		EFI_PRINT("ESRT failed to obtain the FMP image info\n");
>> +		goto out;
>> +	}
>> +
>> +	/*
>> +	 * Iterate over all the FW images in the FMP.
>> +	 */
>> +	for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
>> +		struct efi_firmware_image_descriptor *cur_img_info =
>> +			(struct efi_firmware_image_descriptor *)
>> +			((uintptr_t)img_info + desc_idx * desc_size);
>> +
>> +		/*
>> +		 * Obtain the ESRT entry for the FW image with fw_class
>> +		 * equal to cur_img_info->image_type_id.
>> +		 */
>> +		entry = esrt_find_entry(&cur_img_info->image_type_id);
>> +
>> +		if (entry) {
>> +			efi_esrt_image_info_to_entry(cur_img_info, entry,
>> +						     desc_version, image_type,
>> +						     flags);
>> +		} else {
>> +			EFI_PRINT("ESRT failed to add entry for %pUl\n",
>> +				  &cur_img_info->image_type_id);
>> +			continue;
>> +		}
>> +	}
>> +
>> +out:
>> +	EFI_CALL(bt->free_pool(img_info));
>> +	return EFI_SUCCESS;
>> +}
>> +
>> +static void EFIAPI efi_esrt_new_fmp_notify(struct efi_event *event,
>> +					   void *context)
>> +{
>> +	efi_status_t ret;
>> +	efi_handle_t handle = NULL;
>> +	struct efi_firmware_management_protocol *fmp;
>> +	struct efi_boot_services *bt = systab.boottime;
>> +	size_t handle_buff_size = sizeof(handle);
>> +
>> +	if (!bt) {
>> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
>> +		return;
>> +	}
>> +
>> +	/* Obtain a single handle of the FMP type. */
>> +	ret = EFI_CALL(bt->locate_handle(BY_PROTOCOL,
>> +					 &efi_guid_firmware_management_protocol,
>> +					 NULL,
>> +					 &handle_buff_size,
>> +					 &handle
>> +					 ));
>> +	if (ret != EFI_SUCCESS) {
>
> There may be many handles for the FMP protocol. I would expect a handler
> for EFI_BUFFER_TOO_SMALL here.
>
> It is easier to use LocateHandleBuffer() to get all FMP protocols.
>
> As said is wourd prefer replacing the complete ESRT when
> efi_esrt_new_fmp_notify() is called instead of incremental changes.
>
> Best regards
>
> Heinrich
>
>> +		EFI_PRINT("ESRT cannot find FMP handle\n");
>> +		return;
>> +	}
>> +
>> +	/* Translate the received handle to a FMP object. */
>> +	ret = EFI_CALL(bt->handle_protocol(handle,
>> +					   &efi_guid_firmware_management_protocol,
>> +					   (void **)&fmp));
>> +	if (ret != EFI_SUCCESS) {
>> +		EFI_PRINT(" ESRT cannot obtain FMP\n");
>> +		return;
>> +	}
>> +
>> +	/* Add all the FW images managed by FMP into the ESRT. */
>> +	ret = efi_esrt_add_from_fmp(fmp);
>> +	if (ret != EFI_SUCCESS) {
>> +		EFI_PRINT("ESRT failed to populate ESRT entry\n");
>> +		return;
>> +	}
>> +}
>> +
>> +/**
>> + * efi_esrt_register() - Install the ESRT system table.
>> + *
>> + * Return: status code
>> + */
>> +efi_status_t efi_esrt_register(void)
>> +{
>> +	struct efi_boot_services *bt = systab.boottime;
>> +	struct efi_event *ev = NULL;
>> +	void *registration;
>> +	u32 num_entries = EFI_ESRT_MIN_RESIZE_ENTIRES;
>> +	efi_status_t ret;
>> +
>> +	if (!bt) {
>> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
>> +		return EFI_NOT_READY;
>> +	}
>> +
>> +	EFI_PRINT("ESRT creation start\n");
>> +
>> +	esrt = efi_esrt_allocate_install(bt, num_entries);
>> +	if (!esrt) {
>> +		EFI_PRINT("ESRT failed to initialize ESRT\n");
>> +		return EFI_NOT_READY;
>> +	}
>> +
>> +	ret = EFI_CALL(bt->create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
>> +					efi_esrt_new_fmp_notify, NULL, &ev));
>> +	if (ret != EFI_SUCCESS) {
>> +		EFI_PRINT("ESRT failed to create event\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = EFI_CALL(bt->register_protocol_notify(&efi_guid_firmware_management_protocol,
>> +						    ev, &registration));
>> +	if (ret != EFI_SUCCESS) {
>> +		EFI_PRINT("ESRT failed to register FMP callback\n");
>> +		return ret;
>> +	}
>> +
>> +	EFI_PRINT("ESRT table created\n");
>> +
>> +	return ret;
>> +}
>> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>> index 5800cbf6d4..0183abd09e 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;
>>
>> +#ifdef CONFIG_EFI_ESRT
>> +	ret = efi_esrt_register();
>
> Please, add a comment that this function must be called before
> efi_launch_capsules().
>
> Your code does not work for CONFIG_EFI_CAPSULE_ON_DISK_EARLY=y because
> efi_launch_capsules() is called before efi_init_obj_list().
>
> @Sughosh
>
> I think EFI_CAPSULE_ON_DISK_EARLY has to be corrected first.
> arch_efi_load_capsule_drivers() tries to install protocols on the root
> handle before the root handle has been created.

I was wrong here. efi_init_obj_list is always called first.

Cf.
https://lists.denx.de/pipermail/u-boot/2021-February/441313.html

Best regards

Heinrich
>
>> +	if (ret != EFI_SUCCESS)
>> +		goto out;
>> +#endif
>> +
>>  	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
>>  		ret = efi_tcg2_register();
>>  		if (ret != EFI_SUCCESS)
>>
>

[PATCH 1/2] efi: Add ESRT to the EFI system table

[AKASHI Takahiro]

On Tue, Feb 16, 2021 at 10:06:43AM +0100, Heinrich Schuchardt wrote:
> On 08.02.21 13:52, Jose Marinho wrote:
> > The ESRT is initialised during efi_init_objlist after
> > efi_initialize_system_table().
> >
> > The ESRT is initially created with size for 50 FW image entries.
> > The ESRT is resized when it runs out of space. Every resize adds 50
> > additional entries.
> > The ESRT is populated from information provided by FMP instances only.
> >
> > Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> >
> > CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> > CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> > CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > CC: Andre Przywara <andre.przywara@arm.com>
> > CC: Alexander Graf <agraf@csgraf.de>
> > CC: nd at arm.com
> >
> > ---
> >  cmd/efidebug.c                |   4 +
> >  include/efi_api.h             |  21 ++
> >  include/efi_loader.h          |  19 ++
> >  lib/efi_loader/Kconfig        |   7 +
> >  lib/efi_loader/Makefile       |   1 +
> >  lib/efi_loader/efi_boottime.c |   2 -
> >  lib/efi_loader/efi_capsule.c  |   7 +
> >  lib/efi_loader/efi_esrt.c     | 439 ++++++++++++++++++++++++++++++++++
> >  lib/efi_loader/efi_setup.c    |   6 +
> >  9 files changed, 504 insertions(+), 2 deletions(-)
> >  create mode 100644 lib/efi_loader/efi_esrt.c
> >
> > diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> > index 83bc2196a5..4160dde1cf 100644
> > --- a/cmd/efidebug.c
> > +++ b/cmd/efidebug.c
> > @@ -458,6 +458,10 @@ static const struct {
> >  		"Block IO",
> >  		EFI_BLOCK_IO_PROTOCOL_GUID,
> >  	},
> > +	{
> > +		"EFI System Resource Table",
> > +		EFI_SYSTEM_RESOURCE_TABLE_GUID,
> > +	},
> >  	{
> >  		"Simple File System",
> >  		EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 48e48a6263..7eb15bd44c 100644
> > --- a/include/efi_api.h
> > +++ b/include/efi_api.h
> > @@ -1722,6 +1722,23 @@ struct efi_load_file_protocol {
> >  					 void *buffer);
> >  };
> >
> > +struct efi_system_resource_entry {
> > +	efi_guid_t fw_class;
> > +	uint32_t fw_type;
> > +	uint32_t fw_version;
> > +	uint32_t lowest_supported_fw_version;
> > +	uint32_t capsule_flags;
> > +	uint32_t last_attempt_version;
> > +	uint32_t last_attempt_status;
> > +} __packed;
> > +
> > +struct efi_system_resource_table {
> > +	uint32_t fw_resource_count;
> > +	uint32_t fw_resource_count_max;
> > +	uint64_t fw_resource_version;
> > +	struct efi_system_resource_entry entries[];
> > +} __packed;
> > +
> >  /* Boot manager load options */
> >  #define LOAD_OPTION_ACTIVE		0x00000001
> >  #define LOAD_OPTION_FORCE_RECONNECT	0x00000002
> > @@ -1740,6 +1757,10 @@ struct efi_load_file_protocol {
> >  #define ESRT_FW_TYPE_DEVICEFIRMWARE	0x00000002
> >  #define ESRT_FW_TYPE_UEFIDRIVER		0x00000003
> >
> > +#define EFI_SYSTEM_RESOURCE_TABLE_GUID\
> > +	EFI_GUID(0xb122a263, 0x3661, 0x4f68,\
> > +		0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80 )
> 
> Please, run scripts/checkpatch.pl on your patches before submitting.
> 
> ERROR: space prohibited before that close parenthesis ')'
> 
> > +
> >  /* Last Attempt Status Values */
> >  #define LAST_ATTEMPT_STATUS_SUCCESS			0x00000000
> >  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL		0x00000001
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index f470bbd636..c85c540041 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -884,4 +884,23 @@ static inline efi_status_t efi_launch_capsules(void)
> >
> >  #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
> >
> > +/*
> > + * Install the ESRT system table.
> > + *
> > + * @return	status code
> > + */
> > +efi_status_t efi_esrt_register(void);
> > +
> > +/**
> > + * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
> > + * images in the FMP.
> > + *
> > + * @fmp:        the fmp from which fw images are added to the ESRT
> > + *
> > + * Return:
> > + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> > + * - Error status otherwise
> > + */
> > +efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp);
> > +
> >  #endif /* _EFI_LOADER_H */
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index e729f727df..12b29180a0 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -347,4 +347,11 @@ config EFI_SECURE_BOOT
> >  	  it is signed with a trusted key. To do that, you need to install,
> >  	  at least, PK, KEK and db.
> >
> > +config EFI_ESRT
> > +	bool "Enable the UEFI ESRT generation"
> > +	depends on EFI_LOADER
> 
> This line is after "if EFI_LOADER".
> 
> I assume EFI_ESRT should depend on CONFIG_EFI_HAVE_CAPSULE_SUPPORT.

Strictly speaking, this can be doubtful: According to "23.4.2
ESRT and Firmware Management Protocol,"
  Although the ESRT does not require firmware to use Firmware Management
  Protocol for updates it is designed to work with and extend the capabilities
  of FMP.

But as a matter of fact, the current implementation by Jose surely
has some dependency.

-Takahiro Akashi

> > +	default y
> > +	help
> > +	  Enabling this option creates the ESRT UEFI system table.
> > +
> >  endif
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index 10b42e8847..dec791b310 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -52,6 +52,7 @@ obj-y += efi_variable.o
> >  obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
> >  endif
> >  obj-y += efi_watchdog.o
> > +obj-y += efi_esrt.o
> >  obj-$(CONFIG_LCD) += efi_gop.o
> >  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
> >  obj-$(CONFIG_PARTITIONS) += efi_disk.o
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index ce658a8e73..9b0b15571a 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -1148,8 +1148,6 @@ efi_status_t efi_add_protocol(const efi_handle_t handle,
> >  		}
> >  	}
> >
> > -	if (!guidcmp(&efi_guid_device_path, protocol))
> > -		EFI_PRINT("installed device path '%pD'\n", protocol_interface);
> 
> This change is unrelated. Why would you want to remove the debug output?
> 
> >  	return EFI_SUCCESS;
> >  }
> >
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 0d5a7b63ec..60703bcdaa 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -404,6 +404,13 @@ static efi_status_t efi_capsule_update_firmware(
> >  			efi_free_pool(abort_reason);
> >  			goto out;
> >  		}
> > +
> > +#ifdef CONFIG_EFI_ESRT
> 
> Please, use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'
> where possible.
> 
> > +		/* Update the ESRT entries corresponding to fmp. */
> > +		ret = efi_esrt_add_from_fmp(fmp);
> 
> Why don't you reuse efi_esrt_new_fmp_notify()?
> 
> > +		if (ret != EFI_SUCCESS)
> > +			log_warning("EFI Capsule: failed to populate ESRT entry\n");
> 
> I think the warning can be moved to inside efi_esrt_add_from_fmp().
> 
> > +#endif
> 
> This would not catch an update via an FMP driver loaded via the bootefi
> command. Shouldn't the call be add the end of efi_update_capsule()?
> 
> >  	}
> >
> >  out:
> > diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
> > new file mode 100644
> > index 0000000000..34133346ca
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_esrt.c
> > @@ -0,0 +1,439 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + *  EFI application ESRT tables support
> > + *
> > + *  Copyright (C) 2021 Arm Ltd.
> > + */
> > +
> > +#include <common.h>
> > +#include <efi_loader.h>
> > +#include <log.h>
> > +#include <efi_api.h>
> > +#include <malloc.h>
> > +
> > +static efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
> > +
> > +struct efi_system_resource_table *esrt;
> > +
> > +#define EFI_ESRT_MIN_RESIZE_ENTIRES 50
> > +#define EFI_ESRT_VERSION 1
> > +
> > +/**
> > + * efi_esrt_image_info_to_entry() - copy the information present in a fw image
> > + * descriptor to a ESRT entry.
> > + *
> > + * @img_info:     the source image info descriptor
> > + * @entry:        pointer to the ESRT entry to be filled
> > + * @desc_version: the version of the elements in img_info
> > + * @image_type:   the image type value to be set in the ESRT entry
> > + * @flags:        the capsule flags value to be set in the ESRT entry
> > + *
> > + */
> > +void
> > +efi_esrt_image_info_to_entry(struct efi_firmware_image_descriptor *img_info,
> > +			     struct efi_system_resource_entry *entry,
> > +			     u32 desc_version, u32 image_type, u32 flags)
> > +{
> > +	guidcpy(&entry->fw_class, &img_info->image_type_id);
> > +	entry->fw_version = img_info->version;
> > +
> > +	entry->fw_type = image_type;
> > +	entry->capsule_flags = flags;
> > +
> > +	/*
> > +	 * The field lowest_supported_image_version is only present
> > +	 * on image info structure of version 2 or greater.
> > +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
> > +	 */
> > +	if (desc_version >= 2) {
> > +		entry->lowest_supported_fw_version =
> > +			img_info->lowest_supported_image_version;
> > +	} else {
> > +		entry->lowest_supported_fw_version = 0;
> > +	}
> > +
> > +	/*
> > +	 * The fields last_attempt_version and last_attempt_status
> > +	 * are only present on image info structure of version 3 or
> > +	 * greater.
> > +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
> > +	 */
> > +	if (desc_version >= 3) {
> > +		entry->last_attempt_version =
> > +			img_info->last_attempt_version;
> > +
> > +		entry->last_attempt_status =
> > +			img_info->last_attempt_status;
> > +	} else {
> > +		entry->last_attempt_version = 0;
> > +		entry->last_attempt_status = EFI_SUCCESS;
> 
> This should be LAST_ATTEMPT_STATUS_SUCCESS.
> 
> > +	}
> > +}
> > +
> > +/**
> > + * efi_esrt_entries_to_size() - Obtain the bytes used by an ESRT
> > + * datastructure with @num_entries.
> > + *
> > + * @num_entries: the ESRT which we obtain the filled size in bytes.
> > + *
> > + * Return: the number of bytes an ESRT with @num_entries occupies in memory.
> > + */
> > +static
> > +inline u32 efi_esrt_entries_to_size(u32 num_entries)
> > +{
> > +	u32 esrt_size = sizeof(struct efi_system_resource_table) +
> > +		num_entries * sizeof(struct efi_system_resource_entry);
> > +
> > +	return esrt_size;
> > +}
> > +
> > +/**
> > + * efi_esrt_allocate_install() - Allocates @num_entries for the ESRT and
> > + * performs basic ESRT initialization.
> > + *
> > + * @bt	       : pointer to the boottime services structure.
> > + * @num_entries: the number of entries that the ESRT will hold.
> > + *
> > + * Return:
> > + * - pointer to the ESRT if successful.
> > + * - NULL otherwise.
> > + */
> > +static
> > +struct efi_system_resource_table *efi_esrt_allocate_install(struct efi_boot_services *bt,
> > +							    u32 num_entries)
> > +{
> > +	efi_status_t ret;
> > +	struct efi_system_resource_table *new_esrt;
> > +	u32 size = efi_esrt_entries_to_size(num_entries);
> > +
> > +	/* Allocated pages must be on the lower 32bit address space. */
> > +	new_esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
> > +
> > +	/* Reserve num_pages for ESRT */
> > +	ret = EFI_CALL(bt->allocate_pool(EFI_RUNTIME_SERVICES_DATA,
> > +					 size,
> > +					 (void **)&new_esrt));
> > +
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT("ESRT Failed to allocate memory for ESRT with %d entries (%d bytes)\n",
> > +			  num_entries, efi_esrt_entries_to_size(num_entries));
> > +
> > +		return NULL;
> > +	}
> > +
> > +	new_esrt->fw_resource_count_max = num_entries;
> > +	new_esrt->fw_resource_version = EFI_ESRT_VERSION;
> > +
> > +	/* Install the ESRT in the system configuration table. */
> > +	ret = EFI_CALL(bt->install_configuration_table(&esrt_guid, (void *)new_esrt));
> > +	if (ret != EFI_SUCCESS)
> > +		EFI_PRINT("ESRT failed to install the ESRT\n");
> > +
> > +	return new_esrt;
> > +}
> > +
> > +/**
> > + * efi_esrt_resize() - Increments the ESRT to hold an addition @inc_entries FW
> > + * image entries.
> > + *
> > + * @inc_entries: the additional number of FW image entries to add to the ESRT.
> > + *
> > + * Return:
> > + * - EFI_SUCCESS if ESRT correctly resized.
> > + * - Error code otherwise.
> > + */
> > +static efi_status_t efi_esrt_resize(u32 inc_entries)
> > +{
> > +	struct efi_boot_services *bt = systab.boottime;
> > +	struct efi_system_resource_table *old_esrt = esrt;
> > +	struct efi_system_resource_table *new_esrt;
> 
> This seems overly complex. Just dispose of the existing table and create
> a new one if either an FMP protocol is installed or CapsuleUpdate()  occurs.
> 
> > +
> > +	u32 old_max_entries = old_esrt->fw_resource_count_max;
> > +	u32 old_filled_size;
> > +	efi_status_t ret;
> > +
> > +	if (!bt) {
> > +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> > +		return EFI_NOT_READY;
> > +	}
> > +
> > +	/* Allocate and install the bigger ESRT. */
> > +	new_esrt = efi_esrt_allocate_install(bt, old_max_entries + inc_entries);
> > +	if (!esrt) {
> > +		EFI_PRINT("ESRT failed to allocate and install resized ESRT\n");
> > +		esrt = old_esrt;
> > +		return EFI_OUT_OF_RESOURCES;
> > +	}
> > +	esrt = new_esrt;
> > +
> > +	old_filled_size = efi_esrt_entries_to_size(old_esrt->fw_resource_count_max);
> > +
> > +	/* Copy the old ESRT entries onto the new table. */
> > +	memcpy(new_esrt->entries, old_esrt->entries, old_filled_size
> > +		- sizeof(struct efi_system_resource_table));
> > +
> > +	new_esrt->fw_resource_count =  old_esrt->fw_resource_count;
> > +
> > +	ret = EFI_CALL(bt->free_pool(old_esrt));
> > +	if (ret != EFI_SUCCESS) {
> > +		/*
> > +		 * This should never happen.
> > +		 *
> > +		 * if ret!= EFI_SUCCESS then old_esrt is invalid.
> > +		 */
> > +		panic("EFI ESRT: could not deallocate old ESRT at %p\n", old_esrt);
> > +	}
> > +
> > +	return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + * esrt_find_entry() - Obtain the ESRT entry for the image with GUID
> > + * @img_fw_class.
> > + *
> > + * If the img_fw_class is not yet present in the ESRT, this function
> > + * reserves the tail element of the current ESRT as the entry for that fw_class.
> > + * The number of elements in the ESRT is updated in that case.
> > + *
> > + * @img_fw_class: the GUID of the FW image which ESRT entry we want to obtain.
> > + *
> > + * Return:
> > + *  - a pointer to the ESRT entry for the image with GUID img_fw_class,
> > + *  - NULL if:
> > + *   - there is no more space in the ESRT,
> > + *   - ESRT is not initialized,
> > + *   - boot services are not present.
> > + */
> > +static
> > +struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
> > +{
> > +	u32 filled_entries;
> > +	u32 max_entries;
> > +	struct efi_system_resource_entry *entry;
> > +
> > +	if (!esrt) {
> > +		EFI_PRINT("ESRT access before initialized\n");
> > +		return NULL;
> > +	}
> > +
> > +	filled_entries = esrt->fw_resource_count;
> > +	entry = esrt->entries;
> > +
> > +	/* Check if the image with img_fw_class is already in the ESRT. */
> > +	for (u32 idx = 0; idx < filled_entries; idx++) {
> > +		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
> > +			EFI_PRINT("ESRT found entry for image %pUl at index %d\n",
> > +				  img_fw_class, idx);
> > +			return &entry[idx];
> > +		}
> > +	}
> > +
> > +	max_entries = esrt->fw_resource_count_max;
> > +	/* Since the image with img_fw_class is not present in the ESRT, check
> > +	 * if ESRT is full before appending the new entry to it. */
> > +	if (filled_entries == max_entries) {
> > +		efi_status_t ret;
> > +
> > +		/* ESRT is full, attempt to extend the ESRT entries. */
> > +		ret = efi_esrt_resize(EFI_ESRT_MIN_RESIZE_ENTIRES);
> > +		if (ret != EFI_SUCCESS) {
> > +			EFI_PRINT("ESRT full, failed to resize\n");
> > +			return NULL;
> > +		}
> > +
> > +		entry = esrt->entries;
> > +		EFI_PRINT("ESRT table resized\n");
> > +	}
> > +
> > +	/*
> > +	 * This is a new entry for a fw image, increment the element
> > +	 * number in the table and set the fw_class field.
> > +	 */
> > +	esrt->fw_resource_count++;
> > +	entry[filled_entries].fw_class = *img_fw_class;
> > +	EFI_PRINT("ESRT allocated new entry for image %pUl at index %d\n",
> > +		  img_fw_class, filled_entries);
> > +
> > +	return &entry[filled_entries];
> > +}
> > +
> > +/**
> > + * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
> > + * images in the FMP.
> > + *
> > + * @fmp: the FMP instance from which FW images are added to the ESRT
> > + *
> > + * Return:
> > + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> > + * - Error status otherwise
> > + */
> > +efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp)
> > +{
> > +	struct efi_boot_services *bt = systab.boottime;
> > +	struct efi_system_resource_entry *entry = NULL;
> > +	size_t info_size = 0;
> > +	struct efi_firmware_image_descriptor *img_info = NULL;
> > +	u32 desc_version;
> > +	u8 desc_count;
> > +	size_t desc_size;
> > +	efi_status_t ret = EFI_SUCCESS;
> > +
> > +	if (!bt) {
> > +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> > +		return EFI_NOT_READY;
> > +	}
> > +
> > +	/*
> > +	 * TODO: set the field image_type depending on the FW image type
> > +	 * defined in a platform basis.
> > +	 */
> > +	u32 image_type = ESRT_FW_TYPE_UNKNOWN;
> > +
> > +	/* TODO: set the capsule flags as a function of the FW image type. */
> > +	u32 flags = 0;
> > +
> > +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> > +			&desc_version, &desc_count,
> > +			&desc_size, NULL, NULL);
> > +
> > +	if (ret != EFI_BUFFER_TOO_SMALL) {
> > +		/*
> > +		 * An input of info_size=0 should always lead
> > +		 * fmp->get_image_info to return BUFFER_TO_SMALL.
> > +		 */
> > +		EFI_PRINT("Erroneous FMP implementation\n");
> > +		return EFI_INVALID_PARAMETER;
> > +	}
> > +
> > +	ret = EFI_CALL(bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
> > +					 (void **)&img_info));
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT("ESRT failed to allocate memory for image info.\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> > +			&desc_version, &desc_count,
> > +			&desc_size, NULL, NULL);
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT("ESRT failed to obtain the FMP image info\n");
> > +		goto out;
> > +	}
> > +
> > +	/*
> > +	 * Iterate over all the FW images in the FMP.
> > +	 */
> > +	for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
> > +		struct efi_firmware_image_descriptor *cur_img_info =
> > +			(struct efi_firmware_image_descriptor *)
> > +			((uintptr_t)img_info + desc_idx * desc_size);
> > +
> > +		/*
> > +		 * Obtain the ESRT entry for the FW image with fw_class
> > +		 * equal to cur_img_info->image_type_id.
> > +		 */
> > +		entry = esrt_find_entry(&cur_img_info->image_type_id);
> > +
> > +		if (entry) {
> > +			efi_esrt_image_info_to_entry(cur_img_info, entry,
> > +						     desc_version, image_type,
> > +						     flags);
> > +		} else {
> > +			EFI_PRINT("ESRT failed to add entry for %pUl\n",
> > +				  &cur_img_info->image_type_id);
> > +			continue;
> > +		}
> > +	}
> > +
> > +out:
> > +	EFI_CALL(bt->free_pool(img_info));
> > +	return EFI_SUCCESS;
> > +}
> > +
> > +static void EFIAPI efi_esrt_new_fmp_notify(struct efi_event *event,
> > +					   void *context)
> > +{
> > +	efi_status_t ret;
> > +	efi_handle_t handle = NULL;
> > +	struct efi_firmware_management_protocol *fmp;
> > +	struct efi_boot_services *bt = systab.boottime;
> > +	size_t handle_buff_size = sizeof(handle);
> > +
> > +	if (!bt) {
> > +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> > +		return;
> > +	}
> > +
> > +	/* Obtain a single handle of the FMP type. */
> > +	ret = EFI_CALL(bt->locate_handle(BY_PROTOCOL,
> > +					 &efi_guid_firmware_management_protocol,
> > +					 NULL,
> > +					 &handle_buff_size,
> > +					 &handle
> > +					 ));
> > +	if (ret != EFI_SUCCESS) {
> 
> There may be many handles for the FMP protocol. I would expect a handler
> for EFI_BUFFER_TOO_SMALL here.
> 
> It is easier to use LocateHandleBuffer() to get all FMP protocols.
> 
> As said is wourd prefer replacing the complete ESRT when
> efi_esrt_new_fmp_notify() is called instead of incremental changes.
> 
> Best regards
> 
> Heinrich
> 
> > +		EFI_PRINT("ESRT cannot find FMP handle\n");
> > +		return;
> > +	}
> > +
> > +	/* Translate the received handle to a FMP object. */
> > +	ret = EFI_CALL(bt->handle_protocol(handle,
> > +					   &efi_guid_firmware_management_protocol,
> > +					   (void **)&fmp));
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT(" ESRT cannot obtain FMP\n");
> > +		return;
> > +	}
> > +
> > +	/* Add all the FW images managed by FMP into the ESRT. */
> > +	ret = efi_esrt_add_from_fmp(fmp);
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT("ESRT failed to populate ESRT entry\n");
> > +		return;
> > +	}
> > +}
> > +
> > +/**
> > + * efi_esrt_register() - Install the ESRT system table.
> > + *
> > + * Return: status code
> > + */
> > +efi_status_t efi_esrt_register(void)
> > +{
> > +	struct efi_boot_services *bt = systab.boottime;
> > +	struct efi_event *ev = NULL;
> > +	void *registration;
> > +	u32 num_entries = EFI_ESRT_MIN_RESIZE_ENTIRES;
> > +	efi_status_t ret;
> > +
> > +	if (!bt) {
> > +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> > +		return EFI_NOT_READY;
> > +	}
> > +
> > +	EFI_PRINT("ESRT creation start\n");
> > +
> > +	esrt = efi_esrt_allocate_install(bt, num_entries);
> > +	if (!esrt) {
> > +		EFI_PRINT("ESRT failed to initialize ESRT\n");
> > +		return EFI_NOT_READY;
> > +	}
> > +
> > +	ret = EFI_CALL(bt->create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> > +					efi_esrt_new_fmp_notify, NULL, &ev));
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT("ESRT failed to create event\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = EFI_CALL(bt->register_protocol_notify(&efi_guid_firmware_management_protocol,
> > +						    ev, &registration));
> > +	if (ret != EFI_SUCCESS) {
> > +		EFI_PRINT("ESRT failed to register FMP callback\n");
> > +		return ret;
> > +	}
> > +
> > +	EFI_PRINT("ESRT table created\n");
> > +
> > +	return ret;
> > +}
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 5800cbf6d4..0183abd09e 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;
> >
> > +#ifdef CONFIG_EFI_ESRT
> > +	ret = efi_esrt_register();
> 
> Please, add a comment that this function must be called before
> efi_launch_capsules().
> 
> Your code does not work for CONFIG_EFI_CAPSULE_ON_DISK_EARLY=y because
> efi_launch_capsules() is called before efi_init_obj_list().
> 
> @Sughosh
> 
> I think EFI_CAPSULE_ON_DISK_EARLY has to be corrected first.
> arch_efi_load_capsule_drivers() tries to install protocols on the root
> handle before the root handle has been created.
> 
> Best regards
> 
> Heinrich
> 
> > +	if (ret != EFI_SUCCESS)
> > +		goto out;
> > +#endif
> > +
> >  	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> >  		ret = efi_tcg2_register();
> >  		if (ret != EFI_SUCCESS)
> >
> 

[PATCH 2/2] efi: ESRT cration unit test

[Jose Marinho]

This commit exercises the ESRT creation -- introduced in the previous
commit.

A fake FMP, controlling TEST_ESRT_NUM_ENTRIES, is installed in the system
leading to the corresponding ESRT entries being populated.
The ESRT entries are checked against the FMP initialization input
datastructure.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>

CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
CC: Sughosh Ganu <sughosh.ganu@linaro.org>
CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
CC: Andre Przywara <andre.przywara@arm.com>
CC: Alexander Graf <agraf@csgraf.de>
CC: nd at arm.com

---
 test/lib/Makefile   |   1 +
 test/lib/efi_esrt.c | 188 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 189 insertions(+)
 create mode 100644 test/lib/efi_esrt.c

diff --git a/test/lib/Makefile b/test/lib/Makefile
index 97c11e35a8..687e791db0 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_UT_LIB_ASN1) += asn1.o
 obj-$(CONFIG_UT_LIB_RSA) += rsa.o
 obj-$(CONFIG_AES) += test_aes.o
 obj-$(CONFIG_GETOPT) += getopt.o
+obj-y += efi_esrt.o
diff --git a/test/lib/efi_esrt.c b/test/lib/efi_esrt.c
new file mode 100644
index 0000000000..3c9d1de731
--- /dev/null
+++ b/test/lib/efi_esrt.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  Test ESRT tables support
+ *
+ *  Copyright (C) 2021 Arm Ltd.
+ */
+#include <common.h>
+#include <efi_loader.h>
+#include <net.h>
+#include <test/lib.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+#define TEST_ESRT_NUM_ENTRIES 255
+
+#if 0x100 < TEST_ESRT_NUM_ENTRIES
+#error TEST_ESRT_NUM_ENTRIES must be lower or equal to 255.
+#endif
+
+static
+struct efi_firmware_image_descriptor static_img_info[TEST_ESRT_NUM_ENTRIES];
+static const efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
+
+static void efi_test_esrt_init_info(void)
+{
+	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++) {
+		static_img_info[idx].image_index = 1;
+
+		// Note: the 16 byte value present in
+		// static_img_info[idx].image_type_id is not strictly a GUID.
+		// The value is used for the sake of code testing.
+		static_img_info[idx].image_type_id.b[0] = idx;
+
+		static_img_info[idx].image_id = 0;
+		static_img_info[idx].image_id_name = NULL;
+		static_img_info[idx].version = 0;
+		static_img_info[idx].version_name = NULL;
+		static_img_info[idx].size = 0;
+		static_img_info[idx].lowest_supported_image_version = 1;
+		static_img_info[idx].last_attempt_version = 2;
+		static_img_info[idx].last_attempt_status = 3;
+		static_img_info[idx].hardware_instance = 1;
+	}
+}
+
+static efi_status_t
+EFIAPI efi_test_fmp_get_image_info(struct efi_firmware_management_protocol *this,
+				   efi_uintn_t *image_info_size,
+				   struct efi_firmware_image_descriptor *image_info,
+				   u32 *descriptor_version,
+				   u8 *descriptor_count,
+				   efi_uintn_t *descriptor_size,
+				   u32 *package_version,
+				   u16 **package_version_name)
+{
+	efi_status_t ret = EFI_SUCCESS;
+
+	EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this,
+		  image_info_size, image_info,
+		  descriptor_version, descriptor_count, descriptor_size,
+		  package_version, package_version_name);
+
+	if (!image_info_size)
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+	if (descriptor_version)
+		*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
+	if (descriptor_count)
+		*descriptor_count = TEST_ESRT_NUM_ENTRIES;
+	if (descriptor_size)
+		*descriptor_size = sizeof(*image_info);
+	if (package_version)
+		*package_version = 0xffffffff;
+	if (package_version_name)
+		*package_version_name = NULL;
+
+	if (*image_info_size < sizeof(*image_info)) {
+		*image_info_size = *descriptor_size * *descriptor_count;
+		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+	}
+
+	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
+		image_info[idx] = static_img_info[idx];
+
+	return EFI_EXIT(ret);
+}
+
+struct efi_firmware_management_protocol efi_test_fmp = {
+	.get_image_info = efi_test_fmp_get_image_info,
+	.get_image = NULL,
+	.set_image = NULL,
+	.check_image = NULL,
+	.get_package_info = NULL,
+	.set_package_info = NULL,
+};
+
+static void *lib_test_get_esrt(void)
+{
+	for (int idx = 0; idx < systab.nr_tables; idx++)
+		if (!guidcmp(&esrt_guid, &systab.tables[idx].guid))
+			return systab.tables[idx].table;
+
+	return NULL;
+}
+
+static bool lib_test_check_uuid_entry(struct efi_system_resource_table *esrt,
+				      struct efi_firmware_image_descriptor
+				      *img_info)
+{
+	const u32 filled_entries = esrt->fw_resource_count;
+	struct efi_system_resource_entry *entry = esrt->entries;
+
+	for (u32 idx = 0; idx < filled_entries; idx++) {
+		if (!guidcmp(&entry[idx].fw_class, &img_info->image_type_id)) {
+			if (entry[idx].fw_version != img_info->version)
+				return false;
+
+			if (entry[idx].lowest_supported_fw_version !=
+				img_info->lowest_supported_image_version)
+				return false;
+
+			if (entry[idx].last_attempt_version !=
+				img_info->last_attempt_version)
+				return false;
+
+			if (entry[idx].last_attempt_status !=
+				img_info->last_attempt_status)
+				return false;
+
+			/*
+			 * The entry with fw_class = img_uuid matches with the
+			 * remainder fmp input.
+			 */
+			return true;
+		}
+	}
+
+	/* There exists no entry with fw_class equal to img_uuid in the ESRT. */
+	return false;
+}
+
+int lib_test_efi_esrt(struct unit_test_state *uts)
+{
+	struct efi_system_resource_table *esrt;
+	efi_status_t ret = EFI_SUCCESS;
+	struct efi_boot_services *bt;
+
+	efi_test_esrt_init_info();
+
+	ret = efi_init_obj_list();
+
+	bt = systab.boottime;
+	ut_asserteq(ret, EFI_SUCCESS);
+
+	assert(bt);
+
+	ret = EFI_CALL(bt->install_multiple_protocol_interfaces
+		(&efi_root,
+		 &efi_guid_firmware_management_protocol,
+		 &efi_test_fmp,
+		 NULL));
+
+	ut_asserteq(ret, EFI_SUCCESS);
+
+	esrt = lib_test_get_esrt();
+	ut_assert(esrt);
+
+	ut_asserteq(esrt->fw_resource_count, TEST_ESRT_NUM_ENTRIES);
+
+	/* Add the same FMP. */
+	efi_esrt_add_from_fmp(&efi_test_fmp);
+
+	/* Verify that the number of images remains the same. */
+	ut_asserteq(esrt->fw_resource_count, TEST_ESRT_NUM_ENTRIES);
+
+	for (u32 idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
+		ut_assert(lib_test_check_uuid_entry(esrt, &static_img_info[idx]));
+
+	ret = EFI_CALL(bt->uninstall_multiple_protocol_interfaces
+		(efi_root, &efi_guid_firmware_management_protocol,
+		 &efi_test_fmp, NULL));
+
+	ut_asserteq(ret, EFI_SUCCESS);
+
+	return ret;
+}
+
+LIB_TEST(lib_test_efi_esrt, 0);
-- 
2.17.1

[PATCH 2/2] efi: ESRT cration unit test

[AKASHI Takahiro]

On Mon, Feb 08, 2021 at 12:52:46PM +0000, Jose Marinho wrote:
> This commit exercises the ESRT creation -- introduced in the previous
> commit.
> 
> A fake FMP, controlling TEST_ESRT_NUM_ENTRIES, is installed in the system
> leading to the corresponding ESRT entries being populated.
> The ESRT entries are checked against the FMP initialization input
> datastructure.

I hope that you will also provide a pytest to make sure that
an entry in ESRT is correctly updated after capsule update.

There are already some tests for capsule update.

-Takahiro Akashi


> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> 
> CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
> 
> ---
>  test/lib/Makefile   |   1 +
>  test/lib/efi_esrt.c | 188 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 189 insertions(+)
>  create mode 100644 test/lib/efi_esrt.c
> 
> diff --git a/test/lib/Makefile b/test/lib/Makefile
> index 97c11e35a8..687e791db0 100644
> --- a/test/lib/Makefile
> +++ b/test/lib/Makefile
> @@ -15,3 +15,4 @@ obj-$(CONFIG_UT_LIB_ASN1) += asn1.o
>  obj-$(CONFIG_UT_LIB_RSA) += rsa.o
>  obj-$(CONFIG_AES) += test_aes.o
>  obj-$(CONFIG_GETOPT) += getopt.o
> +obj-y += efi_esrt.o
> diff --git a/test/lib/efi_esrt.c b/test/lib/efi_esrt.c
> new file mode 100644
> index 0000000000..3c9d1de731
> --- /dev/null
> +++ b/test/lib/efi_esrt.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *  Test ESRT tables support
> + *
> + *  Copyright (C) 2021 Arm Ltd.
> + */
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <net.h>
> +#include <test/lib.h>
> +#include <test/test.h>
> +#include <test/ut.h>
> +
> +#define TEST_ESRT_NUM_ENTRIES 255
> +
> +#if 0x100 < TEST_ESRT_NUM_ENTRIES
> +#error TEST_ESRT_NUM_ENTRIES must be lower or equal to 255.
> +#endif
> +
> +static
> +struct efi_firmware_image_descriptor static_img_info[TEST_ESRT_NUM_ENTRIES];
> +static const efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
> +
> +static void efi_test_esrt_init_info(void)
> +{
> +	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++) {
> +		static_img_info[idx].image_index = 1;
> +
> +		// Note: the 16 byte value present in
> +		// static_img_info[idx].image_type_id is not strictly a GUID.
> +		// The value is used for the sake of code testing.
> +		static_img_info[idx].image_type_id.b[0] = idx;
> +
> +		static_img_info[idx].image_id = 0;
> +		static_img_info[idx].image_id_name = NULL;
> +		static_img_info[idx].version = 0;
> +		static_img_info[idx].version_name = NULL;
> +		static_img_info[idx].size = 0;
> +		static_img_info[idx].lowest_supported_image_version = 1;
> +		static_img_info[idx].last_attempt_version = 2;
> +		static_img_info[idx].last_attempt_status = 3;
> +		static_img_info[idx].hardware_instance = 1;
> +	}
> +}
> +
> +static efi_status_t
> +EFIAPI efi_test_fmp_get_image_info(struct efi_firmware_management_protocol *this,
> +				   efi_uintn_t *image_info_size,
> +				   struct efi_firmware_image_descriptor *image_info,
> +				   u32 *descriptor_version,
> +				   u8 *descriptor_count,
> +				   efi_uintn_t *descriptor_size,
> +				   u32 *package_version,
> +				   u16 **package_version_name)
> +{
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this,
> +		  image_info_size, image_info,
> +		  descriptor_version, descriptor_count, descriptor_size,
> +		  package_version, package_version_name);
> +
> +	if (!image_info_size)
> +		return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> +	if (descriptor_version)
> +		*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> +	if (descriptor_count)
> +		*descriptor_count = TEST_ESRT_NUM_ENTRIES;
> +	if (descriptor_size)
> +		*descriptor_size = sizeof(*image_info);
> +	if (package_version)
> +		*package_version = 0xffffffff;
> +	if (package_version_name)
> +		*package_version_name = NULL;
> +
> +	if (*image_info_size < sizeof(*image_info)) {
> +		*image_info_size = *descriptor_size * *descriptor_count;
> +		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> +	}
> +
> +	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
> +		image_info[idx] = static_img_info[idx];
> +
> +	return EFI_EXIT(ret);
> +}
> +
> +struct efi_firmware_management_protocol efi_test_fmp = {
> +	.get_image_info = efi_test_fmp_get_image_info,
> +	.get_image = NULL,
> +	.set_image = NULL,
> +	.check_image = NULL,
> +	.get_package_info = NULL,
> +	.set_package_info = NULL,
> +};
> +
> +static void *lib_test_get_esrt(void)
> +{
> +	for (int idx = 0; idx < systab.nr_tables; idx++)
> +		if (!guidcmp(&esrt_guid, &systab.tables[idx].guid))
> +			return systab.tables[idx].table;
> +
> +	return NULL;
> +}
> +
> +static bool lib_test_check_uuid_entry(struct efi_system_resource_table *esrt,
> +				      struct efi_firmware_image_descriptor
> +				      *img_info)
> +{
> +	const u32 filled_entries = esrt->fw_resource_count;
> +	struct efi_system_resource_entry *entry = esrt->entries;
> +
> +	for (u32 idx = 0; idx < filled_entries; idx++) {
> +		if (!guidcmp(&entry[idx].fw_class, &img_info->image_type_id)) {
> +			if (entry[idx].fw_version != img_info->version)
> +				return false;
> +
> +			if (entry[idx].lowest_supported_fw_version !=
> +				img_info->lowest_supported_image_version)
> +				return false;
> +
> +			if (entry[idx].last_attempt_version !=
> +				img_info->last_attempt_version)
> +				return false;
> +
> +			if (entry[idx].last_attempt_status !=
> +				img_info->last_attempt_status)
> +				return false;
> +
> +			/*
> +			 * The entry with fw_class = img_uuid matches with the
> +			 * remainder fmp input.
> +			 */
> +			return true;
> +		}
> +	}
> +
> +	/* There exists no entry with fw_class equal to img_uuid in the ESRT. */
> +	return false;
> +}
> +
> +int lib_test_efi_esrt(struct unit_test_state *uts)
> +{
> +	struct efi_system_resource_table *esrt;
> +	efi_status_t ret = EFI_SUCCESS;
> +	struct efi_boot_services *bt;
> +
> +	efi_test_esrt_init_info();
> +
> +	ret = efi_init_obj_list();
> +
> +	bt = systab.boottime;
> +	ut_asserteq(ret, EFI_SUCCESS);
> +
> +	assert(bt);
> +
> +	ret = EFI_CALL(bt->install_multiple_protocol_interfaces
> +		(&efi_root,
> +		 &efi_guid_firmware_management_protocol,
> +		 &efi_test_fmp,
> +		 NULL));
> +
> +	ut_asserteq(ret, EFI_SUCCESS);
> +
> +	esrt = lib_test_get_esrt();
> +	ut_assert(esrt);
> +
> +	ut_asserteq(esrt->fw_resource_count, TEST_ESRT_NUM_ENTRIES);
> +
> +	/* Add the same FMP. */
> +	efi_esrt_add_from_fmp(&efi_test_fmp);
> +
> +	/* Verify that the number of images remains the same. */
> +	ut_asserteq(esrt->fw_resource_count, TEST_ESRT_NUM_ENTRIES);
> +
> +	for (u32 idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
> +		ut_assert(lib_test_check_uuid_entry(esrt, &static_img_info[idx]));
> +
> +	ret = EFI_CALL(bt->uninstall_multiple_protocol_interfaces
> +		(efi_root, &efi_guid_firmware_management_protocol,
> +		 &efi_test_fmp, NULL));
> +
> +	ut_asserteq(ret, EFI_SUCCESS);
> +
> +	return ret;
> +}
> +
> +LIB_TEST(lib_test_efi_esrt, 0);
> -- 
> 2.17.1
> 

[PATCHv2 0/2] Add ESRT and test ESRT creation

[Jose Marinho]

The following 2 commits add the ESRT and provide a test of the
functionality.

The first commit adds the ESRT as defined in the UEFI 2.8 specification.
An empty ESRT is created during the execution of the efi_init_obj_list().
The ESRT is updated when:
  1) a FMP protocol is installed in the system: this will add the
corresponding entries to the ESRT.
  2) a capsule is installed via UpdateCapsule: this should update
entries already present in the ESRT.

This implementation of the ESRT creation takes input from FMP only.
It is assumed that the FMP will maintain the following values across
reboot:
 - LastAttemptVersion.
 - LastAttemptStatus.


The second commit enables testing the ESRT creation in the sandbox
platform. That commit is composed of 2 tests.
- Test 1 executes from the u-boot shell with "ut lib".
- Test 2 executes in the pytest environment.


Patch v2:
- The ESRT is now regenerated from scratch at every FMP EVT_NOTIFY_SIGNAL
  and whenever a capsule is updated.
- Extended TestEfiCapsuleFirmwareFit::test_efi_capsule_fw3 to verify
  that the ESRT is correctly populated after an UpdateCapsule.
- Addressed v1 comments.

Patch v1:
- reworked the ESRT creation code, allowing table to resize as
FMPs are installed.
- registered a callback for the FMP protocol install.
- Created a unit test running on the sandbox platform.

rfc: initial patch submission

CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
CC: Sughosh Ganu <sughosh.ganu@linaro.org>
CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
CC: Andre Przywara <andre.przywara@arm.com>
CC: Alexander Graf <agraf@csgraf.de>
CC: nd at arm.com

Jose Marinho (2):
  efi: Add ESRT to the EFI system table
  efi: ESRT cration unit test

 cmd/efidebug.c                |   4 +
 include/efi_api.h             |  21 ++
 include/efi_loader.h          |  19 ++
 lib/efi_loader/Kconfig        |   7 +
 lib/efi_loader/Makefile       |   1 +
 lib/efi_loader/efi_boottime.c |   2 -
 lib/efi_loader/efi_capsule.c  |   7 +
 lib/efi_loader/efi_esrt.c     | 439 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c    |   6 +
 test/lib/Makefile             |   1 +
 test/lib/efi_esrt.c           | 188 +++++++++++++++
 11 files changed, 693 insertions(+), 2 deletions(-)
 create mode 100644 lib/efi_loader/efi_esrt.c
 create mode 100644 test/lib/efi_esrt.c

-- 
2.17.1

[PATCH 1/2] efi: Add ESRT to the EFI system table

[Jose Marinho]

The ESRT is initialised during efi_init_objlist after
efi_initialize_system_table().

The ESRT is initially created with size for 50 FW image entries.
The ESRT is resized when it runs out of space. Every resize adds 50
additional entries.
The ESRT is populated from information provided by FMP instances only.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>

CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
CC: Sughosh Ganu <sughosh.ganu@linaro.org>
CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
CC: Andre Przywara <andre.przywara@arm.com>
CC: Alexander Graf <agraf@csgraf.de>
CC: nd at arm.com

---
 cmd/efidebug.c               |   4 +
 include/efi_api.h            |  21 ++
 include/efi_loader.h         |  20 ++
 lib/efi_loader/Kconfig       |   7 +
 lib/efi_loader/Makefile      |   1 +
 lib/efi_loader/efi_capsule.c |   8 +
 lib/efi_loader/efi_esrt.c    | 522 +++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c   |   6 +
 8 files changed, 589 insertions(+)
 create mode 100644 lib/efi_loader/efi_esrt.c

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index bbbcb0a546..a7dace2f80 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -459,6 +459,10 @@ static const struct {
 		"Block IO",
 		EFI_BLOCK_IO_PROTOCOL_GUID,
 	},
+	{
+		"EFI System Resource Table",
+		EFI_SYSTEM_RESOURCE_TABLE_GUID,
+	},
 	{
 		"Simple File System",
 		EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
diff --git a/include/efi_api.h b/include/efi_api.h
index 48e48a6263..fb53637419 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -1722,6 +1722,23 @@ struct efi_load_file_protocol {
 					 void *buffer);
 };
 
+struct efi_system_resource_entry {
+	efi_guid_t fw_class;
+	u32 fw_type;
+	u32 fw_version;
+	u32 lowest_supported_fw_version;
+	u32 capsule_flags;
+	u32 last_attempt_version;
+	u32 last_attempt_status;
+} __packed;
+
+struct efi_system_resource_table {
+	u32 fw_resource_count;
+	u32 fw_resource_count_max;
+	u64 fw_resource_version;
+	struct efi_system_resource_entry entries[];
+} __packed;
+
 /* Boot manager load options */
 #define LOAD_OPTION_ACTIVE		0x00000001
 #define LOAD_OPTION_FORCE_RECONNECT	0x00000002
@@ -1740,6 +1757,10 @@ struct efi_load_file_protocol {
 #define ESRT_FW_TYPE_DEVICEFIRMWARE	0x00000002
 #define ESRT_FW_TYPE_UEFIDRIVER		0x00000003
 
+#define EFI_SYSTEM_RESOURCE_TABLE_GUID\
+	EFI_GUID(0xb122a263, 0x3661, 0x4f68,\
+		0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80)
+
 /* Last Attempt Status Values */
 #define LAST_ATTEMPT_STATUS_SUCCESS			0x00000000
 #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL		0x00000001
diff --git a/include/efi_loader.h b/include/efi_loader.h
index f470bbd636..c2720f2823 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -214,6 +214,8 @@ extern const efi_guid_t efi_guid_rng_protocol;
 extern const efi_guid_t efi_guid_capsule_report;
 /* GUID of firmware management protocol */
 extern const efi_guid_t efi_guid_firmware_management_protocol;
+/* GUID for the ESRT */
+extern const efi_guid_t efi_esrt_guid;
 
 extern unsigned int __efi_runtime_start, __efi_runtime_stop;
 extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
@@ -884,4 +886,22 @@ static inline efi_status_t efi_launch_capsules(void)
 
 #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
 
+/**
+ * Install the ESRT system table.
+ *
+ * @return	status code
+ */
+efi_status_t efi_esrt_register(void);
+
+/**
+ * efi_esrt_populate() - Populates the ESRT entries from the FMP instances
+ * present in the system.
+ * If an ESRT already exists, the old ESRT is replaced in the system table.
+ * The memory of the old ESRT is deallocated.
+ *
+ * Return:
+ * - EFI_SUCCESS if the ESRT is correctly created
+ * - error code otherwise.
+ */
+efi_status_t efi_esrt_populate(void);
 #endif /* _EFI_LOADER_H */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e729f727df..a96014ce18 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -347,4 +347,11 @@ config EFI_SECURE_BOOT
 	  it is signed with a trusted key. To do that, you need to install,
 	  at least, PK, KEK and db.
 
+config EFI_ESRT
+	bool "Enable the UEFI ESRT generation"
+	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
+	default y
+	help
+	  Enabling this option creates the ESRT UEFI system table.
+
 endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 10b42e8847..9a8127846f 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -52,6 +52,7 @@ obj-y += efi_variable.o
 obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
 endif
 obj-y += efi_watchdog.o
+obj-$(CONFIG_EFI_ESRT) += efi_esrt.o
 obj-$(CONFIG_LCD) += efi_gop.o
 obj-$(CONFIG_DM_VIDEO) += efi_gop.o
 obj-$(CONFIG_PARTITIONS) += efi_disk.o
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index b57f0302c5..a1a69e619d 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -482,6 +482,14 @@ efi_status_t EFIAPI efi_update_capsule(
 			goto out;
 	}
 out:
+
+	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
+		/* Rebuild the ESRT to reflect any updated FW images. */
+		ret = EFI_CALL(efi_esrt_populate());
+		if (ret != EFI_SUCCESS)
+			log_warning("EFI Capsule: failed to update ESRT\n");
+	}
+
 	return EFI_EXIT(ret);
 }
 
diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
new file mode 100644
index 0000000000..e2d0848dc3
--- /dev/null
+++ b/lib/efi_loader/efi_esrt.c
@@ -0,0 +1,522 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  EFI application ESRT tables support
+ *
+ *  Copyright (C) 2021 Arm Ltd.
+ */
+
+#include <common.h>
+#include <efi_loader.h>
+#include <log.h>
+#include <efi_api.h>
+#include <malloc.h>
+
+const efi_guid_t efi_esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
+
+static struct efi_system_resource_table *esrt;
+
+#define EFI_ESRT_VERSION 1
+
+/**
+ * efi_esrt_image_info_to_entry() - copy the information present in a fw image
+ * descriptor to a ESRT entry.
+ * The function ensures the ESRT entry matches the image_type_id in @img_info.
+ * In case of a mismatch we leave the entry unchanged.
+ *
+ * @img_info:     the source image info descriptor
+ * @entry:        pointer to the ESRT entry to be filled
+ * @desc_version: the version of the elements in img_info
+ * @image_type:   the image type value to be set in the ESRT entry
+ * @flags:        the capsule flags value to be set in the ESRT entry
+ *
+ * Return:
+ * - EFI_SUCCESS if the entry is correctly updated
+ * - EFI_INVALID_PARAMETER if entry does not match image_type_id in @img_info.
+ */
+static efi_status_t
+efi_esrt_image_info_to_entry(struct efi_firmware_image_descriptor *img_info,
+			     struct efi_system_resource_entry *entry,
+			     u32 desc_version, u32 image_type, u32 flags)
+{
+	if (guidcmp(&entry->fw_class, &img_info->image_type_id)) {
+		EFI_PRINT("ESRT entry %pUL mismatches img_type_id %pUL\n",
+			  &entry->fw_class, &img_info->image_type_id);
+		return EFI_INVALID_PARAMETER;
+	}
+
+	entry->fw_version = img_info->version;
+
+	entry->fw_type = image_type;
+	entry->capsule_flags = flags;
+
+	/*
+	 * The field lowest_supported_image_version is only present
+	 * on image info structure of version 2 or greater.
+	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
+	 */
+	if (desc_version >= 2) {
+		entry->lowest_supported_fw_version =
+			img_info->lowest_supported_image_version;
+	} else {
+		entry->lowest_supported_fw_version = 0;
+	}
+
+	/*
+	 * The fields last_attempt_version and last_attempt_status
+	 * are only present on image info structure of version 3 or
+	 * greater.
+	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
+	 */
+	if (desc_version >= 3) {
+		entry->last_attempt_version =
+			img_info->last_attempt_version;
+
+		entry->last_attempt_status =
+			img_info->last_attempt_status;
+	} else {
+		entry->last_attempt_version = 0;
+		entry->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
+	}
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * efi_esrt_entries_to_size() - Obtain the bytes used by an ESRT
+ * datastructure with @num_entries.
+ *
+ * @num_entries: the number of entries in the ESRT.
+ *
+ * Return: the number of bytes an ESRT with @num_entries occupies in memory.
+ */
+static
+inline u32 efi_esrt_entries_to_size(u32 num_entries)
+{
+	u32 esrt_size = sizeof(struct efi_system_resource_table) +
+		num_entries * sizeof(struct efi_system_resource_entry);
+
+	return esrt_size;
+}
+
+/**
+ * efi_esrt_allocate_install() - Allocates @num_entries for the ESRT and
+ * performs basic ESRT initialization.
+ *
+ * @bt:          pointer to the boottime services structure.
+ * @num_entries: the number of entries that the ESRT will hold.
+ *
+ * Return:
+ * - pointer to the ESRT if successful.
+ * - NULL otherwise.
+ */
+static
+efi_status_t efi_esrt_allocate_install(struct efi_boot_services *bt,
+				       u32 num_entries)
+{
+	efi_status_t ret;
+	struct efi_system_resource_table *new_esrt;
+	u32 size = efi_esrt_entries_to_size(num_entries);
+	efi_guid_t esrt_guid = efi_esrt_guid;
+
+	/* Allocated pages must be on the lower 32bit address space. */
+	new_esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
+
+	/* Reserve num_pages for ESRT */
+	ret = bt->allocate_pool(EFI_BOOT_SERVICES_DATA, size,
+				(void **)&new_esrt);
+
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT cannot allocate memory for %d entries (%d bytes)\n",
+			  num_entries, efi_esrt_entries_to_size(num_entries));
+
+		return ret;
+	}
+
+	new_esrt->fw_resource_count_max = num_entries;
+	new_esrt->fw_resource_count = 0;
+	new_esrt->fw_resource_version = EFI_ESRT_VERSION;
+
+	/* Install the ESRT in the system configuration table. */
+	ret = bt->install_configuration_table(&esrt_guid, (void *)new_esrt);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to install the ESRT in the system table\n");
+		return ret;
+	}
+
+	/* If there was a previous ESRT, deallocate its memory now. */
+	if (esrt)
+		ret = bt->free_pool(esrt);
+
+	esrt = new_esrt;
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * esrt_find_entry() - Obtain the ESRT entry for the image with GUID
+ * @img_fw_class.
+ *
+ * If the img_fw_class is not yet present in the ESRT, this function
+ * reserves the tail element of the current ESRT as the entry for that fw_class.
+ * The number of elements in the ESRT is updated in that case.
+ *
+ * @img_fw_class: the GUID of the FW image which ESRT entry we want to obtain.
+ *
+ * Return:
+ *  - A pointer to the ESRT entry for the image with GUID img_fw_class,
+ *  - NULL if:
+ *   - there is no more space in the ESRT,
+ *   - ESRT is not initialized,
+ */
+static
+struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
+{
+	u32 filled_entries;
+	u32 max_entries;
+	struct efi_system_resource_entry *entry;
+
+	if (!esrt) {
+		EFI_PRINT("ESRT access before initialized\n");
+		return NULL;
+	}
+
+	filled_entries = esrt->fw_resource_count;
+	entry = esrt->entries;
+
+	/* Check if the image with img_fw_class is already in the ESRT. */
+	for (u32 idx = 0; idx < filled_entries; idx++) {
+		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
+			EFI_PRINT("ESRT found entry for image %pUl@index %d\n",
+				  img_fw_class, idx);
+			return &entry[idx];
+		}
+	}
+
+	max_entries = esrt->fw_resource_count_max;
+	/*
+	 * Since the image with img_fw_class is not present in the ESRT, check
+	 * if ESRT is full before appending the new entry to it.
+	 */
+	if (filled_entries == max_entries) {
+		EFI_PRINT("ESRT full, this should not happen\n");
+		return NULL;
+	}
+
+	/*
+	 * This is a new entry for a fw image, increment the element
+	 * number in the table and set the fw_class field.
+	 */
+	esrt->fw_resource_count++;
+	entry[filled_entries].fw_class = *img_fw_class;
+	EFI_PRINT("ESRT allocated new entry for image %pUl at index %d\n",
+		  img_fw_class, filled_entries);
+
+	return &entry[filled_entries];
+}
+
+/**
+ * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
+ * images in the FMP.
+ *
+ * @bt : pointer to the boottime services structure.
+ * @fmp: the FMP instance from which FW images are added to the ESRT
+ *
+ * Return:
+ * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
+ * - Error status otherwise
+ */
+static
+efi_status_t efi_esrt_add_from_fmp(struct efi_boot_services *bt,
+				   struct efi_firmware_management_protocol *fmp)
+{
+	struct efi_system_resource_entry *entry = NULL;
+	size_t info_size = 0;
+	struct efi_firmware_image_descriptor *img_info = NULL;
+	u32 desc_version;
+	u8 desc_count;
+	size_t desc_size;
+	u32 package_version;
+	u16 *package_version_name;
+	efi_status_t ret = EFI_SUCCESS;
+
+	/*
+	 * TODO: set the field image_type depending on the FW image type
+	 * defined in a platform basis.
+	 */
+	u32 image_type = ESRT_FW_TYPE_UNKNOWN;
+
+	/* TODO: set the capsule flags as a function of the FW image type. */
+	u32 flags = 0;
+
+	ret = fmp->get_image_info(fmp, &info_size, img_info,
+			&desc_version, &desc_count,
+			&desc_size, NULL, NULL);
+
+	if (ret != EFI_BUFFER_TOO_SMALL) {
+		/*
+		 * An input of info_size=0 should always lead
+		 * fmp->get_image_info to return BUFFER_TO_SMALL.
+		 */
+		EFI_PRINT("Erroneous FMP implementation\n");
+		return EFI_INVALID_PARAMETER;
+	}
+
+	ret = bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
+				(void **)&img_info);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to allocate memory for image info.\n");
+		return ret;
+	}
+
+	ret = fmp->get_image_info(fmp, &info_size, img_info,
+			&desc_version, &desc_count,
+			&desc_size, &package_version, &package_version_name);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to obtain the FMP image info\n");
+		goto out;
+	}
+
+	/*
+	 * Iterate over all the FW images in the FMP.
+	 */
+	for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
+		struct efi_firmware_image_descriptor *cur_img_info =
+			(struct efi_firmware_image_descriptor *)
+			((uintptr_t)img_info + desc_idx * desc_size);
+
+		/*
+		 * Obtain the ESRT entry for the FW image with fw_class
+		 * equal to cur_img_info->image_type_id.
+		 */
+		entry = esrt_find_entry(&cur_img_info->image_type_id);
+
+		if (entry) {
+			ret = efi_esrt_image_info_to_entry(cur_img_info, entry,
+							   desc_version,
+							   image_type, flags);
+			if (ret != EFI_SUCCESS)
+				EFI_PRINT("ESRT entry mismatches image_type\n");
+
+		} else {
+			EFI_PRINT("ESRT failed to add entry for %pUl\n",
+				  &cur_img_info->image_type_id);
+			continue;
+		}
+	}
+
+out:
+	bt->free_pool(img_info);
+	return EFI_SUCCESS;
+}
+
+/**
+ * efi_esrt_populate() - Populates the ESRT entries from the FMP instances
+ * present in the system.
+ * If an ESRT already exists, the old ESRT is replaced in the system table.
+ * The memory of the old ESRT is deallocated.
+ *
+ * Return:
+ * - EFI_SUCCESS if the ESRT is correctly created
+ * - error code otherwise.
+ */
+efi_status_t efi_esrt_populate(void)
+{
+	efi_handle_t *base_handle = NULL;
+	efi_handle_t *it_handle;
+	size_t no_handles = 0;
+	struct efi_firmware_management_protocol *fmp;
+	efi_status_t ret;
+	u32 num_entries = 0;
+	struct efi_boot_services *bt = systab.boottime;
+
+	if (!bt) {
+		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
+		return EFI_NOT_READY;
+	}
+
+	/*
+	 * Obtain the number of registered FMP handles.
+	 */
+	ret = bt->locate_handle_buffer(BY_PROTOCOL,
+				       &efi_guid_firmware_management_protocol,
+				       NULL, &no_handles,
+				       (efi_handle_t **)&base_handle);
+
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT There are no FMP instances\n");
+
+		ret = efi_esrt_allocate_install(bt, 0);
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT failed to create table with 0 entries\n");
+			return ret;
+		}
+		return EFI_SUCCESS;
+	}
+
+	EFI_PRINT("ESRT populate esrt from (%ld) available FMP handles\n",
+		  no_handles);
+
+	/*
+	 * Iterate over all FMPs to determine an upper bound on the number of
+	 * ESRT entries.
+	 */
+	it_handle = base_handle;
+	for (u32 idx = 0; idx < no_handles; idx++, it_handle++) {
+		struct efi_firmware_image_descriptor *img_info = NULL;
+		size_t info_size = 0;
+		u32 desc_version = 0;
+		u8 desc_count = 0;
+		size_t desc_size = 0;
+		u32 package_version;
+		u16 *package_version_name;
+
+		ret = bt->handle_protocol(*it_handle,
+					  &efi_guid_firmware_management_protocol,
+					  (void **)&fmp);
+
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT Unable to find FMP handle (%d)\n",
+				  idx);
+			goto out;
+		}
+
+		ret = fmp->get_image_info(fmp, &info_size, NULL,
+					  &desc_version, &desc_count,
+					  &desc_size, &package_version, &package_version_name);
+
+		if (ret != EFI_BUFFER_TOO_SMALL) {
+			/*
+			 * An input of info_size=0 should always lead
+			 * fmp->get_image_info to return BUFFER_TO_SMALL.
+			 */
+			EFI_PRINT("ESRT erroneous FMP implementation\n");
+			ret = EFI_INVALID_PARAMETER;
+			goto out;
+		}
+
+		ret = bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
+				(void **)&img_info);
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT failed to allocate memory for image info\n");
+			goto out;
+		}
+
+		/*
+		 * Calls to a FMP get_image_info method  do not return the
+		 * desc_count value if the return status differs from EFI_SUCCESS.
+		 * We need to repeat the call to get_image_info with a properly
+		 * sized buffer in order to obtain the real number of images
+		 * handled by the FMP.
+		 */
+		ret = fmp->get_image_info(fmp, &info_size, img_info,
+					  &desc_version, &desc_count,
+					  &desc_size, &package_version, &package_version_name);
+
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT failed to obtain image info from FMP\n");
+			bt->free_pool(img_info);
+			goto out;
+		}
+
+		num_entries += desc_count;
+
+		bt->free_pool(img_info);
+	}
+
+	EFI_PRINT("ESRT create table with %d entries\n", num_entries);
+	/*
+	 * Allocate an ESRT with the sufficient number of entries to accommodate
+	 * all the FMPs in the system.
+	 */
+	ret = efi_esrt_allocate_install(bt, num_entries);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to initialize table\n");
+		goto out;
+	}
+
+	/*
+	 * Populate the ESRT entries with any already existing FMP.
+	 */
+	it_handle = base_handle;
+	for (u32 idx = 0; idx < no_handles; idx++, it_handle++) {
+		ret = bt->handle_protocol(*it_handle,
+					  &efi_guid_firmware_management_protocol,
+					  (void **)&fmp);
+
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT unable to find FMP handle (%d)\n",
+				  idx);
+			break;
+		}
+
+		ret = efi_esrt_add_from_fmp(bt, fmp);
+		if (ret != EFI_SUCCESS)
+			EFI_PRINT("ESRT failed to add FMP to the table\n");
+	}
+
+out:
+
+	bt->free_pool(base_handle);
+
+	return ret;
+}
+
+/**
+ * efi_esrt_new_fmp_notify() - Callback for the EVT_NOTIFY_SIGNAL event raised
+ * when a new FMP protocol instance is registered in the system.
+ */
+static void EFIAPI efi_esrt_new_fmp_notify(struct efi_event *event,
+					   void *context)
+{
+	efi_status_t ret;
+
+	ret = efi_esrt_populate();
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to populate ESRT entry\n");
+		return;
+	}
+}
+
+/**
+ * efi_esrt_register() - Install the ESRT system table.
+ *
+ * Return: status code
+ */
+efi_status_t efi_esrt_register(void)
+{
+	struct efi_boot_services *bt = systab.boottime;
+	struct efi_event *ev = NULL;
+	void *registration;
+	efi_status_t ret;
+
+	if (!bt) {
+		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
+		return EFI_NOT_READY;
+	}
+
+	EFI_PRINT("ESRT creation start\n");
+
+	ret = efi_esrt_populate();
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to initiate the table\n");
+		return ret;
+	}
+
+	ret = bt->create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+			       efi_esrt_new_fmp_notify, NULL, &ev);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to create event\n");
+		return ret;
+	}
+
+	ret = bt->register_protocol_notify(&efi_guid_firmware_management_protocol,
+					   ev, &registration);
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to register FMP callback\n");
+		return ret;
+	}
+
+	EFI_PRINT("ESRT table created\n");
+
+	return ret;
+}
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index b1c5125032..3c5cf9a435 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -227,6 +227,12 @@ efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
+	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
+		ret = efi_esrt_register();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
 		ret = efi_tcg2_register();
 		if (ret != EFI_SUCCESS)
-- 
2.17.1

[PATCH 1/2] efi: Add ESRT to the EFI system table

[Ilias Apalodimas]

Hi Jose, 

On Fri, Feb 19, 2021 at 06:04:20PM +0000, Jose Marinho wrote:
> The ESRT is initialised during efi_init_objlist after
> efi_initialize_system_table().
> 
> The ESRT is initially created with size for 50 FW image entries.
> The ESRT is resized when it runs out of space. Every resize adds 50
> additional entries.
> The ESRT is populated from information provided by FMP instances only.
> 
> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> 
> CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
> 
> ---
>  cmd/efidebug.c               |   4 +
>  include/efi_api.h            |  21 ++
>  include/efi_loader.h         |  20 ++
>  lib/efi_loader/Kconfig       |   7 +
>  lib/efi_loader/Makefile      |   1 +
>  lib/efi_loader/efi_capsule.c |   8 +
>  lib/efi_loader/efi_esrt.c    | 522 +++++++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_setup.c   |   6 +
>  8 files changed, 589 insertions(+)
>  create mode 100644 lib/efi_loader/efi_esrt.c
> 
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index bbbcb0a546..a7dace2f80 100644
> --- a/cmd/efidebug.c

[...]

> +static efi_status_t
> +efi_esrt_image_info_to_entry(struct efi_firmware_image_descriptor *img_info,
> +			     struct efi_system_resource_entry *entry,
> +			     u32 desc_version, u32 image_type, u32 flags)
> +{
> +	if (guidcmp(&entry->fw_class, &img_info->image_type_id)) {
> +		EFI_PRINT("ESRT entry %pUL mismatches img_type_id %pUL\n",
> +			  &entry->fw_class, &img_info->image_type_id);
> +		return EFI_INVALID_PARAMETER;
> +	}
> +
> +	entry->fw_version = img_info->version;
> +
> +	entry->fw_type = image_type;
> +	entry->capsule_flags = flags;
> +
> +	/*
> +	 * The field lowest_supported_image_version is only present
> +	 * on image info structure of version 2 or greater.
> +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
> +	 */
> +	if (desc_version >= 2) {
> +		entry->lowest_supported_fw_version =
> +			img_info->lowest_supported_image_version;
> +	} else {
> +		entry->lowest_supported_fw_version = 0;
> +	}

You can ditch the {} here

> +
> +	/*
> +	 * The fields last_attempt_version and last_attempt_status
> +	 * are only present on image info structure of version 3 or
> +	 * greater.
> +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.

[...]

> +static
> +efi_status_t efi_esrt_allocate_install(struct efi_boot_services *bt,
> +				       u32 num_entries)
> +{
> +	efi_status_t ret;
> +	struct efi_system_resource_table *new_esrt;
> +	u32 size = efi_esrt_entries_to_size(num_entries);
> +	efi_guid_t esrt_guid = efi_esrt_guid;
> +
> +	/* Allocated pages must be on the lower 32bit address space. */
> +	new_esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;

I don't think this is needed

> +
> +	/* Reserve num_pages for ESRT */
> +	ret = bt->allocate_pool(EFI_BOOT_SERVICES_DATA, size,
> +				(void **)&new_esrt);

allocate_pool will call allocate_pages with EFI_ALLOCATE_ANY_PAGES not 
EFI_ALLOCATE_ADDRESS (which I assume it was your intention here).
So you either need to check the address after the allocation or just don't
require it to be in the lower 32bit space.

> +
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT cannot allocate memory for %d entries (%d bytes)\n",
> +			  num_entries, efi_esrt_entries_to_size(num_entries));
> +
> +		return ret;
> +	}
> +
> +	new_esrt->fw_resource_count_max = num_entries;
> +	new_esrt->fw_resource_count = 0;
> +	new_esrt->fw_resource_version = EFI_ESRT_VERSION;
> +
> +	/* Install the ESRT in the system configuration table. */
> +	ret = bt->install_configuration_table(&esrt_guid, (void *)new_esrt);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to install the ESRT in the system table\n");
> +		return ret;
> +	}
> +
> +	/* If there was a previous ESRT, deallocate its memory now. */
> +	if (esrt)
> +		ret = bt->free_pool(esrt);
> +
> +	esrt = new_esrt;
> +
> +	return EFI_SUCCESS;
> +}

[...]

Regards
/Ilias

[PATCH 1/2] efi: Add ESRT to the EFI system table

[Heinrich Schuchardt]

On 2/19/21 7:04 PM, Jose Marinho wrote:
> The ESRT is initialised during efi_init_objlist after
> efi_initialize_system_table().
>
> The ESRT is initially created with size for 50 FW image entries.
> The ESRT is resized when it runs out of space. Every resize adds 50
> additional entries.

Please, simplify the logic. Just recreate the complete table with all
entries when an FMP protocol registers or capsule updates occurs.

> The ESRT is populated from information provided by FMP instances only.
>
> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
>
> CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
>
> ---
>   cmd/efidebug.c               |   4 +
>   include/efi_api.h            |  21 ++
>   include/efi_loader.h         |  20 ++
>   lib/efi_loader/Kconfig       |   7 +
>   lib/efi_loader/Makefile      |   1 +
>   lib/efi_loader/efi_capsule.c |   8 +
>   lib/efi_loader/efi_esrt.c    | 522 +++++++++++++++++++++++++++++++++++
>   lib/efi_loader/efi_setup.c   |   6 +
>   8 files changed, 589 insertions(+)
>   create mode 100644 lib/efi_loader/efi_esrt.c
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index bbbcb0a546..a7dace2f80 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -459,6 +459,10 @@ static const struct {
>   		"Block IO",
>   		EFI_BLOCK_IO_PROTOCOL_GUID,
>   	},
> +	{
> +		"EFI System Resource Table",
> +		EFI_SYSTEM_RESOURCE_TABLE_GUID,
> +	},
>   	{
>   		"Simple File System",
>   		EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID,
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 48e48a6263..fb53637419 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -1722,6 +1722,23 @@ struct efi_load_file_protocol {
>   					 void *buffer);
>   };
>
> +struct efi_system_resource_entry {
> +	efi_guid_t fw_class;
> +	u32 fw_type;
> +	u32 fw_version;
> +	u32 lowest_supported_fw_version;
> +	u32 capsule_flags;
> +	u32 last_attempt_version;
> +	u32 last_attempt_status;
> +} __packed;
> +
> +struct efi_system_resource_table {
> +	u32 fw_resource_count;
> +	u32 fw_resource_count_max;
> +	u64 fw_resource_version;
> +	struct efi_system_resource_entry entries[];
> +} __packed;
> +
>   /* Boot manager load options */
>   #define LOAD_OPTION_ACTIVE		0x00000001
>   #define LOAD_OPTION_FORCE_RECONNECT	0x00000002
> @@ -1740,6 +1757,10 @@ struct efi_load_file_protocol {
>   #define ESRT_FW_TYPE_DEVICEFIRMWARE	0x00000002
>   #define ESRT_FW_TYPE_UEFIDRIVER		0x00000003
>
> +#define EFI_SYSTEM_RESOURCE_TABLE_GUID\
> +	EFI_GUID(0xb122a263, 0x3661, 0x4f68,\
> +		0x99, 0x29, 0x78, 0xf8, 0xb0, 0xd6, 0x21, 0x80)
> +
>   /* Last Attempt Status Values */
>   #define LAST_ATTEMPT_STATUS_SUCCESS			0x00000000
>   #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL		0x00000001
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index f470bbd636..c2720f2823 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -214,6 +214,8 @@ extern const efi_guid_t efi_guid_rng_protocol;
>   extern const efi_guid_t efi_guid_capsule_report;
>   /* GUID of firmware management protocol */
>   extern const efi_guid_t efi_guid_firmware_management_protocol;
> +/* GUID for the ESRT */
> +extern const efi_guid_t efi_esrt_guid;
>
>   extern unsigned int __efi_runtime_start, __efi_runtime_stop;
>   extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
> @@ -884,4 +886,22 @@ static inline efi_status_t efi_launch_capsules(void)
>
>   #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
>
> +/**
> + * Install the ESRT system table.
> + *
> + * @return	status code
> + */
> +efi_status_t efi_esrt_register(void);
> +
> +/**
> + * efi_esrt_populate() - Populates the ESRT entries from the FMP instances
> + * present in the system.
> + * If an ESRT already exists, the old ESRT is replaced in the system table.
> + * The memory of the old ESRT is deallocated.
> + *
> + * Return:
> + * - EFI_SUCCESS if the ESRT is correctly created
> + * - error code otherwise.
> + */
> +efi_status_t efi_esrt_populate(void);
>   #endif /* _EFI_LOADER_H */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e729f727df..a96014ce18 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -347,4 +347,11 @@ config EFI_SECURE_BOOT
>   	  it is signed with a trusted key. To do that, you need to install,
>   	  at least, PK, KEK and db.
>
> +config EFI_ESRT
> +	bool "Enable the UEFI ESRT generation"
> +	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
> +	default y
> +	help
> +	  Enabling this option creates the ESRT UEFI system table.
> +
>   endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 10b42e8847..9a8127846f 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -52,6 +52,7 @@ obj-y += efi_variable.o
>   obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
>   endif
>   obj-y += efi_watchdog.o
> +obj-$(CONFIG_EFI_ESRT) += efi_esrt.o
>   obj-$(CONFIG_LCD) += efi_gop.o
>   obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>   obj-$(CONFIG_PARTITIONS) += efi_disk.o
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index b57f0302c5..a1a69e619d 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -482,6 +482,14 @@ efi_status_t EFIAPI efi_update_capsule(
>   			goto out;
>   	}
>   out:
> +
> +	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> +		/* Rebuild the ESRT to reflect any updated FW images. */
> +		ret = EFI_CALL(efi_esrt_populate());
> +		if (ret != EFI_SUCCESS)
> +			log_warning("EFI Capsule: failed to update ESRT\n");
> +	}
> +
>   	return EFI_EXIT(ret);
>   }
>
> diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
> new file mode 100644
> index 0000000000..e2d0848dc3
> --- /dev/null
> +++ b/lib/efi_loader/efi_esrt.c
> @@ -0,0 +1,522 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *  EFI application ESRT tables support
> + *
> + *  Copyright (C) 2021 Arm Ltd.
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <log.h>
> +#include <efi_api.h>
> +#include <malloc.h>
> +
> +const efi_guid_t efi_esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
> +
> +static struct efi_system_resource_table *esrt;
> +
> +#define EFI_ESRT_VERSION 1
> +
> +/**
> + * efi_esrt_image_info_to_entry() - copy the information present in a fw image
> + * descriptor to a ESRT entry.
> + * The function ensures the ESRT entry matches the image_type_id in @img_info.
> + * In case of a mismatch we leave the entry unchanged.
> + *
> + * @img_info:     the source image info descriptor
> + * @entry:        pointer to the ESRT entry to be filled
> + * @desc_version: the version of the elements in img_info
> + * @image_type:   the image type value to be set in the ESRT entry
> + * @flags:        the capsule flags value to be set in the ESRT entry
> + *
> + * Return:
> + * - EFI_SUCCESS if the entry is correctly updated
> + * - EFI_INVALID_PARAMETER if entry does not match image_type_id in @img_info.
> + */
> +static efi_status_t
> +efi_esrt_image_info_to_entry(struct efi_firmware_image_descriptor *img_info,
> +			     struct efi_system_resource_entry *entry,
> +			     u32 desc_version, u32 image_type, u32 flags)
> +{
> +	if (guidcmp(&entry->fw_class, &img_info->image_type_id)) {
> +		EFI_PRINT("ESRT entry %pUL mismatches img_type_id %pUL\n",
> +			  &entry->fw_class, &img_info->image_type_id);
> +		return EFI_INVALID_PARAMETER;
> +	}
> +
> +	entry->fw_version = img_info->version;
> +
> +	entry->fw_type = image_type;
> +	entry->capsule_flags = flags;
> +
> +	/*
> +	 * The field lowest_supported_image_version is only present
> +	 * on image info structure of version 2 or greater.
> +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
> +	 */
> +	if (desc_version >= 2) {
> +		entry->lowest_supported_fw_version =
> +			img_info->lowest_supported_image_version;
> +	} else {
> +		entry->lowest_supported_fw_version = 0;
> +	}
> +
> +	/*
> +	 * The fields last_attempt_version and last_attempt_status
> +	 * are only present on image info structure of version 3 or
> +	 * greater.
> +	 * See the EFI_FIRMWARE_IMAGE_DESCRIPTOR definition in UEFI.
> +	 */
> +	if (desc_version >= 3) {
> +		entry->last_attempt_version =
> +			img_info->last_attempt_version;
> +
> +		entry->last_attempt_status =
> +			img_info->last_attempt_status;
> +	} else {
> +		entry->last_attempt_version = 0;
> +		entry->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
> +	}
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_esrt_entries_to_size() - Obtain the bytes used by an ESRT
> + * datastructure with @num_entries.
> + *
> + * @num_entries: the number of entries in the ESRT.
> + *
> + * Return: the number of bytes an ESRT with @num_entries occupies in memory.
> + */
> +static
> +inline u32 efi_esrt_entries_to_size(u32 num_entries)
> +{
> +	u32 esrt_size = sizeof(struct efi_system_resource_table) +
> +		num_entries * sizeof(struct efi_system_resource_entry);
> +
> +	return esrt_size;
> +}
> +
> +/**
> + * efi_esrt_allocate_install() - Allocates @num_entries for the ESRT and
> + * performs basic ESRT initialization.
> + *
> + * @bt:          pointer to the boottime services structure.
> + * @num_entries: the number of entries that the ESRT will hold.
> + *
> + * Return:
> + * - pointer to the ESRT if successful.
> + * - NULL otherwise.
> + */
> +static
> +efi_status_t efi_esrt_allocate_install(struct efi_boot_services *bt,
> +				       u32 num_entries)
> +{
> +	efi_status_t ret;
> +	struct efi_system_resource_table *new_esrt;
> +	u32 size = efi_esrt_entries_to_size(num_entries);
> +	efi_guid_t esrt_guid = efi_esrt_guid;
> +
> +	/* Allocated pages must be on the lower 32bit address space. */
> +	new_esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
> +
> +	/* Reserve num_pages for ESRT */
> +	ret = bt->allocate_pool(EFI_BOOT_SERVICES_DATA, size,
> +				(void **)&new_esrt);

The UEFI recommentds to use runtime services data.

"In general, UEFI Configuration Tables loaded at boot time (e.g., SMBIOS
table) can be contained in memory of type EfiRuntimeServicesData
(recommended), EfiBootServicesData, EfiACPIReclaimMemory or
EfiACPIMemoryNVS."

> +
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT cannot allocate memory for %d entries (%d bytes)\n",
> +			  num_entries, efi_esrt_entries_to_size(num_entries));
> +
> +		return ret;
> +	}
> +
> +	new_esrt->fw_resource_count_max = num_entries;
> +	new_esrt->fw_resource_count = 0;
> +	new_esrt->fw_resource_version = EFI_ESRT_VERSION;
> +
> +	/* Install the ESRT in the system configuration table. */
> +	ret = bt->install_configuration_table(&esrt_guid, (void *)new_esrt);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to install the ESRT in the system table\n");
> +		return ret;
> +	}
> +
> +	/* If there was a previous ESRT, deallocate its memory now. */
> +	if (esrt)
> +		ret = bt->free_pool(esrt);
> +
> +	esrt = new_esrt;
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * esrt_find_entry() - Obtain the ESRT entry for the image with GUID
> + * @img_fw_class.
> + *
> + * If the img_fw_class is not yet present in the ESRT, this function
> + * reserves the tail element of the current ESRT as the entry for that fw_class.
> + * The number of elements in the ESRT is updated in that case.
> + *
> + * @img_fw_class: the GUID of the FW image which ESRT entry we want to obtain.
> + *
> + * Return:
> + *  - A pointer to the ESRT entry for the image with GUID img_fw_class,
> + *  - NULL if:
> + *   - there is no more space in the ESRT,
> + *   - ESRT is not initialized,
> + */
> +static
> +struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
> +{
> +	u32 filled_entries;
> +	u32 max_entries;
> +	struct efi_system_resource_entry *entry; > +
> +	if (!esrt) {
> +		EFI_PRINT("ESRT access before initialized\n");
> +		return NULL;
> +	}
> +
> +	filled_entries = esrt->fw_resource_count;
> +	entry = esrt->entries;
> +
> +	/* Check if the image with img_fw_class is already in the ESRT. */
> +	for (u32 idx = 0; idx < filled_entries; idx++) {
> +		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
> +			EFI_PRINT("ESRT found entry for image %pUl at index %d\n",
> +				  img_fw_class, idx);
> +			return &entry[idx];
> +		}
> +	}
> +
> +	max_entries = esrt->fw_resource_count_max;
> +	/*
> +	 * Since the image with img_fw_class is not present in the ESRT, check
> +	 * if ESRT is full before appending the new entry to it.
> +	 */
> +	if (filled_entries == max_entries) {
> +		EFI_PRINT("ESRT full, this should not happen\n");
> +		return NULL;
> +	}
> +
> +	/*
> +	 * This is a new entry for a fw image, increment the element
> +	 * number in the table and set the fw_class field.
> +	 */
> +	esrt->fw_resource_count++;
> +	entry[filled_entries].fw_class = *img_fw_class;
> +	EFI_PRINT("ESRT allocated new entry for image %pUl at index %d\n",
> +		  img_fw_class, filled_entries);
> +
> +	return &entry[filled_entries];
> +}
> +
> +/**
> + * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
> + * images in the FMP.
> + *
> + * @bt : pointer to the boottime services structure.
> + * @fmp: the FMP instance from which FW images are added to the ESRT
> + *
> + * Return:
> + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> + * - Error status otherwise
> + */
> +static
> +efi_status_t efi_esrt_add_from_fmp(struct efi_boot_services *bt,
> +				   struct efi_firmware_management_protocol *fmp)
> +{
> +	struct efi_system_resource_entry *entry = NULL;
> +	size_t info_size = 0;
> +	struct efi_firmware_image_descriptor *img_info = NULL;
> +	u32 desc_version;
> +	u8 desc_count;
> +	size_t desc_size;
> +	u32 package_version;
> +	u16 *package_version_name;
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	/*
> +	 * TODO: set the field image_type depending on the FW image type
> +	 * defined in a platform basis.
> +	 */
> +	u32 image_type = ESRT_FW_TYPE_UNKNOWN;
> +
> +	/* TODO: set the capsule flags as a function of the FW image type. */
> +	u32 flags = 0;
> +
> +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> +			&desc_version, &desc_count,
> +			&desc_size, NULL, NULL);
> +
> +	if (ret != EFI_BUFFER_TOO_SMALL) {
> +		/*
> +		 * An input of info_size=0 should always lead
> +		 * fmp->get_image_info to return BUFFER_TO_SMALL.
> +		 */
> +		EFI_PRINT("Erroneous FMP implementation\n");
> +		return EFI_INVALID_PARAMETER;
> +	}
> +
> +	ret = bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
> +				(void **)&img_info);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to allocate memory for image info.\n");
> +		return ret;
> +	}
> +
> +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> +			&desc_version, &desc_count,
> +			&desc_size, &package_version, &package_version_name);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to obtain the FMP image info\n");
> +		goto out;
> +	}
> +
> +	/*
> +	 * Iterate over all the FW images in the FMP.
> +	 */
> +	for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
> +		struct efi_firmware_image_descriptor *cur_img_info =
> +			(struct efi_firmware_image_descriptor *)
> +			((uintptr_t)img_info + desc_idx * desc_size);
> +
> +		/*
> +		 * Obtain the ESRT entry for the FW image with fw_class
> +		 * equal to cur_img_info->image_type_id.
> +		 */
> +		entry = esrt_find_entry(&cur_img_info->image_type_id);
> +
> +		if (entry) {
> +			ret = efi_esrt_image_info_to_entry(cur_img_info, entry,
> +							   desc_version,
> +							   image_type, flags);
> +			if (ret != EFI_SUCCESS)
> +				EFI_PRINT("ESRT entry mismatches image_type\n");
> +
> +		} else {
> +			EFI_PRINT("ESRT failed to add entry for %pUl\n",
> +				  &cur_img_info->image_type_id);
> +			continue;
> +		}
> +	}
> +
> +out:
> +	bt->free_pool(img_info);
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_esrt_populate() - Populates the ESRT entries from the FMP instances
> + * present in the system.
> + * If an ESRT already exists, the old ESRT is replaced in the system table.
> + * The memory of the old ESRT is deallocated.
> + *
> + * Return:
> + * - EFI_SUCCESS if the ESRT is correctly created
> + * - error code otherwise.
> + */
> +efi_status_t efi_esrt_populate(void)
> +{
> +	efi_handle_t *base_handle = NULL;
> +	efi_handle_t *it_handle;
> +	size_t no_handles = 0;
> +	struct efi_firmware_management_protocol *fmp;
> +	efi_status_t ret;
> +	u32 num_entries = 0;
> +	struct efi_boot_services *bt = systab.boottime;
> +
> +	if (!bt) {
> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> +		return EFI_NOT_READY;
> +	}
> +
> +	/*
> +	 * Obtain the number of registered FMP handles.
> +	 */
> +	ret = bt->locate_handle_buffer(BY_PROTOCOL,
> +				       &efi_guid_firmware_management_protocol,
> +				       NULL, &no_handles,
> +				       (efi_handle_t **)&base_handle);
> +
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT There are no FMP instances\n");
> +
> +		ret = efi_esrt_allocate_install(bt, 0);
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT failed to create table with 0 entries\n");
> +			return ret;
> +		}
> +		return EFI_SUCCESS;
> +	}
> +
> +	EFI_PRINT("ESRT populate esrt from (%ld) available FMP handles\n",
> +		  no_handles);
> +
> +	/*
> +	 * Iterate over all FMPs to determine an upper bound on the number of
> +	 * ESRT entries.
> +	 */
> +	it_handle = base_handle;
> +	for (u32 idx = 0; idx < no_handles; idx++, it_handle++) {
> +		struct efi_firmware_image_descriptor *img_info = NULL;
> +		size_t info_size = 0;
> +		u32 desc_version = 0;
> +		u8 desc_count = 0;
> +		size_t desc_size = 0;
> +		u32 package_version;
> +		u16 *package_version_name;
> +
> +		ret = bt->handle_protocol(*it_handle,
> +					  &efi_guid_firmware_management_protocol,
> +					  (void **)&fmp);
> +
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT Unable to find FMP handle (%d)\n",
> +				  idx);
> +			goto out;
> +		}
> +
> +		ret = fmp->get_image_info(fmp, &info_size, NULL,
> +					  &desc_version, &desc_count,
> +					  &desc_size, &package_version, &package_version_name);
> +
> +		if (ret != EFI_BUFFER_TOO_SMALL) {
> +			/*
> +			 * An input of info_size=0 should always lead
> +			 * fmp->get_image_info to return BUFFER_TO_SMALL.
> +			 */
> +			EFI_PRINT("ESRT erroneous FMP implementation\n");
> +			ret = EFI_INVALID_PARAMETER;
> +			goto out;

This FMP protocol has a problem. Why would you skip all others?

Best regards

Heinrich

> +		}
> +
> +		ret = bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
> +				(void **)&img_info);

Why don't you simply use malloc() here?

> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT failed to allocate memory for image info\n");
> +			goto out;
> +		}
> +
> +		/*
> +		 * Calls to a FMP get_image_info method  do not return the
> +		 * desc_count value if the return status differs from EFI_SUCCESS.
> +		 * We need to repeat the call to get_image_info with a properly
> +		 * sized buffer in order to obtain the real number of images
> +		 * handled by the FMP.
> +		 */
> +		ret = fmp->get_image_info(fmp, &info_size, img_info,
> +					  &desc_version, &desc_count,
> +					  &desc_size, &package_version, &package_version_name);
> +
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT failed to obtain image info from FMP\n");
> +			bt->free_pool(img_info);
> +			goto out;

What about the other FMP protocols?

> +		}
> +
> +		num_entries += desc_count;
> +
> +		bt->free_pool(img_info);
> +	}
> +
> +	EFI_PRINT("ESRT create table with %d entries\n", num_entries);
> +	/*
> +	 * Allocate an ESRT with the sufficient number of entries to accommodate
> +	 * all the FMPs in the system.
> +	 */
> +	ret = efi_esrt_allocate_install(bt, num_entries);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to initialize table\n");
> +		goto out;
> +	}
> +
> +	/*
> +	 * Populate the ESRT entries with any already existing FMP.
> +	 */
> +	it_handle = base_handle;
> +	for (u32 idx = 0; idx < no_handles; idx++, it_handle++) {
> +		ret = bt->handle_protocol(*it_handle,
> +					  &efi_guid_firmware_management_protocol,
> +					  (void **)&fmp);
> +
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT unable to find FMP handle (%d)\n",
> +				  idx);
> +			break;
> +		}
> +
> +		ret = efi_esrt_add_from_fmp(bt, fmp);
> +		if (ret != EFI_SUCCESS)
> +			EFI_PRINT("ESRT failed to add FMP to the table\n");
> +	}
> +
> +out:
> +
> +	bt->free_pool(base_handle);
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_esrt_new_fmp_notify() - Callback for the EVT_NOTIFY_SIGNAL event raised
> + * when a new FMP protocol instance is registered in the system.
> + */
> +static void EFIAPI efi_esrt_new_fmp_notify(struct efi_event *event,
> +					   void *context)
> +{
> +	efi_status_t ret;
> +
> +	ret = efi_esrt_populate();
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to populate ESRT entry\n");
> +		return;
> +	}
> +}
> +
> +/**
> + * efi_esrt_register() - Install the ESRT system table.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_esrt_register(void)
> +{
> +	struct efi_boot_services *bt = systab.boottime;
> +	struct efi_event *ev = NULL;
> +	void *registration;
> +	efi_status_t ret;
> +
> +	if (!bt) {
> +		EFI_PRINT("ESRT cannot obtain pointer to BS\n");
> +		return EFI_NOT_READY;
> +	}
> +
> +	EFI_PRINT("ESRT creation start\n");
> +
> +	ret = efi_esrt_populate();
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to initiate the table\n");
> +		return ret;
> +	}
> +
> +	ret = bt->create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +			       efi_esrt_new_fmp_notify, NULL, &ev);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to create event\n");
> +		return ret;
> +	}
> +
> +	ret = bt->register_protocol_notify(&efi_guid_firmware_management_protocol,
> +					   ev, &registration);
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to register FMP callback\n");
> +		return ret;
> +	}
> +
> +	EFI_PRINT("ESRT table created\n");
> +
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index b1c5125032..3c5cf9a435 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -227,6 +227,12 @@ efi_status_t efi_init_obj_list(void)
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>
> +	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> +		ret = efi_esrt_register();
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
>   	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
>   		ret = efi_tcg2_register();
>   		if (ret != EFI_SUCCESS)
>