All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3 v5] Add ESRT and test ESRT creation
@ 2021-03-02 12:13 Jose Marinho
  2021-03-02 12:13 ` [PATCH 1/3 v5] efi: Add ESRT to the EFI system table Jose Marinho
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Jose Marinho @ 2021-03-02 12:13 UTC (permalink / raw)
  To: u-boot

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

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

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

The second commit introduces a helper cmd function to print the ESRT in
the u-boot shell.

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

Patch v5:
- Address v4 comments.
- split the v4 2/2 commit into the v5 2/3 (debug print ESRT) and 3/3
  ESRT tests.

Patch v4:
- update stale [Patch 1/2 v3] commit message.

Patch v3:
- Address v2 comments.

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

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

rfc: initial patch submission

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

Jose Marinho (3):
  efi: Add ESRT to the EFI system table
  cmd: efi: ESRT table debug print
  efi: ESRT creation tests

 cmd/efidebug.c                                |  68 +++
 include/efi_api.h                             |  21 +
 include/efi_loader.h                          |  24 +
 lib/efi_loader/Kconfig                        |   7 +
 lib/efi_loader/Makefile                       |   1 +
 lib/efi_loader/efi_boottime.c                 |   7 +-
 lib/efi_loader/efi_capsule.c                  |   8 +
 lib/efi_loader/efi_esrt.c                     | 510 ++++++++++++++++++
 lib/efi_loader/efi_setup.c                    |   6 +
 lib/efi_selftest/Makefile                     |   2 +
 lib/efi_selftest/efi_selftest_esrt.c          | 227 ++++++++
 .../test_efi_capsule/test_capsule_firmware.py |   4 +
 12 files changed, 881 insertions(+), 4 deletions(-)
 create mode 100644 lib/efi_loader/efi_esrt.c
 create mode 100644 lib/efi_selftest/efi_selftest_esrt.c

-- 
2.17.1

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

* [PATCH 1/3 v5] efi: Add ESRT to the EFI system table
  2021-03-02 12:13 [PATCH 0/3 v5] Add ESRT and test ESRT creation Jose Marinho
@ 2021-03-02 12:13 ` Jose Marinho
  2021-03-02 12:13 ` [PATCH 2/3 v5] cmd: efi: ESRT table debug print Jose Marinho
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Jose Marinho @ 2021-03-02 12:13 UTC (permalink / raw)
  To: u-boot

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 if an FMP instance is uninstalled;
- the fields image_type and flags are in the current implementation left
undefined. Setting these values will require a per-platform function
that returns the image_type/flags as a function of the image fw_class.

Signed-off-by: Jose Marinho <jose.marinho@arm.com>
---
 cmd/efidebug.c                |   4 +
 include/efi_api.h             |  21 ++
 include/efi_loader.h          |  24 ++
 lib/efi_loader/Kconfig        |   7 +
 lib/efi_loader/Makefile       |   1 +
 lib/efi_loader/efi_boottime.c |   7 +-
 lib/efi_loader/efi_capsule.c  |   8 +
 lib/efi_loader/efi_esrt.c     | 510 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c    |   6 +
 9 files changed, 584 insertions(+), 4 deletions(-)
 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..ae450ffc73 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;
@@ -552,6 +554,10 @@ struct efi_simple_file_system_protocol *efi_simple_file_system(
 /* open file from device-path: */
 struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp);

+/* Registers a callback function for a notification event. */
+efi_status_t EFIAPI efi_register_protocol_notify(const efi_guid_t *protocol,
+                                                struct efi_event *event,
+                                                void **registration);
 /**
  * efi_size_in_pages() - convert size in bytes to size in pages
  *
@@ -884,4 +890,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_boottime.c b/lib/efi_loader/efi_boottime.c
index 41b8949b04..8e8b0a9bc6 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1406,10 +1406,9 @@ out:
  *
  * Return: status code
  */
-static efi_status_t EFIAPI efi_register_protocol_notify(
-                                               const efi_guid_t *protocol,
-                                               struct efi_event *event,
-                                               void **registration)
+efi_status_t EFIAPI efi_register_protocol_notify(const efi_guid_t *protocol,
+                                                struct efi_event *event,
+                                                void **registration)
 {
        struct efi_register_notify_event *item;
        efi_status_t ret = EFI_SUCCESS;
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..c43e898d63
--- /dev/null
+++ b/lib/efi_loader/efi_esrt.c
@@ -0,0 +1,510 @@
+// 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.
+ *
+ * @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(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 = efi_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 = EFI_CALL(efi_install_configuration_table(&esrt_guid, (void *)new_esrt));
+       if (ret != EFI_SUCCESS) {
+               EFI_PRINT("ESRT failed to install the ESRT in the system table\n");
+               return ret;
+       }
+
+       /* If there was a previous ESRT, deallocate its memory now. */
+       if (esrt)
+               ret = EFI_CALL(efi_free_pool(esrt));
+
+       esrt = new_esrt;
+
+       return EFI_SUCCESS;
+}
+
+/**
+ * esrt_find_entry() - Obtain the ESRT entry for the image with GUID
+ * @img_fw_class.
+ *
+ * If the img_fw_class is not yet present in the ESRT, this function
+ * reserves the tail element of the current ESRT as the entry for that fw_class.
+ * The number of elements in the ESRT is updated in that case.
+ *
+ * @img_fw_class: the GUID of the FW image which ESRT entry we want to obtain.
+ *
+ * Return:
+ *  - A pointer to the ESRT entry for the image with GUID img_fw_class,
+ *  - NULL if:
+ *   - there is no more space in the ESRT,
+ *   - ESRT is not initialized,
+ */
+static
+struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
+{
+       u32 filled_entries;
+       u32 max_entries;
+       struct efi_system_resource_entry *entry;
+
+       if (!esrt) {
+               EFI_PRINT("ESRT access before initialized\n");
+               return NULL;
+       }
+
+       filled_entries = esrt->fw_resource_count;
+       entry = esrt->entries;
+
+       /* Check if the image with img_fw_class is already in the ESRT. */
+       for (u32 idx = 0; idx < filled_entries; idx++) {
+               if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
+                       EFI_PRINT("ESRT found entry for image %pUl@index %d\n",
+                                 img_fw_class, idx);
+                       return &entry[idx];
+               }
+       }
+
+       max_entries = esrt->fw_resource_count_max;
+       /*
+        * Since the image with img_fw_class is not present in the ESRT, check
+        * if ESRT is full before appending the new entry to it.
+        */
+       if (filled_entries == max_entries) {
+               EFI_PRINT("ESRT full, this should not happen\n");
+               return NULL;
+       }
+
+       /*
+        * This is a new entry for a fw image, increment the element
+        * number in the table and set the fw_class field.
+        */
+       esrt->fw_resource_count++;
+       entry[filled_entries].fw_class = *img_fw_class;
+       EFI_PRINT("ESRT allocated new entry for image %pUl at index %d\n",
+                 img_fw_class, filled_entries);
+
+       return &entry[filled_entries];
+}
+
+/**
+ * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
+ * images in the FMP.
+ *
+ * @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_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 = EFI_CALL(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(efi_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 = EFI_CALL(fmp->get_image_info(fmp, &info_size, img_info,
+                                          &desc_version, &desc_count,
+                                          &desc_size, &package_version,
+                                          &package_version_name));
+       if (ret != EFI_SUCCESS) {
+               EFI_PRINT("ESRT failed to obtain the FMP image info\n");
+               goto out;
+       }
+
+       /*
+        * Iterate over all the FW images in the FMP.
+        */
+       for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
+               struct efi_firmware_image_descriptor *cur_img_info =
+                       (struct efi_firmware_image_descriptor *)
+                       ((uintptr_t)img_info + desc_idx * desc_size);
+
+               /*
+                * Obtain the ESRT entry for the FW image with fw_class
+                * equal to cur_img_info->image_type_id.
+                */
+               entry = esrt_find_entry(&cur_img_info->image_type_id);
+
+               if (entry) {
+                       ret = efi_esrt_image_info_to_entry(cur_img_info, entry,
+                                                          desc_version,
+                                                          image_type, flags);
+                       if (ret != EFI_SUCCESS)
+                               EFI_PRINT("ESRT entry mismatches image_type\n");
+
+               } else {
+                       EFI_PRINT("ESRT failed to add entry for %pUl\n",
+                                 &cur_img_info->image_type_id);
+                       continue;
+               }
+       }
+
+out:
+       EFI_CALL(efi_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_handler *handler;
+
+       /*
+        * 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 **)&base_handle));
+
+       if (ret != EFI_SUCCESS) {
+               EFI_PRINT("ESRT There are no FMP instances\n");
+
+               ret = efi_esrt_allocate_install(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 = EFI_CALL(efi_search_protocol(*it_handle,
+                                                  &efi_guid_firmware_management_protocol,
+                                                  &handler));
+
+               if (ret != EFI_SUCCESS) {
+                       EFI_PRINT("ESRT Unable to find FMP handle (%d)\n",
+                                 idx);
+                       goto out;
+               }
+               fmp = handler->protocol_interface;
+
+               ret = EFI_CALL(fmp->get_image_info(fmp, &info_size, NULL,
+                                                  &desc_version, &desc_count,
+                                                  &desc_size, &package_version,
+                                                  &package_version_name));
+
+               if (ret != EFI_BUFFER_TOO_SMALL) {
+                       /*
+                        * An input of info_size=0 should always lead
+                        * fmp->get_image_info to return BUFFER_TO_SMALL.
+                        */
+                       EFI_PRINT("ESRT erroneous FMP implementation\n");
+                       ret = EFI_INVALID_PARAMETER;
+                       goto out;
+               }
+
+               ret = EFI_CALL(efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
+                                                (void **)&img_info));
+               if (ret != EFI_SUCCESS) {
+                       EFI_PRINT("ESRT failed to allocate memory for image info\n");
+                       goto out;
+               }
+
+               /*
+                * Calls to a FMP get_image_info method do not return the
+                * desc_count value if the return status differs from EFI_SUCCESS.
+                * We need to repeat the call to get_image_info with a properly
+                * sized buffer in order to obtain the real number of images
+                * handled by the FMP.
+                */
+               ret = EFI_CALL(fmp->get_image_info(fmp, &info_size, img_info,
+                                                  &desc_version, &desc_count,
+                                                  &desc_size, &package_version,
+                                                  &package_version_name));
+
+               if (ret != EFI_SUCCESS) {
+                       EFI_PRINT("ESRT failed to obtain image info from FMP\n");
+                       EFI_CALL(efi_free_pool(img_info));
+                       goto out;
+               }
+
+               num_entries += desc_count;
+
+               EFI_CALL(efi_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(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 = EFI_CALL(efi_search_protocol(*it_handle,
+                                                  &efi_guid_firmware_management_protocol,
+                                                  &handler));
+
+               if (ret != EFI_SUCCESS) {
+                       EFI_PRINT("ESRT unable to find FMP handle (%d)\n",
+                                 idx);
+                       break;
+               }
+               fmp = handler->protocol_interface;
+
+               ret = efi_esrt_add_from_fmp(fmp);
+               if (ret != EFI_SUCCESS)
+                       EFI_PRINT("ESRT failed to add FMP to the table\n");
+       }
+
+out:
+
+       EFI_CALL(efi_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;
+
+       EFI_ENTRY();
+
+       ret = efi_esrt_populate();
+       if (ret != EFI_SUCCESS)
+               EFI_PRINT("ESRT failed to populate ESRT entry\n");
+
+       EFI_EXIT(ret);
+}
+
+/**
+ * efi_esrt_register() - Install the ESRT system table.
+ *
+ * Return: status code
+ */
+efi_status_t efi_esrt_register(void)
+{
+       struct efi_event *ev = NULL;
+       void *registration;
+       efi_status_t ret;
+
+       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 = EFI_CALL(efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+                                       efi_esrt_new_fmp_notify, NULL, NULL, &ev));
+       if (ret != EFI_SUCCESS) {
+               EFI_PRINT("ESRT failed to create event\n");
+               return ret;
+       }
+
+       ret = EFI_CALL(efi_register_protocol_notify(&efi_guid_firmware_management_protocol,
+                                                   ev, &registration));
+       if (ret != EFI_SUCCESS) {
+               EFI_PRINT("ESRT failed to register FMP callback\n");
+               return ret;
+       }
+
+       EFI_PRINT("ESRT table created\n");
+
+       return ret;
+}
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index b1c5125032..3c5cf9a435 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -227,6 +227,12 @@ efi_status_t efi_init_obj_list(void)
        if (ret != EFI_SUCCESS)
                goto out;

+       if (IS_ENABLED(CONFIG_EFI_ESRT)) {
+               ret = efi_esrt_register();
+               if (ret != EFI_SUCCESS)
+                       goto out;
+       }
+
        if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
                ret = efi_tcg2_register();
                if (ret != EFI_SUCCESS)
--
2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* [PATCH 2/3 v5] cmd: efi: ESRT table debug print
  2021-03-02 12:13 [PATCH 0/3 v5] Add ESRT and test ESRT creation Jose Marinho
  2021-03-02 12:13 ` [PATCH 1/3 v5] efi: Add ESRT to the EFI system table Jose Marinho
