[RFC RESEND 0/2] Add ESRT and test ESRT creation

[RFC RESEND 0/2] Add ESRT and test ESRT creation

[Jose Marinho]

The following 2 commits add the ESRT and provide some intial testing.

The first commit adds the ESRT as defined in the UEFI 2.8 specification.
The ESRT is created during the execution of the efi_init_obj_list().
The ESRT will, during initialization, look for FMP instances already
present in the system and populate the corresponding FW image entries.

When a new FMP is added, via efi_add_protocol(), the
method esrt_add_from_fmp() is called to populate the corresponding ESRT
entries.

The second commit creates a fake FMP instance with the intent of testing
the code populating the ESRT entries.

CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
CC: Sughosh Ganu <sughosh.ganu@linaro.org>
CC: AKASHI Takahiro <takahiro.akashi@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: Add fake FMP instance to test ESRT creation

 include/efi_api.h             |  21 +++
 include/efi_loader.h          |  22 +++
 lib/efi_loader/Makefile       |   2 +
 lib/efi_loader/efi_boottime.c |  12 ++
 lib/efi_loader/efi_esrt.c     | 294 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_fake_fmp.c | 172 ++++++++++++++++++++
 lib/efi_loader/efi_setup.c    |   4 +
 7 files changed, 527 insertions(+)
 create mode 100644 lib/efi_loader/efi_esrt.c
 create mode 100644 lib/efi_loader/efi_fake_fmp.c

-- 
2.17.1

[RFC RESEND 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().

In this commit, the ESRT occupies at most one page.
During initialization, the list of all the already existing
FMP instances is traversed in order to fill the corresponsing entries.

Later, when any FMP is added, the corresponding ESRT entries are
initialized as long as there is available entries in the table.

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: Andre Przywara <andre.przywara@arm.com>
CC: Alexander Graf <agraf@csgraf.de>
CC: nd at arm.com
---
 include/efi_api.h             |  21 +++
 include/efi_loader.h          |  22 +++
 lib/efi_loader/Makefile       |   1 +
 lib/efi_loader/efi_boottime.c |  12 ++
 lib/efi_loader/efi_esrt.c     | 294 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c    |   4 +
 6 files changed, 354 insertions(+)
 create mode 100644 lib/efi_loader/efi_esrt.c

diff --git a/include/efi_api.h b/include/efi_api.h
index 48e48a6263..f4a88603e9 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;
+} __attribute__((__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[];
+} __attribute__((__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..7b20ec1ad2 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -884,4 +884,26 @@ 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);
+
+/**
+ * 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
+ * @image_type: the type of the FW image in the ESRT entry
+ * @flags:      the capsule flags to be set in the ESRT entry
+ *
+ * Return:
+ * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
+ * - Error status otherwise
+ */
+efi_status_t esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp,
+	u32 image_type, u32 flags);
+
 #endif /* _EFI_LOADER_H */
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..8077b467c7 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1150,6 +1150,18 @@ 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);
+
+	if (!guidcmp(&efi_guid_firmware_management_protocol, protocol)) {
+		EFI_PRINT("installed FMP, updating ESRT entries\n");
+
+		ret = esrt_add_from_fmp((struct efi_firmware_management_protocol *)
+			protocol_interface, ESRT_FW_TYPE_UNKNOWN, 0);
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("Failed to update FMP ESRT entries\n");
+			return ret;
+		}
+	}
+
 	return EFI_SUCCESS;
 }
 
diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
new file mode 100644
index 0000000000..c651ce6c73
--- /dev/null
+++ b/lib/efi_loader/efi_esrt.c
@@ -0,0 +1,294 @@
+// 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 const efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
+static bool esrt_initialized;
+static struct efi_system_resource_table *esrt;
+
+#define EFI_ESRT_NUM_PAGES 1
+
+#define EFI_ESRT_SIZE (EFI_ESRT_NUM_PAGES * EFI_PAGE_SIZE)
+
+#define EFI_ESRT_ENTRIES_MAX ((EFI_ESRT_SIZE -             \
+	sizeof(struct efi_system_resource_table)) /    \
+	sizeof(struct efi_system_resource_entry))
+
+#define EFI_ESRT_VERSION 1
+
+/**
+ * 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
+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;
+	}
+}
+
+/**
+ * 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_clas.
+ * The number of elements in the ESRT is updated in that case.
+ *
+ * @fw_img_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.
+ */
+static
+struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
+{
+	// if this function is called before the esrt init then it is erroneous
+	// behaviour.
+	assert(esrt_initialized);
+
+	const u32 filled_entries = esrt->fw_resource_count;
+	const u32 max_entries = esrt->fw_resource_count_max;
+	struct efi_system_resource_entry *entry = esrt->entries;
+
+	for (u32 idx; idx < filled_entries; idx++) {
+		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
+			log_debug("EFI ESRT: found entry for image %pUl\n",
+				  img_fw_class);
+			return &entry[idx];
+		}
+	}
+
+	if (filled_entries == max_entries) {
+		log_warning("EFI ESRT: table is full\n");
+		return NULL;
+
+	} else {
+		/*
+		 * 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;
+		return &entry[filled_entries];
+	}
+}
+
+/**
+ * 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
+ * @image_type: the type of the FW image in the ESRT entry
+ * @flags:      the capsule flags to be set in the ESRT entry
+ *
+ * Return:
+ * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
+ * - Error status otherwise
+ */
+efi_status_t esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp,
+			       u32 image_type, u32 flags)
+{
+	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 (!esrt_initialized)
+		return EFI_NOT_READY;
+
+	ret = fmp->get_image_info(fmp, &info_size, img_info,
+			&desc_version, &desc_count,
+			&desc_size, NULL, NULL);
+
+	/*
+	 * An input of info_size=0 should always lead to BUFFER_TO_SMALL return
+	 * if this is not the case then the get_image_info implementation is
+	 * anoumalous, hence the assert.
+	 */
+	assert_noisy(ret == EFI_BUFFER_TOO_SMALL);
+
+	img_info = malloc(info_size);
+
+	ret = fmp->get_image_info(fmp, &info_size, img_info,
+			&desc_version, &desc_count,
+			&desc_size, NULL, NULL);
+	if (ret != EFI_SUCCESS) {
+		log_err("EFI 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) {
+			esrt_image_info_to_entry(cur_img_info, entry,
+						 desc_version, image_type,
+						 flags);
+		} else {
+			log_err("EFI ESRT: failed to add esrt entry for %pUl\n",
+				&cur_img_info->image_type_id);
+			continue;
+		}
+	}
+
+out:
+	free(img_info);
+	return true;
+}
+
+/**
+ * esrt_populate() - Populates the ESRT entries from the FMP instances
+ * registered so far and performs basic initialization of the ESRT fields.
+ *
+ * Return:
+ * - true if ESRT initialized successfully.
+ * - false otherwise.
+ */
+static bool esrt_populate(void)
+{
+	efi_handle_t *handle = NULL;
+	size_t no_handles = 0;
+	struct efi_firmware_management_protocol *fmp;
+	efi_status_t ret;
+
+	esrt->fw_resource_count_max = EFI_ESRT_ENTRIES_MAX;
+	esrt->fw_resource_version = EFI_ESRT_VERSION;
+
+	esrt_initialized = true;
+
+	/*
+	 * Obtain the number of registered FMP handles.
+	 */
+	ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL, &efi_guid_firmware_management_protocol,
+						NULL, &no_handles,
+						(efi_handle_t **)&handle));
+
+	if (ret != EFI_SUCCESS) {
+		log_warning("EFI ESRT: failed to obtain FMP handles (%lx)\n",
+			    ret);
+		return esrt_initialized;
+	}
+
+	log_debug("EFI ESRT: populate esrt from (%ld) avaialble FMP handles\n",
+		  no_handles);
+	/*
+	 * Populate the ESRT entries with any already existing FMP.
+	 */
+	for (u32 idx = 0; idx < no_handles; idx++, handle++) {
+		ret = EFI_CALL(efi_handle_protocol(*handle, &efi_guid_firmware_management_protocol,
+						   (void **)&fmp));
+		if (ret != EFI_SUCCESS) {
+			log_err("EFI ESRT: Unable to find FMP handle (%d)\n",
+				idx);
+			continue;
+		}
+
+		ret = esrt_add_from_fmp(fmp, ESRT_FW_TYPE_UNKNOWN, 0);
+		if (ret != EFI_SUCCESS) {
+			log_err("EFI ESRT: failed to add from FMP (%d)\n", idx);
+			continue;
+		}
+	}
+	return esrt_initialized;
+}
+
+/**
+ * efi_esrt_register() - Install the ESRT system table.
+ *
+ * @return	status code
+ */
+efi_status_t efi_esrt_register(void)
+{
+	esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
+	efi_status_t ret;
+
+	log_debug("EFI ESRT: ESRT creation start\n");
+
+	/* Reserve pages for ESRT */
+	ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+				 EFI_RUNTIME_SERVICES_DATA, EFI_ESRT_NUM_PAGES,
+		(uint64_t *)&esrt);
+
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	if (!esrt_populate())
+		log_warning("EFI ESRT: failed to initialize ESRT\n");
+
+	/* And expose them to our EFI payload */
+	ret = efi_install_configuration_table(&esrt_guid, (void *)esrt);
+	if (ret != EFI_SUCCESS) {
+		log_err("EFI ESRT: failed to create ESRT\n");
+		return ret;
+	}
+
+	log_debug("EFI ESRT: table created\n");
+
+	return ret;
+}
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 5800cbf6d4..7660a20822 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -231,6 +231,10 @@ efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
+	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

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

[Heinrich Schuchardt]

On 28.01.21 14:29, Jose Marinho wrote:
> The ESRT is initialised during efi_init_objlist, after
> efi_initialize_system_table().
>
> In this commit, the ESRT occupies at most one page.
> During initialization, the list of all the already existing
> FMP instances is traversed in order to fill the corresponsing entries.
>
> Later, when any FMP is added, the corresponding ESRT entries are
> initialized as long as there is available entries in the table.
>
> 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: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
> ---
>  include/efi_api.h             |  21 +++
>  include/efi_loader.h          |  22 +++
>  lib/efi_loader/Makefile       |   1 +
>  lib/efi_loader/efi_boottime.c |  12 ++
>  lib/efi_loader/efi_esrt.c     | 294 ++++++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_setup.c    |   4 +
>  6 files changed, 354 insertions(+)
>  create mode 100644 lib/efi_loader/efi_esrt.c
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 48e48a6263..f4a88603e9 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;
> +} __attribute__((__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[];
> +} __attribute__((__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 )

Dear Joel,

thank you for addressing this missing piece in U-Boot.

Please, add the GUID to guid_list[] in cmd/efidebug.c.

> +
>  /* 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..7b20ec1ad2 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -884,4 +884,26 @@ 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);
> +
> +/**
> + * 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
> + * @image_type: the type of the FW image in the ESRT entry
> + * @flags:      the capsule flags to be set in the ESRT entry
> + *
> + * Return:
> + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> + * - Error status otherwise
> + */
> +efi_status_t esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp,
> +	u32 image_type, u32 flags);
> +
>  #endif /* _EFI_LOADER_H */
> 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..8077b467c7 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1150,6 +1150,18 @@ 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);
> +
> +	if (!guidcmp(&efi_guid_firmware_management_protocol, protocol)) {
> +		EFI_PRINT("installed FMP, updating ESRT entries\n");
> +
> +		ret = esrt_add_from_fmp((struct efi_firmware_management_protocol *)
> +			protocol_interface, ESRT_FW_TYPE_UNKNOWN, 0);
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("Failed to update FMP ESRT entries\n");
> +			return ret;
> +		}

On some systems we are very tight on memory. So we should have a
configuration setting to disable the ESRT code.

Calling RegisterProtocolNotify() offers a mor UEFI style implementation
alternative but would need more code.

