[PATCH 3/4 v6] efi: ESRT creation tests

[Heinrich Schuchardt]

On 3/8/21 12:29 PM, Jose Marinho wrote:
> This commmit exercises the ESRT creation in a EFI selftest.
>
>   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.
>
> Invocation from the sandbox platform:
> add to sandbox_defconfig:
>    +CONFIG_CMD_BOOTEFI_SELFTEST=y
>
>   make sandbox_capsule_defconfig all
>   ./u-boot -d arch/sandbox/dts/test.dtb
>   bootefi selftest
>
> 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
> Signed-off-by: Jose Marinho <jose.marinho at arm.com>
> ---
>   lib/efi_selftest/Makefile            |   2 +
>   lib/efi_selftest/efi_selftest_esrt.c | 264 +++++++++++++++++++++++++++
>   2 files changed, 266 insertions(+)
>   create mode 100644 lib/efi_selftest/efi_selftest_esrt.c
>
> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> index b02fd56e0a..58b27d6da3 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..d2b04b6a09
> --- /dev/null
> +++ b/lib/efi_selftest/efi_selftest_esrt.c
> @@ -0,0 +1,264 @@
> +// 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 const struct efi_system_table *local_systable;
> +
> +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)
> +{
> +	if (!local_systable) {
> +		efi_st_error("systable not found or uninitialized\n");
> +		return NULL;
> +	}

This cannot occur.

> +
> +	for (int idx = 0; idx < local_systable->nr_tables; idx++)
> +		if (!guidcmp(&efi_esrt_guid, &local_systable->tables[idx].guid))
> +			return local_systable->tables[idx].table;
> +
> +	return NULL;
> +}
> +

Please, provide a Sphinx style description of the function.

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

Why is the outside loop over the images and the inside loop over ESRT
entries? Wouldn't the other way round make more sense?

Execution of the test will fail because you are starting at index 0. You
should start at index base_entry_count.

> +		if (!guidcmp(&entry[idx].fw_class, &img_info->image_type_id)) {
> +			if (entry[idx].fw_version != img_info->version)

It would be helpful to indicate which field is wrong.

> +				return false;

Shouldn't you only check the entry with the matching image index?

> +
> +			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)
> +{
> +	local_systable = systable;
> +
> +	if (!local_systable) {
> +		efi_st_error("systable not found\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	efi_test_esrt_init_info();
> +
> +	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;
> +
> +	if (!local_systable) {
> +		efi_st_error("systable not found\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	bt = local_systable->boottime;
> +
> +	if (!bt) {
> +		efi_st_error("Cannot find boottime services structure\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	ret = bt->uninstall_multiple_protocol_interfaces
> +		(efi_root, &efi_guid_firmware_management_protocol,
> +		 &efi_test_fmp, NULL);
> +
> +	if (ret != EFI_SUCCESS) {
> +		efi_st_error("Failed to uninstall FMP\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	return EFI_ST_SUCCESS;
> +}
> +
> +static int execute(void)
> +{
> +	struct efi_system_resource_table *esrt;
> +	efi_status_t ret = EFI_SUCCESS;
> +	u32 base_entry_count;
> +	u32 entry_delta;
> +	struct efi_boot_services *bt;
> +
> +	if (!local_systable) {
> +		efi_st_error("systable not found\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	bt = local_systable->boottime;
> +
> +	if (!bt) {
> +		efi_st_error("Cannot find boottime services structure\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	esrt = lib_test_get_esrt();
> +	if (!esrt) {
> +		efi_st_error("ESRT table not present\n");
> +		return EFI_ST_FAILURE;
> +	}
> +	base_entry_count = esrt->fw_resource_count;
> +
> +	ret = bt->install_multiple_protocol_interfaces(&efi_root,

You should not access variables from outside efi_selftest here.

There is already an FMP installed on the root node. So
InstallMultipleProtocolInterfaces fails:

lib/efi_selftest/efi_selftest_esrt.c(232):
ERROR: Failed to install FMP

Instead pass the address of a pointer for a handle which is intialized
to zero and let InstallMultipleProtocolInterfaces create a new handle.

Best regards

Heinrich

> +						       &efi_guid_firmware_management_protocol,
> +						       &efi_test_fmp,
> +						       NULL);
> +
> +	if (ret != EFI_SUCCESS) {
> +		efi_st_error("Failed to install FMP\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	esrt = lib_test_get_esrt();
> +	if (!esrt) {
> +		efi_st_error("ESRT table not present\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	entry_delta = esrt->fw_resource_count - base_entry_count;
> +	if (entry_delta != TEST_ESRT_NUM_ENTRIES) {
> +		efi_st_error("ESRT mismatch in new entry count (%d), expected (%d).\n",
> +			     entry_delta, 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])) {
> +			efi_st_error("ESRT entry mistmatch\n");
> +			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,
> +};
>