On 23.02.21 17:50, Jose Marinho wrote:
> The ESRT is initialised during efi_init_objlist after
> efi_initialize_system_table().
>
> The ESRT is recreated from scratch at the following events:
> - successful UpdateCapsule;
> - FMP instance install.
>
> The code ensures that every ESRT entry has a unique fw_class value.
>
> Limitations:
> - The ESRT is not updated when an FMP instance is uninstalled;
> - the fields image_type and flags are currently set to UNKNOWN and 0
> respectively. The mapping between fw_class and the image_type/flags
> fields is platform specific. A mapping function is lacking from the
> current implementation but should be added in the future.
>
> Signed-off-by: Jose Marinho <[email protected]>
>
> CC: Heinrich Schuchardt       <[email protected]>
> CC: Sughosh Ganu <[email protected]>
> CC: AKASHI Takahiro <[email protected]>
> CC: Ilias Apalodimas <[email protected]>
> CC: Andre Przywara <[email protected]>
> CC: Alexander Graf <[email protected]>
> CC: [email protected]
>
> ---
>  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    | 518 +++++++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_setup.c   |   6 +
>  8 files changed, 585 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..d31963ad6e
> --- /dev/null
> +++ b/lib/efi_loader/efi_esrt.c
> @@ -0,0 +1,518 @@
> +// 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.

Please get rid of this parameter. Export the functions that you want to
access instead.

> + * @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;
> +
> +     /* 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);

EFI_CALL() is needed to call the UEFI API.

Call efi_install_configuration_table() instead.

> +     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.

Get rid of this parameter.

> + * @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);

EFI_CALL() is needed to call the UEFI API.

> +
> +     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_CALL().

> +              */
> +             EFI_PRINT("Erroneous FMP implementation\n");
> +             return EFI_INVALID_PARAMETER;
> +     }
> +
> +     ret = bt->allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
> +                             (void **)&img_info);

Call efi_allocate_pool() instead.

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

EFI_CALL()

> +     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;
> +     }

Do not use this pointer. Instead export the required functions.

> +
> +     /*
> +      * 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);

You have to use EFI_CALL() here. To avoid one level of indirection,
please, use EFI_CALL(efi_locate_handle_buffer())

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

Call efi_search_protocol().
handler->protocol_interface is what you look for.

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


EFI_CALL()

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

This would require EFI_CALL(). Just use efi_allocate_pool().

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

EFI_CALL().

> +             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 all 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);


Use efi_search_protocol()

> +
> +             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;

Here you need EFI_ENTRY().


> +
> +     ret = efi_esrt_populate();
> +     if (ret != EFI_SUCCESS) {
> +             EFI_PRINT("ESRT failed to populate ESRT entry\n");
> +             return;
> +     }

You must leave with EFI_EXIT().

> +}
> +
> +/**
> + * 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);

This would require EFI_CALL().

Call efi_create_event() instead.

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

You need EFI_CALL here().

It is preferable to export efi_register_protocol_notify() to avoid the
bt-> indirection.

Best regards

Heinrich

> +     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)
>

Reply via email to