@ 2021-03-02 12:13 ` Jose Marinho
  2021-03-03  0:01   ` AKASHI Takahiro
  2021-03-02 12:13 ` [PATCH 3/3 v5] efi: ESRT creation tests Jose Marinho
  2021-03-02 17:26 ` [PATCH RESEND 1/3 v5] efi: Add ESRT to the EFI system table Jose Marinho
  3 siblings, 1 reply; 10+ messages in thread
From: Jose Marinho @ 2021-03-02 12:13 UTC (permalink / raw)
  To: u-boot

This commit enables the ESRT printing from the u-boot shell by invoking:
- efidebug capsule esrt

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

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

---
 cmd/efidebug.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index a7dace2f80..5a9ff2bd9a 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -129,6 +129,61 @@ static int do_efi_capsule_show(struct cmd_tbl *cmdtp, int flag,
 	return CMD_RET_SUCCESS;
 }
 
+#ifdef CONFIG_EFI_ESRT
+/**
+ * do_efi_capsule_esrt() - manage UEFI capsules
+ *
+ * @cmdtp:	Command table
+ * @flag:	Command flag
+ * @argc:	Number of arguments
+ * @argv:	Argument array
+ * Return:	CMD_RET_SUCCESS on success,
+ *		CMD_RET_USAGE or CMD_RET_RET_FAILURE on failure
+ *
+ * Implement efidebug "capsule esrt" sub-command.
+ * The prints the current ESRT table.
+ *
+ *     efidebug capsule esrt
+ */
+static int do_efi_capsule_esrt(struct cmd_tbl *cmdtp, int flag,
+			       int argc, char * const argv[])
+{
+	struct efi_system_resource_table *esrt = NULL;
+
+	if (argc != 1)
+		return CMD_RET_USAGE;
+
+	for (int idx = 0; idx < systab.nr_tables; idx++)
+		if (!guidcmp(&efi_esrt_guid, &systab.tables[idx].guid))
+			esrt = (struct efi_system_resource_table *)systab.tables[idx].table;
+
+	if (!esrt)
+		return CMD_RET_FAILURE;
+
+	printf("========================================\n");
+	printf("ESRT: fw_resource_count=%d\n", esrt->fw_resource_count);
+	printf("ESRT: fw_resource_count_max=%d\n", esrt->fw_resource_count_max);
+	printf("ESRT: fw_resource_version=%lld\n", esrt->fw_resource_version);
+
+	for (int idx = 0; idx < esrt->fw_resource_count; idx++) {
+		printf("[entry %d]==============================\n", idx);
+		printf("ESRT: fw_class=%pUL\n", &esrt->entries[idx].fw_class);
+		printf("ESRT: fw_type=%d\n", esrt->entries[idx].fw_type);
+		printf("ESRT: fw_version=%d\n", esrt->entries[idx].fw_version);
+		printf("ESRT: lowest_supported_fw_version=%d\n",
+		       esrt->entries[idx].lowest_supported_fw_version);
+		printf("ESRT: capsule_flags=%d\n",
+		       esrt->entries[idx].capsule_flags);
+		printf("ESRT: last_attempt_version=%d\n",
+		       esrt->entries[idx].last_attempt_version);
+		printf("ESRT: last_attempt_status=%d\n",
+		       esrt->entries[idx].last_attempt_status);
+	}
+	printf("========================================\n");
+
+	return CMD_RET_SUCCESS;
+}
+#endif /*  CONFIG_EFI_ESRT */
 /**
  * do_efi_capsule_res() - show a capsule update result
  *
@@ -221,6 +276,10 @@ static struct cmd_tbl cmd_efidebug_capsule_sub[] = {
 			 "", ""),
 	U_BOOT_CMD_MKENT(show, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_show,
 			 "", ""),
+#ifdef CONFIG_EFI_ESRT
+	U_BOOT_CMD_MKENT(esrt, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_esrt,
+			 "", ""),
+#endif
 	U_BOOT_CMD_MKENT(disk-update, 0, 0, do_efi_capsule_on_disk_update,
 			 "", ""),
 	U_BOOT_CMD_MKENT(result, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_res,
@@ -256,6 +315,7 @@ static int do_efi_capsule(struct cmd_tbl *cmdtp, int flag,
 
 	return cp->cmd(cmdtp, flag, argc, argv);
 }
+
 #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
 
 /**
@@ -1580,6 +1640,10 @@ static char efidebug_help_text[] =
 	"  - show capsule information\n"
 	"efidebug capsule result [<capsule result var>]\n"
 	"  - show a capsule update result\n"
+#ifdef CONFIG_EFI_ESRT
+	"efidebug capsule esrt\n"
+	"  - print the ESRT\n"
+#endif
 	"\n"
 #endif
 	"efidebug devices\n"
-- 
2.17.1

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

* [PATCH 3/3 v5] efi: ESRT creation tests
  2021-03-02 12:13 [PATCH 0/3 v5] Add ESRT and test ESRT creation Jose Marinho
  2021-03-02 12:13 ` [PATCH 1/3 v5] efi: Add ESRT to the EFI system table Jose Marinho
  2021-03-02 12:13 ` [PATCH 2/3 v5] cmd: efi: ESRT table debug print Jose Marinho
@ 2021-03-02 12:13 ` Jose Marinho
  2021-03-02 18:30   ` Heinrich Schuchardt
  2021-03-02 17:26 ` [PATCH RESEND 1/3 v5] efi: Add ESRT to the EFI system table Jose Marinho
  3 siblings, 1 reply; 10+ messages in thread
From: Jose Marinho @ 2021-03-02 12:13 UTC (permalink / raw)
  To: u-boot

This commmit exercises the ESRT creation in two tests.

test 1:
 A fake FMP, with TEST_ESRT_NUM_ENTRIES FW images, is installed in the
 system leading to the corresponding ESRT entries being populated.
 The ESRT entries are checked against the datastructure used to
 initialize the FMP.

test 1 invocation:
add to sandbox_defconfig:
  +CONFIG_EFI_SELFTEST=y

 make sandbox_capsule_defconfig all
 ./u-boot -d arch/sandbox/dts/test.dtb
 bootefi selftest

test 2:
 The test is part of test_efi_capsule_fw3.

 In order to run the test the following must be added to
 sandbox_defconfig:
  +CONFIG_CMD_SF=y
  +CONFIG_CMD_MEMORY=y
  +CONFIG_CMD_FAT=y
  +CONFIG_DFU=y

 The ESRT is printed in the u-boot shell by calling efidebug esrt.
 The test ensures that, after the capsule is installed, the  ESRT
 contains entries with the GUIDs:
  - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
  - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;

test 2 invocation:
 sudo ./test/py/test.py --bd sandbox -k capsule_fw3 -l --build

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

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

---
 lib/efi_selftest/Makefile                     |   2 +
 lib/efi_selftest/efi_selftest_esrt.c          | 227 ++++++++++++++++++
 .../test_efi_capsule/test_capsule_firmware.py |   4 +
 3 files changed, 233 insertions(+)
 create mode 100644 lib/efi_selftest/efi_selftest_esrt.c

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 7d6ea30102..017b191a46 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -72,6 +72,8 @@ ifeq ($(CONFIG_BLK)$(CONFIG_DOS_PARTITION),yy)
 obj-y += efi_selftest_block_device.o
 endif
 
+obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
+
 targets += \
 efi_miniapp_file_image_exception.h \
 efi_miniapp_file_image_exit.h \
diff --git a/lib/efi_selftest/efi_selftest_esrt.c b/lib/efi_selftest/efi_selftest_esrt.c
new file mode 100644
index 0000000000..b053b2e6c6
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_esrt.c
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  Test ESRT tables support
+ *
+ *  Copyright (C) 2021 Arm Ltd.
+ */
+#include <common.h>
+#include <efi_loader.h>
+#include <efi_selftest.h>
+
+// This value must not exceed 255.
+// An FMP cannot contain more than 255 FW images.
+#define TEST_ESRT_NUM_ENTRIES 255
+
+static
+struct efi_firmware_image_descriptor static_img_info[TEST_ESRT_NUM_ENTRIES];
+
+static void efi_test_esrt_init_info(void)
+{
+	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++) {
+		static_img_info[idx].image_index = idx;
+
+		// Note: the 16 byte value present in
+		// static_img_info[idx].image_type_id is not strictly a GUID.
+		// The value is used for the sake of code testing.
+		static_img_info[idx].image_type_id.b[0] = idx;
+
+		static_img_info[idx].image_id = 0;
+		static_img_info[idx].image_id_name = NULL;
+		static_img_info[idx].version = 0;
+		static_img_info[idx].version_name = NULL;
+		static_img_info[idx].size = 0;
+		static_img_info[idx].lowest_supported_image_version = 1;
+		static_img_info[idx].last_attempt_version = 2;
+		static_img_info[idx].last_attempt_status = 3;
+		static_img_info[idx].hardware_instance = 1;
+	}
+}
+
+static efi_status_t
+EFIAPI efi_test_fmp_get_image_info(struct efi_firmware_management_protocol *this,
+				   efi_uintn_t *image_info_size,
+				   struct efi_firmware_image_descriptor *image_info,
+				   u32 *descriptor_version,
+				   u8 *descriptor_count,
+				   efi_uintn_t *descriptor_size,
+				   u32 *package_version,
+				   u16 **package_version_name)
+{
+	efi_status_t ret = EFI_SUCCESS;
+
+	if (!image_info_size)
+		return EFI_INVALID_PARAMETER;
+
+	if (descriptor_version)
+		*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
+	if (descriptor_count)
+		*descriptor_count = TEST_ESRT_NUM_ENTRIES;
+	if (descriptor_size)
+		*descriptor_size = sizeof(*image_info);
+	if (package_version)
+		*package_version = 0xffffffff;
+	if (package_version_name)
+		*package_version_name = NULL;
+
+	if (*image_info_size < sizeof(*image_info)) {
+		*image_info_size = *descriptor_size * *descriptor_count;
+		return EFI_BUFFER_TOO_SMALL;
+	}
+
+	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
+		image_info[idx] = static_img_info[idx];
+
+	return ret;
+}
+
+static struct efi_firmware_management_protocol efi_test_fmp = {
+	.get_image_info = efi_test_fmp_get_image_info,
+	.get_image = NULL,
+	.set_image = NULL,
+	.check_image = NULL,
+	.get_package_info = NULL,
+	.set_package_info = NULL,
+};
+
+static void *lib_test_get_esrt(void)
+{
+	for (int idx = 0; idx < systab.nr_tables; idx++)
+		if (!guidcmp(&efi_esrt_guid, &systab.tables[idx].guid))
+			return systab.tables[idx].table;
+
+	return NULL;
+}
+
+static bool lib_test_check_uuid_entry(struct efi_system_resource_table *esrt,
+				      struct efi_firmware_image_descriptor
+				      *img_info)
+{
+	const u32 filled_entries = esrt->fw_resource_count;
+	struct efi_system_resource_entry *entry = esrt->entries;
+
+	for (u32 idx = 0; idx < filled_entries; idx++) {
+		if (!guidcmp(&entry[idx].fw_class, &img_info->image_type_id)) {
+			if (entry[idx].fw_version != img_info->version)
+				return false;
+
+			if (entry[idx].lowest_supported_fw_version !=
+				img_info->lowest_supported_image_version)
+				return false;
+
+			if (entry[idx].last_attempt_version !=
+				img_info->last_attempt_version)
+				return false;
+
+			if (entry[idx].last_attempt_status !=
+				img_info->last_attempt_status)
+				return false;
+
+			/*
+			 * The entry with fw_class = img_uuid matches with the
+			 * remainder fmp input.
+			 */
+			return true;
+		}
+	}
+
+	/* There exists no entry with fw_class equal to img_uuid in the ESRT. */
+	return false;
+}
+
+/*
+ * Setup unit test.
+ *
+ * Initialize the test FMP datastructure.
+ * Install the FMP in the system.
+ *
+ * @handle:	handle of the loaded image
+ * @systable:	system table
+ * @return:	EFI_ST_SUCCESS for success
+ */
+static int setup(const efi_handle_t handle,
+		 const struct efi_system_table *systable)
+{
+	efi_status_t ret = EFI_SUCCESS;
+	struct efi_boot_services *bt = systab.boottime;
+
+	if (!bt)
+		return EFI_ST_FAILURE;
+
+	efi_test_esrt_init_info();
+
+	ret = EFI_CALL(bt->install_multiple_protocol_interfaces
+		(&efi_root,
+		 &efi_guid_firmware_management_protocol,
+		 &efi_test_fmp,
+		 NULL));
+
+	if (ret != EFI_SUCCESS)
+		return EFI_ST_FAILURE;
+
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * Tear down unit test.
+ *
+ * Uninstall the test FMP.
+ *
+ * @return:	EFI_ST_SUCCESS for success
+ */
+static int teardown(void)
+{
+	efi_status_t ret = EFI_SUCCESS;
+	struct efi_boot_services *bt = systab.boottime;
+
+	if (!bt)
+		return EFI_ST_FAILURE;
+
+	ret = EFI_CALL(bt->uninstall_multiple_protocol_interfaces
+		(efi_root, &efi_guid_firmware_management_protocol,
+		 &efi_test_fmp, NULL));
+
+	if (ret != EFI_SUCCESS)
+		return EFI_ST_FAILURE;
+
+	return EFI_ST_SUCCESS;
+}
+
+static int execute(void)
+{
+	struct efi_system_resource_table *esrt;
+	efi_status_t ret = EFI_SUCCESS;
+
+	esrt = lib_test_get_esrt();
+	if (!esrt)
+		return EFI_ST_FAILURE;
+
+	if (esrt->fw_resource_count != TEST_ESRT_NUM_ENTRIES)
+		return EFI_ST_FAILURE;
+
+	/* Update the ESRT. */
+	ret = efi_esrt_populate();
+	if (ret != EFI_SUCCESS)
+		return EFI_ST_FAILURE;
+
+	esrt = lib_test_get_esrt();
+	if (!esrt)
+		return EFI_ST_FAILURE;
+
+	/* Verify that the number of images remains the same. */
+	if (esrt->fw_resource_count != TEST_ESRT_NUM_ENTRIES)
+		return EFI_ST_FAILURE;
+
+	for (u32 idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
+		if (!lib_test_check_uuid_entry(esrt, &static_img_info[idx]))
+			return EFI_ST_FAILURE;
+
+	return EFI_ST_SUCCESS;
+}
+
+EFI_UNIT_TEST(esrt) = {
+	.name = "esrt",
+	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+	.setup = setup,
+	.execute = execute,
+	.teardown = teardown,
+};
diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
index f006fa95d6..3a7c2e1ac8 100644
--- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
+++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
@@ -229,6 +229,10 @@ class TestEfiCapsuleFirmwareFit(object):
                 output = u_boot_console.run_command(
                     'env print -e -all Capsule0000')
 
+            output = u_boot_console.run_command_list(['efidebug capsule esrt'])
+            assert 'AE13FF2D-9AD4-4E25-9AC8-6D80B3B22147' in ''.join(output)
+            assert 'E2BB9C06-70E9-4B14-97A3-5A7913176E3F' in ''.join(output)
+
             output = u_boot_console.run_command_list([
                 'host bind 0 %s' % disk_img,
                 'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
-- 
2.17.1

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

* [PATCH RESEND 1/3 v5] efi: Add ESRT to the EFI system table
  2021-03-02 12:13 [PATCH 0/3 v5] Add ESRT and test ESRT creation Jose Marinho
                   ` (2 preceding siblings ...)
  2021-03-02 12:13 ` [PATCH 3/3 v5] efi: ESRT creation tests Jose Marinho
@ 2021-03-02 17:26 ` Jose Marinho
  2021-03-02 18:11   ` Heinrich Schuchardt
  3 siblings, 1 reply; 10+ messages in thread
From: Jose Marinho @ 2021-03-02 17:26 UTC (permalink / raw)
  To: u-boot

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 if an FMP instance is uninstalled;
- the fields image_type and flags are in the current implementation left
undefined. Setting these values will require a per-platform function
that returns the image_type/flags as a function of the image fw_class.

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

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

---
 cmd/efidebug.c                |   4 +
 include/efi_api.h             |  21 ++
 include/efi_loader.h          |  24 ++
 lib/efi_loader/Kconfig        |   7 +
 lib/efi_loader/Makefile       |   1 +
 lib/efi_loader/efi_boottime.c |   7 +-
 lib/efi_loader/efi_capsule.c  |   8 +
 lib/efi_loader/efi_esrt.c     | 510 ++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c    |   6 +
 9 files changed, 584 insertions(+), 4 deletions(-)
 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..ae450ffc73 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;
@@ -552,6 +554,10 @@ struct efi_simple_file_system_protocol *efi_simple_file_system(
 /* open file from device-path: */
 struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp);
 
+/* Registers a callback function for a notification event. */
+efi_status_t EFIAPI efi_register_protocol_notify(const efi_guid_t *protocol,
+						 struct efi_event *event,
+						 void **registration);
 /**
  * efi_size_in_pages() - convert size in bytes to size in pages
  *
@@ -884,4 +890,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_boottime.c b/lib/efi_loader/efi_boottime.c
index 41b8949b04..8e8b0a9bc6 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1406,10 +1406,9 @@ out:
  *
  * Return: status code
  */
-static efi_status_t EFIAPI efi_register_protocol_notify(
-						const efi_guid_t *protocol,
-						struct efi_event *event,
-						void **registration)
+efi_status_t EFIAPI efi_register_protocol_notify(const efi_guid_t *protocol,
+						 struct efi_event *event,
+						 void **registration)
 {
 	struct efi_register_notify_event *item;
 	efi_status_t ret = EFI_SUCCESS;
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..c43e898d63
--- /dev/null
+++ b/lib/efi_loader/efi_esrt.c
@@ -0,0 +1,510 @@
+// 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.
+ *
+ * @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(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 = efi_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 = EFI_CALL(efi_install_configuration_table(&esrt_guid, (void *)new_esrt));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to install the ESRT in the system table\n");
+		return ret;
+	}
+
+	/* If there was a previous ESRT, deallocate its memory now. */
+	if (esrt)
+		ret = EFI_CALL(efi_free_pool(esrt));
+
+	esrt = new_esrt;
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * esrt_find_entry() - Obtain the ESRT entry for the image with GUID
+ * @img_fw_class.
+ *
+ * If the img_fw_class is not yet present in the ESRT, this function
+ * reserves the tail element of the current ESRT as the entry for that fw_class.
+ * The number of elements in the ESRT is updated in that case.
+ *
+ * @img_fw_class: the GUID of the FW image which ESRT entry we want to obtain.
+ *
+ * Return:
+ *  - A pointer to the ESRT entry for the image with GUID img_fw_class,
+ *  - NULL if:
+ *   - there is no more space in the ESRT,
+ *   - ESRT is not initialized,
+ */
+static
+struct efi_system_resource_entry *esrt_find_entry(efi_guid_t *img_fw_class)
+{
+	u32 filled_entries;
+	u32 max_entries;
+	struct efi_system_resource_entry *entry;
+
+	if (!esrt) {
+		EFI_PRINT("ESRT access before initialized\n");
+		return NULL;
+	}
+
+	filled_entries = esrt->fw_resource_count;
+	entry = esrt->entries;
+
+	/* Check if the image with img_fw_class is already in the ESRT. */
+	for (u32 idx = 0; idx < filled_entries; idx++) {
+		if (!guidcmp(&entry[idx].fw_class, img_fw_class)) {
+			EFI_PRINT("ESRT found entry for image %pUl@index %d\n",
+				  img_fw_class, idx);
+			return &entry[idx];
+		}
+	}
+
+	max_entries = esrt->fw_resource_count_max;
+	/*
+	 * Since the image with img_fw_class is not present in the ESRT, check
+	 * if ESRT is full before appending the new entry to it.
+	 */
+	if (filled_entries == max_entries) {
+		EFI_PRINT("ESRT full, this should not happen\n");
+		return NULL;
+	}
+
+	/*
+	 * This is a new entry for a fw image, increment the element
+	 * number in the table and set the fw_class field.
+	 */
+	esrt->fw_resource_count++;
+	entry[filled_entries].fw_class = *img_fw_class;
+	EFI_PRINT("ESRT allocated new entry for image %pUl at index %d\n",
+		  img_fw_class, filled_entries);
+
+	return &entry[filled_entries];
+}
+
+/**
+ * efi_esrt_add_from_fmp() - Populates a sequence of ESRT entries from the FW
+ * images in the FMP.
+ *
+ * @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_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 = EFI_CALL(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(efi_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 = EFI_CALL(fmp->get_image_info(fmp, &info_size, img_info,
+					   &desc_version, &desc_count,
+					   &desc_size, &package_version,
+					   &package_version_name));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to obtain the FMP image info\n");
+		goto out;
+	}
+
+	/*
+	 * Iterate over all the FW images in the FMP.
+	 */
+	for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
+		struct efi_firmware_image_descriptor *cur_img_info =
+			(struct efi_firmware_image_descriptor *)
+			((uintptr_t)img_info + desc_idx * desc_size);
+
+		/*
+		 * Obtain the ESRT entry for the FW image with fw_class
+		 * equal to cur_img_info->image_type_id.
+		 */
+		entry = esrt_find_entry(&cur_img_info->image_type_id);
+
+		if (entry) {
+			ret = efi_esrt_image_info_to_entry(cur_img_info, entry,
+							   desc_version,
+							   image_type, flags);
+			if (ret != EFI_SUCCESS)
+				EFI_PRINT("ESRT entry mismatches image_type\n");
+
+		} else {
+			EFI_PRINT("ESRT failed to add entry for %pUl\n",
+				  &cur_img_info->image_type_id);
+			continue;
+		}
+	}
+
+out:
+	EFI_CALL(efi_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_handler *handler;
+
+	/*
+	 * 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 **)&base_handle));
+
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT There are no FMP instances\n");
+
+		ret = efi_esrt_allocate_install(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 = EFI_CALL(efi_search_protocol(*it_handle,
+						   &efi_guid_firmware_management_protocol,
+						   &handler));
+
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT Unable to find FMP handle (%d)\n",
+				  idx);
+			goto out;
+		}
+		fmp = handler->protocol_interface;
+
+		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size, NULL,
+						   &desc_version, &desc_count,
+						   &desc_size, &package_version,
+						   &package_version_name));
+
+		if (ret != EFI_BUFFER_TOO_SMALL) {
+			/*
+			 * An input of info_size=0 should always lead
+			 * fmp->get_image_info to return BUFFER_TO_SMALL.
+			 */
+			EFI_PRINT("ESRT erroneous FMP implementation\n");
+			ret = EFI_INVALID_PARAMETER;
+			goto out;
+		}
+
+		ret = EFI_CALL(efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
+						 (void **)&img_info));
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT failed to allocate memory for image info\n");
+			goto out;
+		}
+
+		/*
+		 * Calls to a FMP get_image_info method do not return the
+		 * desc_count value if the return status differs from EFI_SUCCESS.
+		 * We need to repeat the call to get_image_info with a properly
+		 * sized buffer in order to obtain the real number of images
+		 * handled by the FMP.
+		 */
+		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size, img_info,
+						   &desc_version, &desc_count,
+						   &desc_size, &package_version,
+						   &package_version_name));
+
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT failed to obtain image info from FMP\n");
+			EFI_CALL(efi_free_pool(img_info));
+			goto out;
+		}
+
+		num_entries += desc_count;
+
+		EFI_CALL(efi_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(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 = EFI_CALL(efi_search_protocol(*it_handle,
+						   &efi_guid_firmware_management_protocol,
+						   &handler));
+
+		if (ret != EFI_SUCCESS) {
+			EFI_PRINT("ESRT unable to find FMP handle (%d)\n",
+				  idx);
+			break;
+		}
+		fmp = handler->protocol_interface;
+
+		ret = efi_esrt_add_from_fmp(fmp);
+		if (ret != EFI_SUCCESS)
+			EFI_PRINT("ESRT failed to add FMP to the table\n");
+	}
+
+out:
+
+	EFI_CALL(efi_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;
+
+	EFI_ENTRY();
+
+	ret = efi_esrt_populate();
+	if (ret != EFI_SUCCESS)
+		EFI_PRINT("ESRT failed to populate ESRT entry\n");
+
+	EFI_EXIT(ret);
+}
+
+/**
+ * efi_esrt_register() - Install the ESRT system table.
+ *
+ * Return: status code
+ */
+efi_status_t efi_esrt_register(void)
+{
+	struct efi_event *ev = NULL;
+	void *registration;
+	efi_status_t ret;
+
+	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 = EFI_CALL(efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+					efi_esrt_new_fmp_notify, NULL, NULL, &ev));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to create event\n");
+		return ret;
+	}
+
+	ret = EFI_CALL(efi_register_protocol_notify(&efi_guid_firmware_management_protocol,
+						    ev, &registration));
+	if (ret != EFI_SUCCESS) {
+		EFI_PRINT("ESRT failed to register FMP callback\n");
+		return ret;
+	}
+
+	EFI_PRINT("ESRT table created\n");
+
+	return ret;
+}
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index b1c5125032..3c5cf9a435 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -227,6 +227,12 @@ efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
+	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
+		ret = efi_esrt_register();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
 		ret = efi_tcg2_register();
 		if (ret != EFI_SUCCESS)
