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

Heinrich Schuchardt xypron.glpk at gmx.de
Tue Feb 16 11:27:27 CET 2021


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 at arm.com>
>>
>> CC: Heinrich Schuchardt	<xypron.glpk at gmx.de>
>> CC: Sughosh Ganu <sughosh.ganu at linaro.org>
>> CC: AKASHI Takahiro <takahiro.akashi at linaro.org>
>> CC: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>> CC: Andre Przywara <andre.przywara at arm.com>
>> CC: Alexander Graf <agraf at 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)
>>
>



More information about the U-Boot mailing list