> +	}
> +
>  	return EFI_SUCCESS;
>  }
>
> diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
> new file mode 100644
> index 0000000000..c651ce6c73
> --- /dev/null
> +++ b/lib/efi_loader/efi_esrt.c
> @@ -0,0 +1,294 @@
> +// 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 const efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
> +static bool esrt_initialized;
> +static struct efi_system_resource_table *esrt;
> +
> +#define EFI_ESRT_NUM_PAGES 1
> +
> +#define EFI_ESRT_SIZE (EFI_ESRT_NUM_PAGES * EFI_PAGE_SIZE)
> +
> +#define EFI_ESRT_ENTRIES_MAX ((EFI_ESRT_SIZE -             \
> +	sizeof(struct efi_system_resource_table)) /    \
> +	sizeof(struct efi_system_resource_entry))
> +
> +#define EFI_ESRT_VERSION 1
> +
> +/**
> + * 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
> +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;
> +	}
> +}
> +
> +/**
> + * esrt_find_entry() - Obtain the ESRT entry for the image with guid

%s/guid/GUID/

> + * img_fw_class.

%s/img_fw_class/@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_clas.
> + * The number of elements in the ESRT is updated in that case.
> + *
> + * @fw_img_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.
> + */
> +static
> +struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
> +{
> +	// if this function is called before the esrt init then it is erroneous
> +	// behaviour.
> +	assert(esrt_initialized);

assert is a nop if _DEBUG = 0.
Why don't you return NULL in this case?

Can this really occur?

> +
> +	const u32 filled_entries = esrt->fw_resource_count;
> +	const u32 max_entries = esrt->fw_resource_count_max;
> +	struct efi_system_resource_entry *entry = esrt->entries;
> +
> +	for (u32 idx; idx < filled_entries; idx++) {
> +		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
> +			log_debug("EFI ESRT: found entry for image %pUl\n",
> +				  img_fw_class);
> +			return &entry[idx];
> +		}
> +	}
> +
> +	if (filled_entries == max_entries) {
> +		log_warning("EFI ESRT: table is full\n");
> +		return NULL;
> +
> +	} else {
> +		/*
> +		 * 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;
> +		return &entry[filled_entries];
> +	}
> +}
> +
> +/**
> + * 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
> + * @image_type: the type of the FW image in the ESRT entry
> + * @flags:      the capsule flags to be set in the ESRT entry
> + *
> + * Return:
> + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> + * - Error status otherwise
> + */
> +efi_status_t esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp,
> +			       u32 image_type, u32 flags)
> +{
> +	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 (!esrt_initialized)
> +		return EFI_NOT_READY;

Can this really occur?

> +
> +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> +			&desc_version, &desc_count,
> +			&desc_size, NULL, NULL);
> +
> +	/*
> +	 * An input of info_size=0 should always lead to BUFFER_TO_SMALL return
> +	 * if this is not the case then the get_image_info implementation is
> +	 * anoumalous, hence the assert.

%s/anoumalous/anomalous/

> +	 */
> +	assert_noisy(ret == EFI_BUFFER_TOO_SMALL);

What do you want the user to do when he sees this message? Why would you
continue.

I guess the right thing is to return an error EFI_INVALID_PARAMETER if
the fmp protocol is not compliant. You can use EFI_PRINT() to write a
debug message.


> +
> +	img_info = malloc(info_size);

Always check the result of malloc().

> +
> +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> +			&desc_version, &desc_count,
> +			&desc_size, NULL, NULL);
> +	if (ret != EFI_SUCCESS) {
> +		log_err("EFI 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) {
> +			esrt_image_info_to_entry(cur_img_info, entry,
> +						 desc_version, image_type,
> +						 flags);
> +		} else {
> +			log_err("EFI ESRT: failed to add esrt entry for %pUl\n",
> +				&cur_img_info->image_type_id);
> +			continue;
> +		}
> +	}
> +
> +out:
> +	free(img_info);
> +	return true;

EFI_SUCCESS

> +}
> +
> +/**
> + * esrt_populate() - Populates the ESRT entries from the FMP instances
> + * registered so far and performs basic initialization of the ESRT fields.
> + *
> + * Return:
> + * - true if ESRT initialized successfully.
> + * - false otherwise.
> + */
> +static bool esrt_populate(void)
> +{
> +	efi_handle_t *handle = NULL;
> +	size_t no_handles = 0;
> +	struct efi_firmware_management_protocol *fmp;
> +	efi_status_t ret;
> +
> +	esrt->fw_resource_count_max = EFI_ESRT_ENTRIES_MAX;
> +	esrt->fw_resource_version = EFI_ESRT_VERSION;
> +
> +	esrt_initialized = true;
> +
> +	/*
> +	 * Obtain the number of registered FMP handles.
> +	 */
> +	ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL, &efi_guid_firmware_management_protocol,
> +						NULL, &no_handles,
> +						(efi_handle_t **)&handle));
> +
> +	if (ret != EFI_SUCCESS) {
> +		log_warning("EFI ESRT: failed to obtain FMP handles (%lx)\n",
> +			    ret);
> +		return esrt_initialized;
> +	}

Can't we initialize ESRT before any FMP is installed to get rid of this
function module?

