[PATCH 3/3 v5] efi: ESRT creation tests
Jose Marinho
Jose.Marinho at arm.com
Wed Mar 3 10:34:50 CET 2021
Hi Takahiro Akashi,
Thank you for reviewing these tests.
> -----Original Message-----
> From: AKASHI Takahiro <takahiro.akashi at linaro.org>
> Sent: 02 March 2021 23:49
> To: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Jose Marinho <Jose.Marinho at arm.com>; u-boot at lists.denx.de;
> Sughosh Ganu <sughosh.ganu at linaro.org>; Ilias Apalodimas
> <ilias.apalodimas at linaro.org>; Andre Przywara <Andre.Przywara at arm.com>;
> Alexander Graf <agraf at csgraf.de>; nd <nd at 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 at arm.com>
> > >
> > > CC: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > > CC: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > CC: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > > CC: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > CC: Andre Przywara <andre.przywara at arm.com>
> > > CC: Alexander Graf <agraf at csgraf.de>
> > > CC: nd at arm.com
> > >
> > > ---
> > > 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])
> > >
> >
More information about the U-Boot
mailing list