-- 
2.17.1

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

* [PATCH RESEND 1/3 v5] efi: Add ESRT to the EFI system table
  2021-03-02 17:26 ` [PATCH RESEND 1/3 v5] efi: Add ESRT to the EFI system table Jose Marinho
@ 2021-03-02 18:11   ` Heinrich Schuchardt
  0 siblings, 0 replies; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-03-02 18:11 UTC (permalink / raw)
  To: u-boot

On 02.03.21 18:26, 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 if an FMP instance is uninstalled;
> - the fields image_type and flags are in the current implementation left
> undefined. Setting these values will require a per-platform function
> that returns the image_type/flags as a function of the image fw_class.

EDK II uses ESRT_FW_TYPE_SYSTEMFIRMWARE and ESRT_FW_TYPE_DEVICEFIRMWARE.

Our FMP implementations use EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID.
Can't we simply say if the FMP identifies itself with
EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID than it is
ESRT_FW_TYPE_SYSTEMFIRMWARE?

We can do that in a future patch.

>
> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
>
> CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
>
> ---
>  cmd/efidebug.c                |   4 +
>  include/efi_api.h             |  21 ++
>  include/efi_loader.h          |  24 ++
>  lib/efi_loader/Kconfig        |   7 +
>  lib/efi_loader/Makefile       |   1 +
>  lib/efi_loader/efi_boottime.c |   7 +-
>  lib/efi_loader/efi_capsule.c  |   8 +
>  lib/efi_loader/efi_esrt.c     | 510 ++++++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_setup.c    |   6 +
>  9 files changed, 584 insertions(+), 4 deletions(-)
>  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,
> +	},