> +
> +	log_debug("EFI ESRT: populate esrt from (%ld) avaialble FMP handles\n",
> +		  no_handles);
> +	/*
> +	 * Populate the ESRT entries with any already existing FMP.
> +	 */
> +	for (u32 idx = 0; idx < no_handles; idx++, handle++) {
> +		ret = EFI_CALL(efi_handle_protocol(*handle, &efi_guid_firmware_management_protocol,
> +						   (void **)&fmp));
> +		if (ret != EFI_SUCCESS) {
> +			log_err("EFI ESRT: Unable to find FMP handle (%d)\n",
> +				idx);
> +			continue;
> +		}
> +
> +		ret = esrt_add_from_fmp(fmp, ESRT_FW_TYPE_UNKNOWN, 0);
> +		if (ret != EFI_SUCCESS) {
> +			log_err("EFI ESRT: failed to add from FMP (%d)\n", idx);
> +			continue;
> +		}
> +	}
> +	return esrt_initialized;
> +}
> +
> +/**
> + * efi_esrt_register() - Install the ESRT system table.
> + *
> + * @return	status code
> + */
> +efi_status_t efi_esrt_register(void)
> +{
> +	esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
> +	efi_status_t ret;
> +
> +	log_debug("EFI ESRT: ESRT creation start\n");
> +
> +	/* Reserve pages for ESRT */
> +	ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
> +				 EFI_RUNTIME_SERVICES_DATA, EFI_ESRT_NUM_PAGES,

Do we need an ESRT before the installation of the first FMP?

Why are you using a fixed number of pages? Doesn't the size depend on
the number of FMPs? Shouldn't the table be resized whenever a new
protocol is installed?

Unfortunately you never check if you overrun the allocated space. So I
just need an EFI driver that installs a high number of FMP and it will
crash U-Boot.

So I would drop this function and move (re-)allocating the ESRT to
esrt_add_from_fmp().

Best regards

Heinrich

> +		(uint64_t *)&esrt);
> +
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	if (!esrt_populate())
> +		log_warning("EFI ESRT: failed to initialize ESRT\n");
> +
> +	/* And expose them to our EFI payload */
> +	ret = efi_install_configuration_table(&esrt_guid, (void *)esrt);
> +	if (ret != EFI_SUCCESS) {
> +		log_err("EFI ESRT: failed to create ESRT\n");
> +		return ret;
> +	}
> +
> +	log_debug("EFI ESRT: table created\n");
> +
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 5800cbf6d4..7660a20822 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -231,6 +231,10 @@ efi_status_t efi_init_obj_list(void)
>  	if (ret != EFI_SUCCESS)
>  		goto out;
>
> +	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)
>

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

[AKASHI Takahiro]

First of all, one comment:
It would be nice to have a list of what features are supported
and what are not in this patch series.
For example, with this patch, I suspect
- FwVersion in ESRT entry will never be updated at capsule update
- LastAttemptVersion/LastAttemptStatus will not be sustained across reboots.

So I'm not sure that the proposed implementation is useful
in a practical manner.

On Thu, Jan 28, 2021 at 04:29:57PM +0100, Heinrich Schuchardt wrote:
> On 28.01.21 14:29, Jose Marinho wrote:
> > The ESRT is initialised during efi_init_objlist, after
> > efi_initialize_system_table().
> >
> > In this commit, the ESRT occupies at most one page.
> > During initialization, the list of all the already existing
> > FMP instances is traversed in order to fill the corresponsing entries.
> >
> > Later, when any FMP is added, the corresponding ESRT entries are
> > initialized as long as there is available entries in the table.
> >
> > 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: Andre Przywara <andre.przywara@arm.com>
> > CC: Alexander Graf <agraf@csgraf.de>
> > CC: nd at arm.com
> > ---
> >  include/efi_api.h             |  21 +++
> >  include/efi_loader.h          |  22 +++
> >  lib/efi_loader/Makefile       |   1 +
> >  lib/efi_loader/efi_boottime.c |  12 ++
> >  lib/efi_loader/efi_esrt.c     | 294 ++++++++++++++++++++++++++++++++++
> >  lib/efi_loader/efi_setup.c    |   4 +
> >  6 files changed, 354 insertions(+)
> >  create mode 100644 lib/efi_loader/efi_esrt.c
> >
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 48e48a6263..f4a88603e9 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;
> > +} __attribute__((__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[];
> > +} __attribute__((__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 )
> 
> Dear Joel,
> 
> thank you for addressing this missing piece in U-Boot.
> 
> Please, add the GUID to guid_list[] in cmd/efidebug.c.
> 
> > +
> >  /* 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..7b20ec1ad2 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -884,4 +884,26 @@ 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);
> > +
> > +/**
> > + * 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
> > + * @image_type: the type of the FW image in the ESRT entry
> > + * @flags:      the capsule flags to be set in the ESRT entry
> > + *
> > + * Return:
> > + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> > + * - Error status otherwise
> > + */
> > +efi_status_t esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp,
> > +	u32 image_type, u32 flags);
> > +
> >  #endif /* _EFI_LOADER_H */
> > 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..8077b467c7 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -1150,6 +1150,18 @@ 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);
> > +
> > +	if (!guidcmp(&efi_guid_firmware_management_protocol, protocol)) {
> > +		EFI_PRINT("installed FMP, updating ESRT entries\n");
> > +
> > +		ret = esrt_add_from_fmp((struct efi_firmware_management_protocol *)
> > +			protocol_interface, ESRT_FW_TYPE_UNKNOWN, 0);
> > +		if (ret != EFI_SUCCESS) {
> > +			EFI_PRINT("Failed to update FMP ESRT entries\n");
> > +			return ret;
> > +		}
> 
> On some systems we are very tight on memory. So we should have a
> configuration setting to disable the ESRT code.
> 
> Calling RegisterProtocolNotify() offers a mor UEFI style implementation
> alternative but would need more code.

* I don't think that it is a good idea to add a protocol-specific
  handling in a generic function.
* efi_add_protocol() is not even a right place to call esrt_from_fmp()
  because
  - calling get_image_info() makes sense only for a specific instance
    of firmware image.
  - as I said above, FwVersion should be updated logically at every
    time when capsule update takes place.

Meanwhile, the initial entry in ESRT should be separately created at booting
depending on what "firmware" are to be provided on a specific system.

-Takahiro Akashi

> > +	}
> > +
> >  	return EFI_SUCCESS;
> >  }
> >
> > diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
> > new file mode 100644
> > index 0000000000..c651ce6c73
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_esrt.c
> > @@ -0,0 +1,294 @@
> > +// 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 const efi_guid_t esrt_guid = EFI_SYSTEM_RESOURCE_TABLE_GUID;
> > +static bool esrt_initialized;
> > +static struct efi_system_resource_table *esrt;
> > +
> > +#define EFI_ESRT_NUM_PAGES 1
> > +
> > +#define EFI_ESRT_SIZE (EFI_ESRT_NUM_PAGES * EFI_PAGE_SIZE)
> > +
> > +#define EFI_ESRT_ENTRIES_MAX ((EFI_ESRT_SIZE -             \
> > +	sizeof(struct efi_system_resource_table)) /    \
> > +	sizeof(struct efi_system_resource_entry))
> > +
> > +#define EFI_ESRT_VERSION 1
> > +
> > +/**
> > + * 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
> > +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;
> > +	}
> > +}
> > +
> > +/**
> > + * esrt_find_entry() - Obtain the ESRT entry for the image with guid
> 
> %s/guid/GUID/
> 
> > + * img_fw_class.
> 
> %s/img_fw_class/@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_clas.
> > + * The number of elements in the ESRT is updated in that case.
> > + *
> > + * @fw_img_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.
> > + */
> > +static
> > +struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
> > +{
> > +	// if this function is called before the esrt init then it is erroneous
> > +	// behaviour.
> > +	assert(esrt_initialized);
> 
> assert is a nop if _DEBUG = 0.
> Why don't you return NULL in this case?
> 
> Can this really occur?
> 
> > +
> > +	const u32 filled_entries = esrt->fw_resource_count;
> > +	const u32 max_entries = esrt->fw_resource_count_max;
> > +	struct efi_system_resource_entry *entry = esrt->entries;
> > +
> > +	for (u32 idx; idx < filled_entries; idx++) {
> > +		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
> > +			log_debug("EFI ESRT: found entry for image %pUl\n",
> > +				  img_fw_class);
> > +			return &entry[idx];
> > +		}
> > +	}
> > +
> > +	if (filled_entries == max_entries) {
> > +		log_warning("EFI ESRT: table is full\n");
> > +		return NULL;
> > +
> > +	} else {
> > +		/*
> > +		 * 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;
> > +		return &entry[filled_entries];
> > +	}
> > +}
> > +
> > +/**
> > + * 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
> > + * @image_type: the type of the FW image in the ESRT entry
> > + * @flags:      the capsule flags to be set in the ESRT entry
> > + *
> > + * Return:
> > + * - EFI_SUCCESS if all the FW images in the FMP are added to the ESRT
> > + * - Error status otherwise
> > + */
> > +efi_status_t esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp,
> > +			       u32 image_type, u32 flags)
> > +{
> > +	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 (!esrt_initialized)
> > +		return EFI_NOT_READY;
> 
> Can this really occur?
> 
> > +
> > +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> > +			&desc_version, &desc_count,
> > +			&desc_size, NULL, NULL);
> > +
> > +	/*
> > +	 * An input of info_size=0 should always lead to BUFFER_TO_SMALL return
> > +	 * if this is not the case then the get_image_info implementation is
> > +	 * anoumalous, hence the assert.
> 
> %s/anoumalous/anomalous/
> 
> > +	 */
> > +	assert_noisy(ret == EFI_BUFFER_TOO_SMALL);
> 
> What do you want the user to do when he sees this message? Why would you
> continue.
> 
> I guess the right thing is to return an error EFI_INVALID_PARAMETER if
> the fmp protocol is not compliant. You can use EFI_PRINT() to write a
> debug message.
> 
> 
> > +
> > +	img_info = malloc(info_size);
> 
> Always check the result of malloc().
> 
> > +
> > +	ret = fmp->get_image_info(fmp, &info_size, img_info,
> > +			&desc_version, &desc_count,
> > +			&desc_size, NULL, NULL);
> > +	if (ret != EFI_SUCCESS) {
> > +		log_err("EFI 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) {
> > +			esrt_image_info_to_entry(cur_img_info, entry,
> > +						 desc_version, image_type,
> > +						 flags);
> > +		} else {
> > +			log_err("EFI ESRT: failed to add esrt entry for %pUl\n",
> > +				&cur_img_info->image_type_id);
> > +			continue;
> > +		}
> > +	}
> > +
> > +out:
> > +	free(img_info);
> > +	return true;
> 
> EFI_SUCCESS
> 
> > +}
> > +
> > +/**
> > + * esrt_populate() - Populates the ESRT entries from the FMP instances
> > + * registered so far and performs basic initialization of the ESRT fields.
> > + *
> > + * Return:
> > + * - true if ESRT initialized successfully.
> > + * - false otherwise.
> > + */
> > +static bool esrt_populate(void)
> > +{
> > +	efi_handle_t *handle = NULL;
> > +	size_t no_handles = 0;
> > +	struct efi_firmware_management_protocol *fmp;
> > +	efi_status_t ret;
> > +
> > +	esrt->fw_resource_count_max = EFI_ESRT_ENTRIES_MAX;
> > +	esrt->fw_resource_version = EFI_ESRT_VERSION;
> > +
> > +	esrt_initialized = true;
> > +
> > +	/*
> > +	 * Obtain the number of registered FMP handles.
> > +	 */
> > +	ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL, &efi_guid_firmware_management_protocol,
> > +						NULL, &no_handles,
> > +						(efi_handle_t **)&handle));
> > +
> > +	if (ret != EFI_SUCCESS) {
> > +		log_warning("EFI ESRT: failed to obtain FMP handles (%lx)\n",
> > +			    ret);
> > +		return esrt_initialized;
> > +	}
> 
> Can't we initialize ESRT before any FMP is installed to get rid of this
> function module?
> 
> > +
> > +	log_debug("EFI ESRT: populate esrt from (%ld) avaialble FMP handles\n",
> > +		  no_handles);
> > +	/*
> > +	 * Populate the ESRT entries with any already existing FMP.
> > +	 */
> > +	for (u32 idx = 0; idx < no_handles; idx++, handle++) {
> > +		ret = EFI_CALL(efi_handle_protocol(*handle, &efi_guid_firmware_management_protocol,
> > +						   (void **)&fmp));
> > +		if (ret != EFI_SUCCESS) {
> > +			log_err("EFI ESRT: Unable to find FMP handle (%d)\n",
> > +				idx);
> > +			continue;
> > +		}
> > +
> > +		ret = esrt_add_from_fmp(fmp, ESRT_FW_TYPE_UNKNOWN, 0);
> > +		if (ret != EFI_SUCCESS) {
> > +			log_err("EFI ESRT: failed to add from FMP (%d)\n", idx);
> > +			continue;
> > +		}
> > +	}
> > +	return esrt_initialized;
> > +}
> > +
> > +/**
> > + * efi_esrt_register() - Install the ESRT system table.
> > + *
> > + * @return	status code
> > + */
> > +efi_status_t efi_esrt_register(void)
> > +{
> > +	esrt = (struct efi_system_resource_table *)(uintptr_t)U32_MAX;
> > +	efi_status_t ret;
> > +
> > +	log_debug("EFI ESRT: ESRT creation start\n");
> > +
> > +	/* Reserve pages for ESRT */
> > +	ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
> > +				 EFI_RUNTIME_SERVICES_DATA, EFI_ESRT_NUM_PAGES,
> 
> Do we need an ESRT before the installation of the first FMP?
> 
> Why are you using a fixed number of pages? Doesn't the size depend on
> the number of FMPs? Shouldn't the table be resized whenever a new
> protocol is installed?
> 
> Unfortunately you never check if you overrun the allocated space. So I
> just need an EFI driver that installs a high number of FMP and it will
> crash U-Boot.
> 
> So I would drop this function and move (re-)allocating the ESRT to
> esrt_add_from_fmp().
> 
> Best regards
> 
> Heinrich
> 
> > +		(uint64_t *)&esrt);
> > +
> > +	if (ret != EFI_SUCCESS)
> > +		return ret;
> > +
> > +	if (!esrt_populate())
> > +		log_warning("EFI ESRT: failed to initialize ESRT\n");
> > +
> > +	/* And expose them to our EFI payload */
> > +	ret = efi_install_configuration_table(&esrt_guid, (void *)esrt);
> > +	if (ret != EFI_SUCCESS) {
> > +		log_err("EFI ESRT: failed to create ESRT\n");
> > +		return ret;
> > +	}
> > +
> > +	log_debug("EFI ESRT: table created\n");
> > +
> > +	return ret;
> > +}
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 5800cbf6d4..7660a20822 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -231,6 +231,10 @@ efi_status_t efi_init_obj_list(void)
> >  	if (ret != EFI_SUCCESS)
> >  		goto out;
> >
> > +	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)
> >
> 

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