Configuration table GUIDs are at the end of the list.

>  	{
>  		"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..ae450ffc73 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;
> @@ -552,6 +554,10 @@ struct efi_simple_file_system_protocol *efi_simple_file_system(
>  /* open file from device-path: */
>  struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp);
>
> +/* Registers a callback function for a notification event. */
> +efi_status_t EFIAPI efi_register_protocol_notify(const efi_guid_t *protocol,
> +						 struct efi_event *event,
> +						 void **registration);
>  /**
>   * efi_size_in_pages() - convert size in bytes to size in pages
>   *
> @@ -884,4 +890,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_boottime.c b/lib/efi_loader/efi_boottime.c
> index 41b8949b04..8e8b0a9bc6 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1406,10 +1406,9 @@ out:
>   *
>   * Return: status code
>   */
> -static efi_status_t EFIAPI efi_register_protocol_notify(
> -						const efi_guid_t *protocol,
> -						struct efi_event *event,
> -						void **registration)
> +efi_status_t EFIAPI efi_register_protocol_notify(const efi_guid_t *protocol,
> +						 struct efi_event *event,
> +						 void **registration)
>  {
>  	struct efi_register_notify_event *item;
>  	efi_status_t ret = EFI_SUCCESS;
> 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());

ret = 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..c43e898d63
> --- /dev/null
> +++ b/lib/efi_loader/efi_esrt.c
> @@ -0,0 +1,510 @@
> +// 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.
> + *
> + * @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(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 = efi_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 = EFI_CALL(efi_install_configuration_table(&esrt_guid, (void *)new_esrt));

ret = efi_install_configuration_table(&esrt_guid, (void *)new_esrt);

Otherwise

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> +	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 = EFI_CALL(efi_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.
> + *
> + * @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_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 = EFI_CALL(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(efi_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 = EFI_CALL(fmp->get_image_info(fmp, &info_size, img_info,
> +					   &desc_version, &desc_count,
> +					   &desc_size, &package_version,
> +					   &package_version_name));
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to obtain the FMP image info\n");
> +		goto out;
> +	}
> +
> +	/*
> +	 * Iterate over all the FW images in the FMP.
> +	 */
> +	for (u32 desc_idx = 0; desc_idx < desc_count; desc_idx++) {
> +		struct efi_firmware_image_descriptor *cur_img_info =
> +			(struct efi_firmware_image_descriptor *)
> +			((uintptr_t)img_info + desc_idx * desc_size);
> +
> +		/*
> +		 * Obtain the ESRT entry for the FW image with fw_class
> +		 * equal to cur_img_info->image_type_id.
> +		 */
> +		entry = esrt_find_entry(&cur_img_info->image_type_id);
> +
> +		if (entry) {
> +			ret = efi_esrt_image_info_to_entry(cur_img_info, entry,
> +							   desc_version,
> +							   image_type, flags);
> +			if (ret != EFI_SUCCESS)
> +				EFI_PRINT("ESRT entry mismatches image_type\n");
> +
> +		} else {
> +			EFI_PRINT("ESRT failed to add entry for %pUl\n",
> +				  &cur_img_info->image_type_id);
> +			continue;
> +		}
> +	}
> +
> +out:
> +	EFI_CALL(efi_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_handler *handler;
> +
> +	/*
> +	 * 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 **)&base_handle));
> +
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT There are no FMP instances\n");
> +
> +		ret = efi_esrt_allocate_install(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 = EFI_CALL(efi_search_protocol(*it_handle,
> +						   &efi_guid_firmware_management_protocol,
> +						   &handler));
> +
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT Unable to find FMP handle (%d)\n",
> +				  idx);
> +			goto out;
> +		}
> +		fmp = handler->protocol_interface;
> +
> +		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size, NULL,
> +						   &desc_version, &desc_count,
> +						   &desc_size, &package_version,
> +						   &package_version_name));
> +
> +		if (ret != EFI_BUFFER_TOO_SMALL) {
> +			/*
> +			 * An input of info_size=0 should always lead
> +			 * fmp->get_image_info to return BUFFER_TO_SMALL.
> +			 */
> +			EFI_PRINT("ESRT erroneous FMP implementation\n");
> +			ret = EFI_INVALID_PARAMETER;
> +			goto out;
> +		}
> +
> +		ret = EFI_CALL(efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
> +						 (void **)&img_info));
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT failed to allocate memory for image info\n");
> +			goto out;
> +		}
> +
> +		/*
> +		 * Calls to a FMP get_image_info method do not return the
> +		 * desc_count value if the return status differs from EFI_SUCCESS.
> +		 * We need to repeat the call to get_image_info with a properly
> +		 * sized buffer in order to obtain the real number of images
> +		 * handled by the FMP.
> +		 */
> +		ret = EFI_CALL(fmp->get_image_info(fmp, &info_size, img_info,
> +						   &desc_version, &desc_count,
> +						   &desc_size, &package_version,
> +						   &package_version_name));
> +
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT failed to obtain image info from FMP\n");
> +			EFI_CALL(efi_free_pool(img_info));
> +			goto out;
> +		}
> +
> +		num_entries += desc_count;
> +
> +		EFI_CALL(efi_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(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 = EFI_CALL(efi_search_protocol(*it_handle,
> +						   &efi_guid_firmware_management_protocol,
> +						   &handler));
> +
> +		if (ret != EFI_SUCCESS) {
> +			EFI_PRINT("ESRT unable to find FMP handle (%d)\n",
> +				  idx);
> +			break;
> +		}
> +		fmp = handler->protocol_interface;
> +
> +		ret = efi_esrt_add_from_fmp(fmp);
> +		if (ret != EFI_SUCCESS)
> +			EFI_PRINT("ESRT failed to add FMP to the table\n");
> +	}
> +
> +out:
> +
> +	EFI_CALL(efi_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;
> +
> +	EFI_ENTRY();
> +
> +	ret = efi_esrt_populate();
> +	if (ret != EFI_SUCCESS)
> +		EFI_PRINT("ESRT failed to populate ESRT entry\n");
> +
> +	EFI_EXIT(ret);
> +}
> +
> +/**
> + * efi_esrt_register() - Install the ESRT system table.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_esrt_register(void)
> +{
> +	struct efi_event *ev = NULL;
> +	void *registration;
> +	efi_status_t ret;
> +
> +	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 = EFI_CALL(efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +					efi_esrt_new_fmp_notify, NULL, NULL, &ev));
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to create event\n");
> +		return ret;
> +	}
> +
> +	ret = EFI_CALL(efi_register_protocol_notify(&efi_guid_firmware_management_protocol,
> +						    ev, &registration));
> +	if (ret != EFI_SUCCESS) {
> +		EFI_PRINT("ESRT failed to register FMP callback\n");
> +		return ret;
> +	}
> +
> +	EFI_PRINT("ESRT table created\n");
> +
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index b1c5125032..3c5cf9a435 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -227,6 +227,12 @@ efi_status_t efi_init_obj_list(void)
>  	if (ret != EFI_SUCCESS)
>  		goto out;
>
> +	if (IS_ENABLED(CONFIG_EFI_ESRT)) {
> +		ret = efi_esrt_register();
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
>  	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
>  		ret = efi_tcg2_register();
>  		if (ret != EFI_SUCCESS)
>

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

* [PATCH 3/3 v5] efi: ESRT creation tests
  2021-03-02 12:13 ` [PATCH 3/3 v5] efi: ESRT creation tests Jose Marinho
@ 2021-03-02 18:30   ` Heinrich Schuchardt
  2021-03-02 23:48     ` AKASHI Takahiro
  0 siblings, 1 reply; 10+ messages in thread
From: Heinrich Schuchardt @ 2021-03-02 18:30 UTC (permalink / raw)
  To: u-boot

On 02.03.21 13:13, Jose Marinho wrote:
> This commmit exercises the ESRT creation in two tests.
>
> test 1:
>  A fake FMP, with TEST_ESRT_NUM_ENTRIES FW images, is installed in the
>  system leading to the corresponding ESRT entries being populated.
>  The ESRT entries are checked against the datastructure used to
>  initialize the FMP.
>
> test 1 invocation:
> add to sandbox_defconfig:
>   +CONFIG_EFI_SELFTEST=y

EFI_SELFTEST is active on many QEMU devices. So the test could run there.

>
>  make sandbox_capsule_defconfig all
>  ./u-boot -d arch/sandbox/dts/test.dtb
>  bootefi selftest
>
> test 2:
>  The test is part of test_efi_capsule_fw3.
>
>  In order to run the test the following must be added to
>  sandbox_defconfig:
>   +CONFIG_CMD_SF=y
>   +CONFIG_CMD_MEMORY=y
>   +CONFIG_CMD_FAT=y
>   +CONFIG_DFU=y
>
>  The ESRT is printed in the u-boot shell by calling efidebug esrt.
>  The test ensures that, after the capsule is installed, the  ESRT
>  contains entries with the GUIDs:
>   - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
>   - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
>
> test 2 invocation:
>  sudo ./test/py/test.py --bd sandbox -k capsule_fw3 -l --build
>
> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
>
> CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
>
> ---
>  lib/efi_selftest/Makefile                     |   2 +
>  lib/efi_selftest/efi_selftest_esrt.c          | 227 ++++++++++++++++++
>  .../test_efi_capsule/test_capsule_firmware.py |   4 +
>  3 files changed, 233 insertions(+)
>  create mode 100644 lib/efi_selftest/efi_selftest_esrt.c
>
> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> index 7d6ea30102..017b191a46 100644
> --- a/lib/efi_selftest/Makefile
> +++ b/lib/efi_selftest/Makefile
> @@ -72,6 +72,8 @@ ifeq ($(CONFIG_BLK)$(CONFIG_DOS_PARTITION),yy)
>  obj-y += efi_selftest_block_device.o
>  endif
>
> +obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
> +
>  targets += \
>  efi_miniapp_file_image_exception.h \
>  efi_miniapp_file_image_exit.h \
> diff --git a/lib/efi_selftest/efi_selftest_esrt.c b/lib/efi_selftest/efi_selftest_esrt.c
> new file mode 100644
> index 0000000000..b053b2e6c6
> --- /dev/null
> +++ b/lib/efi_selftest/efi_selftest_esrt.c
> @@ -0,0 +1,227 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + *  Test ESRT tables support
> + *
> + *  Copyright (C) 2021 Arm Ltd.
> + */
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <efi_selftest.h>
> +
> +// This value must not exceed 255.
> +// An FMP cannot contain more than 255 FW images.
> +#define TEST_ESRT_NUM_ENTRIES 255
> +
> +static
> +struct efi_firmware_image_descriptor static_img_info[TEST_ESRT_NUM_ENTRIES];
> +
> +static void efi_test_esrt_init_info(void)
> +{
> +	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++) {
> +		static_img_info[idx].image_index = idx;
> +
> +		// Note: the 16 byte value present in
> +		// static_img_info[idx].image_type_id is not strictly a GUID.
> +		// The value is used for the sake of code testing.
> +		static_img_info[idx].image_type_id.b[0] = idx;
> +
> +		static_img_info[idx].image_id = 0;
> +		static_img_info[idx].image_id_name = NULL;
> +		static_img_info[idx].version = 0;
> +		static_img_info[idx].version_name = NULL;
> +		static_img_info[idx].size = 0;
> +		static_img_info[idx].lowest_supported_image_version = 1;
> +		static_img_info[idx].last_attempt_version = 2;
> +		static_img_info[idx].last_attempt_status = 3;
> +		static_img_info[idx].hardware_instance = 1;
> +	}
> +}
> +
> +static efi_status_t
> +EFIAPI efi_test_fmp_get_image_info(struct efi_firmware_management_protocol *this,
> +				   efi_uintn_t *image_info_size,
> +				   struct efi_firmware_image_descriptor *image_info,
> +				   u32 *descriptor_version,
> +				   u8 *descriptor_count,
> +				   efi_uintn_t *descriptor_size,
> +				   u32 *package_version,
> +				   u16 **package_version_name)
> +{
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	if (!image_info_size)
> +		return EFI_INVALID_PARAMETER;
> +
> +	if (descriptor_version)
> +		*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> +	if (descriptor_count)
> +		*descriptor_count = TEST_ESRT_NUM_ENTRIES;
> +	if (descriptor_size)
> +		*descriptor_size = sizeof(*image_info);
> +	if (package_version)
> +		*package_version = 0xffffffff;
> +	if (package_version_name)
> +		*package_version_name = NULL;
> +
> +	if (*image_info_size < sizeof(*image_info)) {
> +		*image_info_size = *descriptor_size * *descriptor_count;
> +		return EFI_BUFFER_TOO_SMALL;
> +	}
> +
> +	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
> +		image_info[idx] = static_img_info[idx];
> +
> +	return ret;
> +}
> +
> +static struct efi_firmware_management_protocol efi_test_fmp = {
> +	.get_image_info = efi_test_fmp_get_image_info,
> +	.get_image = NULL,
> +	.set_image = NULL,
> +	.check_image = NULL,
> +	.get_package_info = NULL,
> +	.set_package_info = NULL,
> +};
> +
> +static void *lib_test_get_esrt(void)
> +{
> +	for (int idx = 0; idx < systab.nr_tables; idx++)
> +		if (!guidcmp(&efi_esrt_guid, &systab.tables[idx].guid))
> +			return systab.tables[idx].table;
> +
> +	return NULL;
> +}
> +
> +static bool lib_test_check_uuid_entry(struct efi_system_resource_table *esrt,
> +				      struct efi_firmware_image_descriptor
> +				      *img_info)
> +{
> +	const u32 filled_entries = esrt->fw_resource_count;
> +	struct efi_system_resource_entry *entry = esrt->entries;
> +
> +	for (u32 idx = 0; idx < filled_entries; idx++) {
> +		if (!guidcmp(&entry[idx].fw_class, &img_info->image_type_id)) {
> +			if (entry[idx].fw_version != img_info->version)
> +				return false;
> +
> +			if (entry[idx].lowest_supported_fw_version !=
> +				img_info->lowest_supported_image_version)
> +				return false;
> +
> +			if (entry[idx].last_attempt_version !=
> +				img_info->last_attempt_version)
> +				return false;
> +
> +			if (entry[idx].last_attempt_status !=
> +				img_info->last_attempt_status)
> +				return false;
> +
> +			/*
> +			 * The entry with fw_class = img_uuid matches with the
> +			 * remainder fmp input.
> +			 */
> +			return true;
> +		}
> +	}
> +
> +	/* There exists no entry with fw_class equal to img_uuid in the ESRT. */
> +	return false;
> +}
> +
> +/*
> + * Setup unit test.
> + *
> + * Initialize the test FMP datastructure.
> + * Install the FMP in the system.
> + *
> + * @handle:	handle of the loaded image
> + * @systable:	system table
> + * @return:	EFI_ST_SUCCESS for success
> + */
> +static int setup(const efi_handle_t handle,
> +		 const struct efi_system_table *systable)
> +{
> +	efi_status_t ret = EFI_SUCCESS;
> +	struct efi_boot_services *bt = systab.boottime;

Please, use the value from systable. We want to be able to make the
selftests a standalone UEFI application.

> +
> +	if (!bt)
> +		return EFI_ST_FAILURE;
> +
> +	efi_test_esrt_init_info();
> +
> +	ret = EFI_CALL(bt->install_multiple_protocol_interfaces

No EFI_CALL here. You are in the UEFI world.

> +		(&efi_root,
> +		 &efi_guid_firmware_management_protocol,
> +		 &efi_test_fmp,
> +		 NULL));
> +
> +	if (ret != EFI_SUCCESS)
> +		return EFI_ST_FAILURE;
> +
> +	return EFI_ST_SUCCESS;
> +}
> +
> +/*
> + * Tear down unit test.
> + *
> + * Uninstall the test FMP.
> + *
> + * @return:	EFI_ST_SUCCESS for success
> + */
> +static int teardown(void)
> +{
> +	efi_status_t ret = EFI_SUCCESS;
> +	struct efi_boot_services *bt = systab.boottime;
> +
> +	if (!bt)

Please use the value from setup's systable parameter.

> +		return EFI_ST_FAILURE;
> +
> +	ret = EFI_CALL(bt->uninstall_multiple_protocol_interfaces

No EFI_CALL here. You are in the UEFI world.

> +		(efi_root, &efi_guid_firmware_management_protocol,
> +		 &efi_test_fmp, NULL));
> +
> +	if (ret != EFI_SUCCESS)
> +		return EFI_ST_FAILURE;
> +
> +	return EFI_ST_SUCCESS;
> +}
> +
> +static int execute(void)
> +{
> +	struct efi_system_resource_table *esrt;
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	esrt = lib_test_get_esrt();
> +	if (!esrt)
> +		return EFI_ST_FAILURE;
> +
> +	if (esrt->fw_resource_count != TEST_ESRT_NUM_ENTRIES)
> +		return EFI_ST_FAILURE;
> +
> +	/* Update the ESRT. */
> +	ret = efi_esrt_populate();

This is not an EFI API function you cannot call it here.
The ESRT is already populated by your registered function. There is no
need to call it.

> +	if (ret != EFI_SUCCESS)

Please, write a message with efi_st_error() before returning the error
status.

> +		return EFI_ST_FAILURE;
> +
> +	esrt = lib_test_get_esrt();

You have to the the configuration table from the system table. Cf.
lib/efi_selftest/efi_selftest_fdt.c

> +	if (!esrt)
> +		return EFI_ST_FAILURE;
> +
> +	/* Verify that the number of images remains the same. */
> +	if (esrt->fw_resource_count != TEST_ESRT_NUM_ENTRIES)
> +		return EFI_ST_FAILURE;

You may already have other FMP protocols installed before 'bootefi
selftest' is invoked.

You have to count before installing the new FMP protocol and compare
that number to the number of instances after installing the new FMP
protocols.

> +
> +	for (u32 idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
> +		if (!lib_test_check_uuid_entry(esrt, &static_img_info[idx]))

Please, write a message with efi_st_error() before returning the error
status.

> +			return EFI_ST_FAILURE;
> +
> +	return EFI_ST_SUCCESS;
> +}
> +
> +EFI_UNIT_TEST(esrt) = {
> +	.name = "esrt",
> +	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
> +	.setup = setup,
> +	.execute = execute,
> +	.teardown = teardown,
> +};
> diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> index f006fa95d6..3a7c2e1ac8 100644
> --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> @@ -229,6 +229,10 @@ class TestEfiCapsuleFirmwareFit(object):
>                  output = u_boot_console.run_command(
>                      'env print -e -all Capsule0000')
>
> +            output = u_boot_console.run_command_list(['efidebug capsule esrt'])

Please, add a comment that this is EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID.

> +            assert 'AE13FF2D-9AD4-4E25-9AC8-6D80B3B22147' in ''.join(output)

This seems to be EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID.

> +            assert 'E2BB9C06-70E9-4B14-97A3-5A7913176E3F' in ''.join(output)

As this is a separate test it could be in a separate patch.

Best regards

Heinrich

> +
>              output = u_boot_console.run_command_list([
>                  'host bind 0 %s' % disk_img,
>                  'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
>

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

* [PATCH 3/3 v5] efi: ESRT creation tests
  2021-03-02 18:30   ` Heinrich Schuchardt
@ 2021-03-02 23:48     ` AKASHI Takahiro
  2021-03-03  9:34       ` Jose Marinho
  0 siblings, 1 reply; 10+ messages in thread
From: AKASHI Takahiro @ 2021-03-02 23:48 UTC (permalink / raw)
  To: u-boot

On Tue, Mar 02, 2021 at 07:30:31PM +0100, Heinrich Schuchardt wrote:
> On 02.03.21 13:13, Jose Marinho wrote:
> > This commmit exercises the ESRT creation in two tests.
> >
> > test 1:
> >  A fake FMP, with TEST_ESRT_NUM_ENTRIES FW images, is installed in the
> >  system leading to the corresponding ESRT entries being populated.
> >  The ESRT entries are checked against the datastructure used to
> >  initialize the FMP.
> >
> > test 1 invocation:
> > add to sandbox_defconfig:
> >   +CONFIG_EFI_SELFTEST=y
> 
> EFI_SELFTEST is active on many QEMU devices. So the test could run there.
> 
> >
> >  make sandbox_capsule_defconfig all
> >  ./u-boot -d arch/sandbox/dts/test.dtb
> >  bootefi selftest
> >
> > test 2:
> >  The test is part of test_efi_capsule_fw3.
> >
> >  In order to run the test the following must be added to
> >  sandbox_defconfig:
> >   +CONFIG_CMD_SF=y
> >   +CONFIG_CMD_MEMORY=y
> >   +CONFIG_CMD_FAT=y
> >   +CONFIG_DFU=y
> >
> >  The ESRT is printed in the u-boot shell by calling efidebug esrt.
> >  The test ensures that, after the capsule is installed, the  ESRT
> >  contains entries with the GUIDs:
> >   - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
> >   - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
> >
> > test 2 invocation:
> >  sudo ./test/py/test.py --bd sandbox -k capsule_fw3 -l --build
> >
> > Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> >
> > CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> > CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> > CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > CC: Andre Przywara <andre.przywara@arm.com>
> > CC: Alexander Graf <agraf@csgraf.de>
> > CC: nd at arm.com
> >
> > ---
> >  lib/efi_selftest/Makefile                     |   2 +
> >  lib/efi_selftest/efi_selftest_esrt.c          | 227 ++++++++++++++++++
> >  .../test_efi_capsule/test_capsule_firmware.py |   4 +
> >  3 files changed, 233 insertions(+)
> >  create mode 100644 lib/efi_selftest/efi_selftest_esrt.c
> >
> > diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> > index 7d6ea30102..017b191a46 100644
> > --- a/lib/efi_selftest/Makefile
> > +++ b/lib/efi_selftest/Makefile
> > @@ -72,6 +72,8 @@ ifeq ($(CONFIG_BLK)$(CONFIG_DOS_PARTITION),yy)
> >  obj-y += efi_selftest_block_device.o
> >  endif
> >
> > +obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
> > +
> >  targets += \
> >  efi_miniapp_file_image_exception.h \
> >  efi_miniapp_file_image_exit.h \
> > diff --git a/lib/efi_selftest/efi_selftest_esrt.c b/lib/efi_selftest/efi_selftest_esrt.c
> > new file mode 100644
> > index 0000000000..b053b2e6c6
> > --- /dev/null
> > +++ b/lib/efi_selftest/efi_selftest_esrt.c
> > @@ -0,0 +1,227 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + *  Test ESRT tables support
> > + *
> > + *  Copyright (C) 2021 Arm Ltd.
> > + */
> > +#include <common.h>
> > +#include <efi_loader.h>
> > +#include <efi_selftest.h>
> > +
> > +// This value must not exceed 255.
> > +// An FMP cannot contain more than 255 FW images.
> > +#define TEST_ESRT_NUM_ENTRIES 255
> > +
> > +static
> > +struct efi_firmware_image_descriptor static_img_info[TEST_ESRT_NUM_ENTRIES];
> > +
> > +static void efi_test_esrt_init_info(void)
> > +{
> > +	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++) {
> > +		static_img_info[idx].image_index = idx;
> > +
> > +		// Note: the 16 byte value present in
> > +		// static_img_info[idx].image_type_id is not strictly a GUID.
> > +		// The value is used for the sake of code testing.
> > +		static_img_info[idx].image_type_id.b[0] = idx;
> > +
> > +		static_img_info[idx].image_id = 0;
> > +		static_img_info[idx].image_id_name = NULL;
> > +		static_img_info[idx].version = 0;
> > +		static_img_info[idx].version_name = NULL;
> > +		static_img_info[idx].size = 0;
> > +		static_img_info[idx].lowest_supported_image_version = 1;
> > +		static_img_info[idx].last_attempt_version = 2;
> > +		static_img_info[idx].last_attempt_status = 3;
> > +		static_img_info[idx].hardware_instance = 1;
> > +	}
> > +}
> > +
> > +static efi_status_t
> > +EFIAPI efi_test_fmp_get_image_info(struct efi_firmware_management_protocol *this,
> > +				   efi_uintn_t *image_info_size,
> > +				   struct efi_firmware_image_descriptor *image_info,
> > +				   u32 *descriptor_version,
> > +				   u8 *descriptor_count,
> > +				   efi_uintn_t *descriptor_size,
> > +				   u32 *package_version,
> > +				   u16 **package_version_name)
> > +{
> > +	efi_status_t ret = EFI_SUCCESS;
> > +
> > +	if (!image_info_size)
> > +		return EFI_INVALID_PARAMETER;
> > +
> > +	if (descriptor_version)
> > +		*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> > +	if (descriptor_count)
> > +		*descriptor_count = TEST_ESRT_NUM_ENTRIES;
> > +	if (descriptor_size)
> > +		*descriptor_size = sizeof(*image_info);
> > +	if (package_version)
> > +		*package_version = 0xffffffff;
> > +	if (package_version_name)
> > +		*package_version_name = NULL;
> > +
> > +	if (*image_info_size < sizeof(*image_info)) {
> > +		*image_info_size = *descriptor_size * *descriptor_count;
> > +		return EFI_BUFFER_TOO_SMALL;
> > +	}
> > +
> > +	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
> > +		image_info[idx] = static_img_info[idx];
> > +
> > +	return ret;
> > +}
> > +
> > +static struct efi_firmware_management_protocol efi_test_fmp = {
> > +	.get_image_info = efi_test_fmp_get_image_info,
> > +	.get_image = NULL,
> > +	.set_image = NULL,
> > +	.check_image = NULL,
> > +	.get_package_info = NULL,
> > +	.set_package_info = NULL,
> > +};
> > +
> > +static void *lib_test_get_esrt(void)
> > +{
> > +	for (int idx = 0; idx < systab.nr_tables; idx++)
> > +		if (!guidcmp(&efi_esrt_guid, &systab.tables[idx].guid))
> > +			return systab.tables[idx].table;
> > +
> > +	return NULL;
> > +}
> > +
> > +static bool lib_test_check_uuid_entry(struct efi_system_resource_table *esrt,
> > +				      struct efi_firmware_image_descriptor
> > +				      *img_info)
> > +{
> > +	const u32 filled_entries = esrt->fw_resource_count;
> > +	struct efi_system_resource_entry *entry = esrt->entries;
> > +
> > +	for (u32 idx = 0; idx < filled_entries; idx++) {
> > +		if (!guidcmp(&entry[idx].fw_class, &img_info->image_type_id)) {
> > +			if (entry[idx].fw_version != img_info->version)
> > +				return false;
> > +
> > +			if (entry[idx].lowest_supported_fw_version !=
> > +				img_info->lowest_supported_image_version)
> > +				return false;
> > +
> > +			if (entry[idx].last_attempt_version !=
> > +				img_info->last_attempt_version)
> > +				return false;
> > +
> > +			if (entry[idx].last_attempt_status !=
> > +				img_info->last_attempt_status)
> > +				return false;
> > +
> > +			/*
> > +			 * The entry with fw_class = img_uuid matches with the
> > +			 * remainder fmp input.
> > +			 */
> > +			return true;
> > +		}
> > +	}
> > +
> > +	/* There exists no entry with fw_class equal to img_uuid in the ESRT. */
> > +	return false;
> > +}
> > +
> > +/*
> > + * Setup unit test.
> > + *
> > + * Initialize the test FMP datastructure.
> > + * Install the FMP in the system.
> > + *
> > + * @handle:	handle of the loaded image
> > + * @systable:	system table
> > + * @return:	EFI_ST_SUCCESS for success
> > + */
> > +static int setup(const efi_handle_t handle,
> > +		 const struct efi_system_table *systable)
> > +{
> > +	efi_status_t ret = EFI_SUCCESS;
> > +	struct efi_boot_services *bt = systab.boottime;
> 
> Please, use the value from systable. We want to be able to make the
> selftests a standalone UEFI application.
> 
> > +
> > +	if (!bt)
> > +		return EFI_ST_FAILURE;
> > +
> > +	efi_test_esrt_init_info();
> > +
> > +	ret = EFI_CALL(bt->install_multiple_protocol_interfaces
> 
> No EFI_CALL here. You are in the UEFI world.
> 
> > +		(&efi_root,
> > +		 &efi_guid_firmware_management_protocol,
> > +		 &efi_test_fmp,
> > +		 NULL));
> > +
> > +	if (ret != EFI_SUCCESS)
> > +		return EFI_ST_FAILURE;
> > +
> > +	return EFI_ST_SUCCESS;
> > +}
> > +
> > +/*
> > + * Tear down unit test.
> > + *
> > + * Uninstall the test FMP.
> > + *
> > + * @return:	EFI_ST_SUCCESS for success
> > + */
> > +static int teardown(void)
> > +{
> > +	efi_status_t ret = EFI_SUCCESS;
> > +	struct efi_boot_services *bt = systab.boottime;
> > +
> > +	if (!bt)
> 
> Please use the value from setup's systable parameter.
> 
> > +		return EFI_ST_FAILURE;
> > +
> > +	ret = EFI_CALL(bt->uninstall_multiple_protocol_interfaces
> 
> No EFI_CALL here. You are in the UEFI world.
> 
> > +		(efi_root, &efi_guid_firmware_management_protocol,
> > +		 &efi_test_fmp, NULL));
> > +
> > +	if (ret != EFI_SUCCESS)
> > +		return EFI_ST_FAILURE;
> > +
> > +	return EFI_ST_SUCCESS;
> > +}
> > +
> > +static int execute(void)
> > +{
> > +	struct efi_system_resource_table *esrt;
> > +	efi_status_t ret = EFI_SUCCESS;
> > +
> > +	esrt = lib_test_get_esrt();
> > +	if (!esrt)
> > +		return EFI_ST_FAILURE;
> > +
> > +	if (esrt->fw_resource_count != TEST_ESRT_NUM_ENTRIES)
> > +		return EFI_ST_FAILURE;
> > +
> > +	/* Update the ESRT. */
> > +	ret = efi_esrt_populate();
> 
> This is not an EFI API function you cannot call it here.
> The ESRT is already populated by your registered function. There is no
> need to call it.
> 
> > +	if (ret != EFI_SUCCESS)
> 
> Please, write a message with efi_st_error() before returning the error
> status.
> 
> > +		return EFI_ST_FAILURE;
> > +
> > +	esrt = lib_test_get_esrt();
> 
> You have to the the configuration table from the system table. Cf.
> lib/efi_selftest/efi_selftest_fdt.c
> 
> > +	if (!esrt)
> > +		return EFI_ST_FAILURE;
> > +
> > +	/* Verify that the number of images remains the same. */
> > +	if (esrt->fw_resource_count != TEST_ESRT_NUM_ENTRIES)
> > +		return EFI_ST_FAILURE;
> 
> You may already have other FMP protocols installed before 'bootefi
> selftest' is invoked.
> 
> You have to count before installing the new FMP protocol and compare
> that number to the number of instances after installing the new FMP
> protocols.
> 
> > +
> > +	for (u32 idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
> > +		if (!lib_test_check_uuid_entry(esrt, &static_img_info[idx]))
> 
> Please, write a message with efi_st_error() before returning the error
> status.
> 
> > +			return EFI_ST_FAILURE;
> > +
> > +	return EFI_ST_SUCCESS;
> > +}
> > +
> > +EFI_UNIT_TEST(esrt) = {
> > +	.name = "esrt",
> > +	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
> > +	.setup = setup,
> > +	.execute = execute,
> > +	.teardown = teardown,
> > +};
> > diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> > index f006fa95d6..3a7c2e1ac8 100644
> > --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> > +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> > @@ -229,6 +229,10 @@ class TestEfiCapsuleFirmwareFit(object):
> >                  output = u_boot_console.run_command(
> >                      'env print -e -all Capsule0000')
> >
> > +            output = u_boot_console.run_command_list(['efidebug capsule esrt'])
> 
> Please, add a comment that this is EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID.
> 
> > +            assert 'AE13FF2D-9AD4-4E25-9AC8-6D80B3B22147' in ''.join(output)
> 
> This seems to be EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID.
> 
> > +            assert 'E2BB9C06-70E9-4B14-97A3-5A7913176E3F' in ''.join(output)

Those values are constant as they are "fw_class".
How do you confirm that data in an entry is updated?

-Takahiro Akashi


> As this is a separate test it could be in a separate patch.
> 
> Best regards
> 
> Heinrich
> 
> > +
> >              output = u_boot_console.run_command_list([
> >                  'host bind 0 %s' % disk_img,
> >                  'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
> >
> 

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

* [PATCH 2/3 v5] cmd: efi: ESRT table debug print
  2021-03-02 12:13 ` [PATCH 2/3 v5] cmd: efi: ESRT table debug print Jose Marinho
@ 2021-03-03  0:01   ` AKASHI Takahiro
  0 siblings, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2021-03-03  0:01 UTC (permalink / raw)
  To: u-boot

On Tue, Mar 02, 2021 at 12:13:53PM +0000, Jose Marinho wrote:
> This commit enables the ESRT printing from the u-boot shell by invoking:
> - efidebug capsule esrt
> 
> Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> 
> CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Andre Przywara <andre.przywara@arm.com>
> CC: Alexander Graf <agraf@csgraf.de>
> CC: nd at arm.com
> 
> ---
>  cmd/efidebug.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index a7dace2f80..5a9ff2bd9a 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -129,6 +129,61 @@ static int do_efi_capsule_show(struct cmd_tbl *cmdtp, int flag,
>  	return CMD_RET_SUCCESS;
>  }
>  
> +#ifdef CONFIG_EFI_ESRT
> +/**
> + * do_efi_capsule_esrt() - manage UEFI capsules
> + *
> + * @cmdtp:	Command table
> + * @flag:	Command flag
> + * @argc:	Number of arguments
> + * @argv:	Argument array
> + * Return:	CMD_RET_SUCCESS on success,
> + *		CMD_RET_USAGE or CMD_RET_RET_FAILURE on failure
> + *
> + * Implement efidebug "capsule esrt" sub-command.
> + * The prints the current ESRT table.
> + *
> + *     efidebug capsule esrt

Strictly speaking, ESRT does not always require capsules
to be used as "23.4.2 ESRT and Firmware Management Protocol" says.

> + */
> +static int do_efi_capsule_esrt(struct cmd_tbl *cmdtp, int flag,
> +			       int argc, char * const argv[])
> +{
> +	struct efi_system_resource_table *esrt = NULL;
> +
> +	if (argc != 1)
> +		return CMD_RET_USAGE;
> +
> +	for (int idx = 0; idx < systab.nr_tables; idx++)
> +		if (!guidcmp(&efi_esrt_guid, &systab.tables[idx].guid))
> +			esrt = (struct efi_system_resource_table *)systab.tables[idx].table;
> +
> +	if (!esrt)
> +		return CMD_RET_FAILURE;

Is this really a failure?
Even so, it would be nice to print a verbose message here.

> +
> +	printf("========================================\n");
> +	printf("ESRT: fw_resource_count=%d\n", esrt->fw_resource_count);
> +	printf("ESRT: fw_resource_count_max=%d\n", esrt->fw_resource_count_max);
> +	printf("ESRT: fw_resource_version=%lld\n", esrt->fw_resource_version);
> +
> +	for (int idx = 0; idx < esrt->fw_resource_count; idx++) {
> +		printf("[entry %d]==============================\n", idx);
> +		printf("ESRT: fw_class=%pUL\n", &esrt->entries[idx].fw_class);

More symbolic expression would be friendly.

> +		printf("ESRT: fw_type=%d\n", esrt->entries[idx].fw_type);
> +		printf("ESRT: fw_version=%d\n", esrt->entries[idx].fw_version);
> +		printf("ESRT: lowest_supported_fw_version=%d\n",
> +		       esrt->entries[idx].lowest_supported_fw_version);
> +		printf("ESRT: capsule_flags=%d\n",
> +		       esrt->entries[idx].capsule_flags);
> +		printf("ESRT: last_attempt_version=%d\n",
> +		       esrt->entries[idx].last_attempt_version);
> +		printf("ESRT: last_attempt_status=%d\n",
> +		       esrt->entries[idx].last_attempt_status);

ditto.

> +	}
> +	printf("========================================\n");
> +
> +	return CMD_RET_SUCCESS;
> +}
> +#endif /*  CONFIG_EFI_ESRT */
>  /**
>   * do_efi_capsule_res() - show a capsule update result
>   *
> @@ -221,6 +276,10 @@ static struct cmd_tbl cmd_efidebug_capsule_sub[] = {
>  			 "", ""),
>  	U_BOOT_CMD_MKENT(show, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_show,
>  			 "", ""),
> +#ifdef CONFIG_EFI_ESRT
> +	U_BOOT_CMD_MKENT(esrt, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_esrt,
> +			 "", ""),
> +#endif
>  	U_BOOT_CMD_MKENT(disk-update, 0, 0, do_efi_capsule_on_disk_update,
>  			 "", ""),
>  	U_BOOT_CMD_MKENT(result, CONFIG_SYS_MAXARGS, 1, do_efi_capsule_res,
> @@ -256,6 +315,7 @@ static int do_efi_capsule(struct cmd_tbl *cmdtp, int flag,
>  
>  	return cp->cmd(cmdtp, flag, argc, argv);
>  }
> +
>  #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
>  
>  /**
> @@ -1580,6 +1640,10 @@ static char efidebug_help_text[] =
>  	"  - show capsule information\n"
>  	"efidebug capsule result [<capsule result var>]\n"
>  	"  - show a capsule update result\n"
> +#ifdef CONFIG_EFI_ESRT
> +	"efidebug capsule esrt\n"
> +	"  - print the ESRT\n"
> +#endif
>  	"\n"
>  #endif
>  	"efidebug devices\n"
> -- 
> 2.17.1
> 

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

* [PATCH 3/3 v5] efi: ESRT creation tests
  2021-03-02 23:48     ` AKASHI Takahiro
@ 2021-03-03  9:34       ` Jose Marinho
  0 siblings, 0 replies; 10+ messages in thread
From: Jose Marinho @ 2021-03-03  9:34 UTC (permalink / raw)
  To: u-boot

Hi Takahiro Akashi,

Thank you for reviewing these tests.

> -----Original Message-----
> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Sent: 02 March 2021 23:49
> To: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jose Marinho <Jose.Marinho@arm.com>; u-boot at lists.denx.de;
> Sughosh Ganu <sughosh.ganu@linaro.org>; Ilias Apalodimas
> <ilias.apalodimas@linaro.org>; Andre Przywara <Andre.Przywara@arm.com>;
> Alexander Graf <agraf@csgraf.de>; nd <nd@arm.com>
> Subject: Re: [PATCH 3/3 v5] efi: ESRT creation tests
> 
> On Tue, Mar 02, 2021 at 07:30:31PM +0100, Heinrich Schuchardt wrote:
> > On 02.03.21 13:13, Jose Marinho wrote:
> > > This commmit exercises the ESRT creation in two tests.
> > >
> > > test 1:
> > >  A fake FMP, with TEST_ESRT_NUM_ENTRIES FW images, is installed in
> > > the  system leading to the corresponding ESRT entries being populated.
> > >  The ESRT entries are checked against the datastructure used to
> > > initialize the FMP.
> > >
> > > test 1 invocation:
> > > add to sandbox_defconfig:
> > >   +CONFIG_EFI_SELFTEST=y
> >
> > EFI_SELFTEST is active on many QEMU devices. So the test could run there.
> >
> > >
> > >  make sandbox_capsule_defconfig all
> > >  ./u-boot -d arch/sandbox/dts/test.dtb  bootefi selftest
> > >
> > > test 2:
> > >  The test is part of test_efi_capsule_fw3.
> > >
> > >  In order to run the test the following must be added to
> > >  sandbox_defconfig:
> > >   +CONFIG_CMD_SF=y
> > >   +CONFIG_CMD_MEMORY=y
> > >   +CONFIG_CMD_FAT=y
> > >   +CONFIG_DFU=y
> > >
> > >  The ESRT is printed in the u-boot shell by calling efidebug esrt.
> > >  The test ensures that, after the capsule is installed, the  ESRT
> > > contains entries with the GUIDs:
> > >   - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
> > >   - EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
> > >
> > > test 2 invocation:
> > >  sudo ./test/py/test.py --bd sandbox -k capsule_fw3 -l --build
> > >
> > > Signed-off-by: Jose Marinho <jose.marinho@arm.com>
> > >
> > > CC: Heinrich Schuchardt	<xypron.glpk@gmx.de>
> > > CC: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > CC: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > CC: Andre Przywara <andre.przywara@arm.com>
> > > CC: Alexander Graf <agraf@csgraf.de>
> > > CC: nd at arm.com
> > >
> > > ---
> > >  lib/efi_selftest/Makefile                     |   2 +
> > >  lib/efi_selftest/efi_selftest_esrt.c          | 227 ++++++++++++++++++
> > >  .../test_efi_capsule/test_capsule_firmware.py |   4 +
> > >  3 files changed, 233 insertions(+)
> > >  create mode 100644 lib/efi_selftest/efi_selftest_esrt.c
> > >
> > > diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> > > index 7d6ea30102..017b191a46 100644
> > > --- a/lib/efi_selftest/Makefile
> > > +++ b/lib/efi_selftest/Makefile
> > > @@ -72,6 +72,8 @@ ifeq ($(CONFIG_BLK)$(CONFIG_DOS_PARTITION),yy)
> > >  obj-y += efi_selftest_block_device.o  endif
> > >
> > > +obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
> > > +
> > >  targets += \
> > >  efi_miniapp_file_image_exception.h \  efi_miniapp_file_image_exit.h
> > > \ diff --git a/lib/efi_selftest/efi_selftest_esrt.c
> > > b/lib/efi_selftest/efi_selftest_esrt.c
> > > new file mode 100644
> > > index 0000000000..b053b2e6c6
> > > --- /dev/null
> > > +++ b/lib/efi_selftest/efi_selftest_esrt.c
> > > @@ -0,0 +1,227 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + *  Test ESRT tables support
> > > + *
> > > + *  Copyright (C) 2021 Arm Ltd.
> > > + */
> > > +#include <common.h>
> > > +#include <efi_loader.h>
> > > +#include <efi_selftest.h>
> > > +
> > > +// This value must not exceed 255.
> > > +// An FMP cannot contain more than 255 FW images.
> > > +#define TEST_ESRT_NUM_ENTRIES 255
> > > +
> > > +static
> > > +struct efi_firmware_image_descriptor
> > > +static_img_info[TEST_ESRT_NUM_ENTRIES];
> > > +
> > > +static void efi_test_esrt_init_info(void) {
> > > +	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++) {
> > > +		static_img_info[idx].image_index = idx;
> > > +
> > > +		// Note: the 16 byte value present in
> > > +		// static_img_info[idx].image_type_id is not strictly a GUID.
> > > +		// The value is used for the sake of code testing.
> > > +		static_img_info[idx].image_type_id.b[0] = idx;
> > > +
> > > +		static_img_info[idx].image_id = 0;
> > > +		static_img_info[idx].image_id_name = NULL;
> > > +		static_img_info[idx].version = 0;
> > > +		static_img_info[idx].version_name = NULL;
> > > +		static_img_info[idx].size = 0;
> > > +		static_img_info[idx].lowest_supported_image_version = 1;
> > > +		static_img_info[idx].last_attempt_version = 2;
> > > +		static_img_info[idx].last_attempt_status = 3;
> > > +		static_img_info[idx].hardware_instance = 1;
> > > +	}
> > > +}
> > > +
> > > +static efi_status_t
> > > +EFIAPI efi_test_fmp_get_image_info(struct
> efi_firmware_management_protocol *this,
> > > +				   efi_uintn_t *image_info_size,
> > > +				   struct efi_firmware_image_descriptor
> *image_info,
> > > +				   u32 *descriptor_version,
> > > +				   u8 *descriptor_count,
> > > +				   efi_uintn_t *descriptor_size,
> > > +				   u32 *package_version,
> > > +				   u16 **package_version_name)
> > > +{
> > > +	efi_status_t ret = EFI_SUCCESS;
> > > +
> > > +	if (!image_info_size)
> > > +		return EFI_INVALID_PARAMETER;
> > > +
> > > +	if (descriptor_version)
> > > +		*descriptor_version =
> EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
> > > +	if (descriptor_count)
> > > +		*descriptor_count = TEST_ESRT_NUM_ENTRIES;
> > > +	if (descriptor_size)
> > > +		*descriptor_size = sizeof(*image_info);
> > > +	if (package_version)
> > > +		*package_version = 0xffffffff;
> > > +	if (package_version_name)
> > > +		*package_version_name = NULL;
> > > +
> > > +	if (*image_info_size < sizeof(*image_info)) {
> > > +		*image_info_size = *descriptor_size * *descriptor_count;
> > > +		return EFI_BUFFER_TOO_SMALL;
> > > +	}
> > > +
> > > +	for (int idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
> > > +		image_info[idx] = static_img_info[idx];
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static struct efi_firmware_management_protocol efi_test_fmp = {
> > > +	.get_image_info = efi_test_fmp_get_image_info,
> > > +	.get_image = NULL,
> > > +	.set_image = NULL,
> > > +	.check_image = NULL,
> > > +	.get_package_info = NULL,
> > > +	.set_package_info = NULL,
> > > +};
> > > +
> > > +static void *lib_test_get_esrt(void) {
> > > +	for (int idx = 0; idx < systab.nr_tables; idx++)
> > > +		if (!guidcmp(&efi_esrt_guid, &systab.tables[idx].guid))
> > > +			return systab.tables[idx].table;
> > > +
> > > +	return NULL;
> > > +}
> > > +
> > > +static bool lib_test_check_uuid_entry(struct
> efi_system_resource_table *esrt,
> > > +				      struct efi_firmware_image_descriptor
> > > +				      *img_info)
> > > +{
> > > +	const u32 filled_entries = esrt->fw_resource_count;
> > > +	struct efi_system_resource_entry *entry = esrt->entries;
> > > +
> > > +	for (u32 idx = 0; idx < filled_entries; idx++) {
> > > +		if (!guidcmp(&entry[idx].fw_class, &img_info-
> >image_type_id)) {
> > > +			if (entry[idx].fw_version != img_info->version)
> > > +				return false;
> > > +
> > > +			if (entry[idx].lowest_supported_fw_version !=
> > > +				img_info->lowest_supported_image_version)
> > > +				return false;
> > > +
> > > +			if (entry[idx].last_attempt_version !=
> > > +				img_info->last_attempt_version)
> > > +				return false;
> > > +
> > > +			if (entry[idx].last_attempt_status !=
> > > +				img_info->last_attempt_status)
> > > +				return false;
> > > +
> > > +			/*
> > > +			 * The entry with fw_class = img_uuid matches with
> the
> > > +			 * remainder fmp input.
> > > +			 */
> > > +			return true;
> > > +		}
> > > +	}
> > > +
> > > +	/* There exists no entry with fw_class equal to img_uuid in the ESRT.
> */
> > > +	return false;
> > > +}
> > > +
> > > +/*
> > > + * Setup unit test.
> > > + *
> > > + * Initialize the test FMP datastructure.
> > > + * Install the FMP in the system.
> > > + *
> > > + * @handle:	handle of the loaded image
> > > + * @systable:	system table
> > > + * @return:	EFI_ST_SUCCESS for success
> > > + */
> > > +static int setup(const efi_handle_t handle,
> > > +		 const struct efi_system_table *systable) {
> > > +	efi_status_t ret = EFI_SUCCESS;
> > > +	struct efi_boot_services *bt = systab.boottime;
> >
> > Please, use the value from systable. We want to be able to make the
> > selftests a standalone UEFI application.
> >
> > > +
> > > +	if (!bt)
> > > +		return EFI_ST_FAILURE;
> > > +
> > > +	efi_test_esrt_init_info();
> > > +
> > > +	ret = EFI_CALL(bt->install_multiple_protocol_interfaces
> >
> > No EFI_CALL here. You are in the UEFI world.
> >
> > > +		(&efi_root,
> > > +		 &efi_guid_firmware_management_protocol,
> > > +		 &efi_test_fmp,
> > > +		 NULL));
> > > +
> > > +	if (ret != EFI_SUCCESS)
> > > +		return EFI_ST_FAILURE;
> > > +
> > > +	return EFI_ST_SUCCESS;
> > > +}
> > > +
> > > +/*
> > > + * Tear down unit test.
> > > + *
> > > + * Uninstall the test FMP.
> > > + *
> > > + * @return:	EFI_ST_SUCCESS for success
> > > + */
> > > +static int teardown(void)
> > > +{
> > > +	efi_status_t ret = EFI_SUCCESS;
> > > +	struct efi_boot_services *bt = systab.boottime;
> > > +
> > > +	if (!bt)
> >
> > Please use the value from setup's systable parameter.
> >
> > > +		return EFI_ST_FAILURE;
> > > +
> > > +	ret = EFI_CALL(bt->uninstall_multiple_protocol_interfaces
> >
> > No EFI_CALL here. You are in the UEFI world.
> >
> > > +		(efi_root, &efi_guid_firmware_management_protocol,
> > > +		 &efi_test_fmp, NULL));
> > > +
> > > +	if (ret != EFI_SUCCESS)
> > > +		return EFI_ST_FAILURE;
> > > +
> > > +	return EFI_ST_SUCCESS;
> > > +}
> > > +
> > > +static int execute(void)
> > > +{
> > > +	struct efi_system_resource_table *esrt;
> > > +	efi_status_t ret = EFI_SUCCESS;
> > > +
> > > +	esrt = lib_test_get_esrt();
> > > +	if (!esrt)
> > > +		return EFI_ST_FAILURE;
> > > +
> > > +	if (esrt->fw_resource_count != TEST_ESRT_NUM_ENTRIES)
> > > +		return EFI_ST_FAILURE;
> > > +
> > > +	/* Update the ESRT. */
> > > +	ret = efi_esrt_populate();
> >
> > This is not an EFI API function you cannot call it here.
> > The ESRT is already populated by your registered function. There is no
> > need to call it.
> >
> > > +	if (ret != EFI_SUCCESS)
> >
> > Please, write a message with efi_st_error() before returning the error
> > status.
> >
> > > +		return EFI_ST_FAILURE;
> > > +
> > > +	esrt = lib_test_get_esrt();
> >
> > You have to the the configuration table from the system table. Cf.
> > lib/efi_selftest/efi_selftest_fdt.c
> >
> > > +	if (!esrt)
> > > +		return EFI_ST_FAILURE;
> > > +
> > > +	/* Verify that the number of images remains the same. */
> > > +	if (esrt->fw_resource_count != TEST_ESRT_NUM_ENTRIES)
> > > +		return EFI_ST_FAILURE;
> >
> > You may already have other FMP protocols installed before 'bootefi
> > selftest' is invoked.
> >
> > You have to count before installing the new FMP protocol and compare
> > that number to the number of instances after installing the new FMP
> > protocols.
> >
> > > +
> > > +	for (u32 idx = 0; idx < TEST_ESRT_NUM_ENTRIES; idx++)
> > > +		if (!lib_test_check_uuid_entry(esrt, &static_img_info[idx]))
> >
> > Please, write a message with efi_st_error() before returning the error
> > status.
> >
> > > +			return EFI_ST_FAILURE;
> > > +
> > > +	return EFI_ST_SUCCESS;
> > > +}
> > > +
> > > +EFI_UNIT_TEST(esrt) = {
> > > +	.name = "esrt",
> > > +	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
> > > +	.setup = setup,
> > > +	.execute = execute,
> > > +	.teardown = teardown,
> > > +};
> > > diff --git a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> > > b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> > > index f006fa95d6..3a7c2e1ac8 100644
> > > --- a/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> > > +++ b/test/py/tests/test_efi_capsule/test_capsule_firmware.py
> > > @@ -229,6 +229,10 @@ class TestEfiCapsuleFirmwareFit(object):
> > >                  output = u_boot_console.run_command(
> > >                      'env print -e -all Capsule0000')
> > >
> > > +            output = u_boot_console.run_command_list(['efidebug
> > > + capsule esrt'])
> >
> > Please, add a comment that this is
> EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID.
> >
> > > +            assert 'AE13FF2D-9AD4-4E25-9AC8-6D80B3B22147' in
> > > + ''.join(output)
> >
> > This seems to be EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID.
> >
> > > +            assert 'E2BB9C06-70E9-4B14-97A3-5A7913176E3F' in
> > > + ''.join(output)
> 
> Those values are constant as they are "fw_class".
> How do you confirm that data in an entry is updated?

The 2 asserts above are validating the correct ESRT construction.

We must to rely on the image versions changing in order to detect that an entry is updated.
The FIT and RAW get_image_info implementation currently returns 0 in the image version field.
With the current FIT and raw FMP implementations we lack the information to track that an entry has been updated.

Regards,

Jose

> 
> -Takahiro Akashi
> 
> 
> > As this is a separate test it could be in a separate patch.
> >
> > Best regards
> >
> > Heinrich
> >
> > > +
> > >              output = u_boot_console.run_command_list([
> > >                  'host bind 0 %s' % disk_img,
> > >                  'fatls host 0:1 %s' % CAPSULE_INSTALL_DIR])
> > >
> >

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

end of thread, other threads:[~2021-03-03  9:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 12:13 [PATCH 0/3 v5] Add ESRT and test ESRT creation Jose Marinho
2021-03-02 12:13 ` [PATCH 1/3 v5] efi: Add ESRT to the EFI system table Jose Marinho
2021-03-02 12:13 ` [PATCH 2/3 v5] cmd: efi: ESRT table debug print Jose Marinho
2021-03-03  0:01   ` AKASHI Takahiro
2021-03-02 12:13 ` [PATCH 3/3 v5] efi: ESRT creation tests Jose Marinho
2021-03-02 18:30   ` Heinrich Schuchardt
2021-03-02 23:48     ` AKASHI Takahiro
2021-03-03  9:34       ` Jose Marinho
2021-03-02 17:26 ` [PATCH RESEND 1/3 v5] efi: Add ESRT to the EFI system table Jose Marinho
2021-03-02 18:11   ` Heinrich Schuchardt

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