[Heinrich Schuchardt]

On 29.01.21 06:26, AKASHI Takahiro wrote:
> First of all, one comment:
> It would be nice to have a list of what features are supported
> and what are not in this patch series.
> For example, with this patch, I suspect
> - FwVersion in ESRT entry will never be updated at capsule update

According to the UEFI specification the ESRT should be updated

* at EFI_EVENT_GROUP_READY_TO_BOOT
* when a device (with a Firmware Management Protocol) is added or removed
* after Update Capsule
* at EFI_EVENT_GROUP_EXIT_BOOT_SERVICES

At each of these events we can call the same function collection the
information for a new ESRT by calling
EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo() and freeing the memory
of the old instance.

Monitoring EFI_EVENT_GROUP_READY_TO_BOOT and
EFI_EVENT_GROUP_EXIT_BOOT_SERVICESs seems redundant if we catch the
other two.

> - LastAttemptVersion/LastAttemptStatus will not be sustained across reboots.

This information is provided by the Firmware Management Protocol (see
23.4.3 Mapping Firmware Management Protocol Descriptors to ESRT
Entries). Jose's patches can only use the information provided by the
code that Sughosh and you created.

If update status persistence is missing in the current implementation of
the firmware management protocol, what are your plans for adding it?

Best regards

Heinrich

>
> So I'm not sure that the proposed implementation is useful
> in a practical manner.
>

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

[AKASHI Takahiro]

On Fri, Feb 12, 2021 at 03:17:30PM +0100, Heinrich Schuchardt wrote:
> On 29.01.21 06:26, AKASHI Takahiro wrote:
> > First of all, one comment:
> > It would be nice to have a list of what features are supported
> > and what are not in this patch series.
> > For example, with this patch, I suspect
> > - FwVersion in ESRT entry will never be updated at capsule update
> 
> According to the UEFI specification the ESRT should be updated
> 
> * at EFI_EVENT_GROUP_READY_TO_BOOT
> * when a device (with a Firmware Management Protocol) is added or removed
> * after Update Capsule
> * at EFI_EVENT_GROUP_EXIT_BOOT_SERVICES
> 
> At each of these events we can call the same function collection the
> information for a new ESRT by calling
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo() and freeing the memory
> of the old instance.
> 
> Monitoring EFI_EVENT_GROUP_READY_TO_BOOT and
> EFI_EVENT_GROUP_EXIT_BOOT_SERVICESs seems redundant if we catch the
> other two.
> 
> > - LastAttemptVersion/LastAttemptStatus will not be sustained across reboots.
> 
> This information is provided by the Firmware Management Protocol (see
> 23.4.3 Mapping Firmware Management Protocol Descriptors to ESRT
> Entries). Jose's patches can only use the information provided by the
> code that Sughosh and you created.

Right. I have forgot that.

> If update status persistence is missing in the current implementation of
> the firmware management protocol, what are your plans for adding it?

No plan.
Since there is no notion of "versioning" in the current U-Boot firmware,
providing such information would be vendor-specific.

Sughosh may have his own idea relating to A/B update and anti-rollback
protection.

-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> >
> > So I'm not sure that the proposed implementation is useful
> > in a practical manner.
> >
> 

[RFC RESEND 2/2] efi: Add fake FMP instance to test ESRT creation

[Jose Marinho]

The intent of this commit is to exercise the ESRT creation -- introduced
in the previous commit.

The fake FMP overrides the symbol arch_efi_load_capsule_drivers().
The fake FMP only implements the get_image_info FMP method.
This get_image_info returns an image descriptor with a single image.

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: Andre Przywara <andre.przywara@arm.com>
CC: Alexander Graf <agraf@csgraf.de>
CC: nd at arm.com
---
 lib/efi_loader/Makefile       |   1 +
 lib/efi_loader/efi_fake_fmp.c | 172 ++++++++++++++++++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 lib/efi_loader/efi_fake_fmp.c

diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index dec791b310..8888ea193d 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
 endif
 obj-y += efi_watchdog.o
 obj-y += efi_esrt.o
+obj-y += efi_fake_fmp.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_fake_fmp.c b/lib/efi_loader/efi_fake_fmp.c
new file mode 100644
index 0000000000..ab7a8164c9
--- /dev/null
+++ b/lib/efi_loader/efi_fake_fmp.c
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Fake FMP intended to test ESRT table creation.
+ *
+ *  Copyright (c) 2021 Arm Ltd.
+ */
+
+#include <common.h>
+#include <efi_loader.h>
+#include <net.h>
+
+/*
+ * Note: this GUID is used for code testing purposes.
+ */
+#define EFI_FAKE_IMG_GUID \
+	EFI_GUID(0x00000001, 0x0002, 0x0003, 0x04, 0x05, \
+		 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b)
+
+static const efi_guid_t fake_img_guid = EFI_FAKE_IMG_GUID;
+
+const struct fw_image_info *get_image_info(void);
+
+static
+efi_status_t EFIAPI efi_fake_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;
+
+	log_debug("EFI FMP: fake fmp get image info\n");
+
+	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 (*image_info_size < sizeof(*image_info)) {
+		*image_info_size = sizeof(*image_info);
+		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+	}
+
+	*image_info_size = sizeof(*image_info);
+	if (descriptor_version)
+		*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
+	if (descriptor_count)
+		*descriptor_count = 1;
+	if (descriptor_size)
+		*descriptor_size = sizeof(*image_info);
+	if (package_version)
+		*package_version = 0xffffffff; /* not supported */
+	if (package_version_name)
+		*package_version_name = NULL; /* not supported */
+
+	if (!image_info_size)
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+	if (*image_info_size < sizeof(*image_info)) {
+		*image_info_size = sizeof(*image_info);
+		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
+	}
+
+	if (!image_info)
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
+
+	log_debug("EFI FMP: fake image UUID=%pUl\n", &fake_img_guid);
+
+	image_info[0].image_index = 1;
+	image_info[0].image_type_id = fake_img_guid;
+
+	image_info[0].image_id = 0;
+	image_info[0].image_id_name = NULL;
+	image_info[0].version = 0; /* not supported */
+	image_info[0].version_name = NULL; /* not supported */
+	image_info[0].size = 0;
+
+	image_info[0].hardware_instance = 1;
+
+	return EFI_EXIT(ret);
+}
+
+static
+efi_status_t EFIAPI efi_fake_fmp_get_image(
+	struct efi_firmware_management_protocol *this,
+	u8 image_index,
+	void *image,
+	efi_uintn_t *image_size)
+{
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+static
+efi_status_t EFIAPI efi_fake_fmp_set_image(
+	struct efi_firmware_management_protocol *this,
+	u8 image_index,
+	const void *image,
+	efi_uintn_t image_size,
+	const void *vendor_code,
+	efi_status_t (*progress)(efi_uintn_t completion),
+	u16 **abort_reason)
+{
+	EFI_ENTRY("%p %d %p %ld %p %p %p\n", this, image_index, image,
+		  image_size, vendor_code, progress, abort_reason);
+
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+static
+efi_status_t EFIAPI efi_fake_fmp_check_image(
+	struct efi_firmware_management_protocol *this,
+	u8 image_index,
+	const void *image,
+	efi_uintn_t *image_size,
+	u32 *image_updatable)
+{
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+static
+efi_status_t EFIAPI efi_fake_fmp_get_package_info(
+	struct efi_firmware_management_protocol *this,
+	u32 *package_version,
+	u16 **package_version_name,
+	u32 *package_version_name_maxlen,
+	u64 *attributes_supported,
+	u64 *attributes_setting)
+{
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+static
+efi_status_t EFIAPI efi_fake_fmp_set_package_info(
+	struct efi_firmware_management_protocol *this,
+	const void *image,
+	efi_uintn_t *image_size,
+	const void *vendor_code,
+	u32 package_version,
+	const u16 *package_version_name)
+{
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+const struct efi_firmware_management_protocol efi_fake_fmp = {
+	.get_image_info = efi_fake_fmp_get_image_info,
+	.get_image = efi_fake_fmp_get_image,
+	.set_image = efi_fake_fmp_set_image,
+	.check_image = efi_fake_fmp_check_image,
+	.get_package_info = efi_fake_fmp_get_package_info,
+	.set_package_info = efi_fake_fmp_set_package_info,
+};
+
+efi_status_t arch_efi_load_capsule_drivers(void)
+{
+	efi_status_t ret = EFI_SUCCESS;
+
+	EFI_CALL(efi_install_multiple_protocol_interfaces(
+			&efi_root, &efi_guid_firmware_management_protocol,
+			&efi_fake_fmp, NULL));
+
+	if (ret != EFI_SUCCESS)
+		log_err("EFI FMP: failed to install fake FMP %lx\n", ret);
+
+	return ret;
+}
-- 
2.17.1

[RFC RESEND 2/2] efi: Add fake FMP instance to test ESRT creation

[Heinrich Schuchardt]

On 28.01.21 14:29, Jose Marinho wrote:
> The intent of this commit is to exercise the ESRT creation -- introduced
> in the previous commit.
>
> The fake FMP overrides the symbol arch_efi_load_capsule_drivers().
> The fake FMP only implements the get_image_info FMP method.
> This get_image_info returns an image descriptor with a single image.
>
> 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: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
> ---
>  lib/efi_loader/Makefile       |   1 +
>  lib/efi_loader/efi_fake_fmp.c | 172 ++++++++++++++++++++++++++++++++++
>  2 files changed, 173 insertions(+)
>  create mode 100644 lib/efi_loader/efi_fake_fmp.c

Hello Jose,

this patch is not yet clear to me.

Why do we need a fake fmp protocol? The driver that is in
lib/efi_loader/efi_firmware.c and is installed if
CONFIG_EFI_CAPSULE_ON_DISK=y already provides an FMP protocol. That
should be enough to trigger the ESRT installation.

I am missing an actual test in the series.

Best regards

Heinrich

>
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index dec791b310..8888ea193d 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -53,6 +53,7 @@ obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
>  endif
>  obj-y += efi_watchdog.o
>  obj-y += efi_esrt.o
> +obj-y += efi_fake_fmp.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_fake_fmp.c b/lib/efi_loader/efi_fake_fmp.c
> new file mode 100644
> index 0000000000..ab7a8164c9
> --- /dev/null
> +++ b/lib/efi_loader/efi_fake_fmp.c
> @@ -0,0 +1,172 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Fake FMP intended to test ESRT table creation.
> + *
> + *  Copyright (c) 2021 Arm Ltd.
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <net.h>
> +
> +/*
> + * Note: this GUID is used for code testing purposes.
> + */
> +#define EFI_FAKE_IMG_GUID \
> +	EFI_GUID(0x00000001, 0x0002, 0x0003, 0x04, 0x05, \
> +		 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b)
> +
> +static const efi_guid_t fake_img_guid = EFI_FAKE_IMG_GUID;
> +
> +const struct fw_image_info *get_image_info(void);
> +
> +static
> +efi_status_t EFIAPI efi_fake_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;
> +
> +	log_debug("EFI FMP: fake fmp get image info\n");
> +
> +	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 (*image_info_size < sizeof(*image_info)) {
> +		*image_info_size = sizeof(*image_info);
> +		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> +	}
> +
> +	*image_info_size = sizeof(*image_info);
> +	if (descriptor_version)
> +		*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> +	if (descriptor_count)
> +		*descriptor_count = 1;
> +	if (descriptor_size)
> +		*descriptor_size = sizeof(*image_info);
> +	if (package_version)
> +		*package_version = 0xffffffff; /* not supported */
> +	if (package_version_name)
> +		*package_version_name = NULL; /* not supported */
> +
> +	if (!image_info_size)
> +		return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> +	if (*image_info_size < sizeof(*image_info)) {
> +		*image_info_size = sizeof(*image_info);
> +		return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> +	}
> +
> +	if (!image_info)
> +		return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
> +	log_debug("EFI FMP: fake image UUID=%pUl\n", &fake_img_guid);
> +
> +	image_info[0].image_index = 1;
> +	image_info[0].image_type_id = fake_img_guid;
> +
> +	image_info[0].image_id = 0;
> +	image_info[0].image_id_name = NULL;
> +	image_info[0].version = 0; /* not supported */
> +	image_info[0].version_name = NULL; /* not supported */
> +	image_info[0].size = 0;
> +
> +	image_info[0].hardware_instance = 1;
> +
> +	return EFI_EXIT(ret);
> +}
> +
> +static
> +efi_status_t EFIAPI efi_fake_fmp_get_image(
> +	struct efi_firmware_management_protocol *this,
> +	u8 image_index,
> +	void *image,
> +	efi_uintn_t *image_size)
> +{
> +	return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static
> +efi_status_t EFIAPI efi_fake_fmp_set_image(
> +	struct efi_firmware_management_protocol *this,
> +	u8 image_index,
> +	const void *image,
> +	efi_uintn_t image_size,
> +	const void *vendor_code,
> +	efi_status_t (*progress)(efi_uintn_t completion),
> +	u16 **abort_reason)
> +{
> +	EFI_ENTRY("%p %d %p %ld %p %p %p\n", this, image_index, image,
> +		  image_size, vendor_code, progress, abort_reason);
> +
> +	return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static
> +efi_status_t EFIAPI efi_fake_fmp_check_image(
> +	struct efi_firmware_management_protocol *this,
> +	u8 image_index,
> +	const void *image,
> +	efi_uintn_t *image_size,
> +	u32 *image_updatable)
> +{
> +	return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static
> +efi_status_t EFIAPI efi_fake_fmp_get_package_info(
> +	struct efi_firmware_management_protocol *this,
> +	u32 *package_version,
> +	u16 **package_version_name,
> +	u32 *package_version_name_maxlen,
> +	u64 *attributes_supported,
> +	u64 *attributes_setting)
> +{
> +	return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +static
> +efi_status_t EFIAPI efi_fake_fmp_set_package_info(
> +	struct efi_firmware_management_protocol *this,
> +	const void *image,
> +	efi_uintn_t *image_size,
> +	const void *vendor_code,
> +	u32 package_version,
> +	const u16 *package_version_name)
> +{
> +	return EFI_EXIT(EFI_UNSUPPORTED);
> +}
> +
> +const struct efi_firmware_management_protocol efi_fake_fmp = {
> +	.get_image_info = efi_fake_fmp_get_image_info,
> +	.get_image = efi_fake_fmp_get_image,
> +	.set_image = efi_fake_fmp_set_image,
> +	.check_image = efi_fake_fmp_check_image,
> +	.get_package_info = efi_fake_fmp_get_package_info,
> +	.set_package_info = efi_fake_fmp_set_package_info,
> +};
> +
> +efi_status_t arch_efi_load_capsule_drivers(void)
> +{
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	EFI_CALL(efi_install_multiple_protocol_interfaces(
> +			&efi_root, &efi_guid_firmware_management_protocol,
> +			&efi_fake_fmp, NULL));
> +
> +	if (ret != EFI_SUCCESS)
> +		log_err("EFI FMP: failed to install fake FMP %lx\n", ret);
> +
> +	return ret;
> +}
>