[RFC PATCH V2 2/2] efi_selftest: unit test for RISCV_EFI_BOOT_PROTOCOL

[Heinrich Schuchardt]

On 1/28/22 16:18, Sunil V L wrote:
> Add a test for the RISCV_EFI_BOOT_PROTOCOL.
>
> Signed-off-by: Sunil V L <sunilvl at ventanamicro.com>
> ---
>   lib/efi_selftest/Makefile             |   1 +
>   lib/efi_selftest/efi_selftest_riscv.c | 119 ++++++++++++++++++++++++++
>   2 files changed, 120 insertions(+)
>   create mode 100644 lib/efi_selftest/efi_selftest_riscv.c
>
> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> index 9ff6e1760c..0b05f8a591 100644
> --- a/lib/efi_selftest/Makefile
> +++ b/lib/efi_selftest/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_EFI_LOADER_HII) += efi_selftest_hii.o
>   obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_selftest_rng.o
>   obj-$(CONFIG_EFI_GET_TIME) += efi_selftest_rtc.o
>   obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_selftest_tcg2.o
> +obj-$(CONFIG_EFI_RISCV_BOOT_PROTOCOL) += efi_selftest_riscv.o
>
>   ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
>   obj-y += efi_selftest_fdt.o
> diff --git a/lib/efi_selftest/efi_selftest_riscv.c b/lib/efi_selftest/efi_selftest_riscv.c
> new file mode 100644
> index 0000000000..9d2182e61f
> --- /dev/null
> +++ b/lib/efi_selftest/efi_selftest_riscv.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * efi_selftest_riscv
> + *
> + * Copyright (c) 2022 Ventana Micro Systems Inc
> + *
> + * Test the RISCV_EFI_BOOT_PROTOCOL.
> + *
> + * The following services are tested:
> + * get_boot_hartid
> + */
> +
> +#include <efi_selftest.h>
> +#include <efi_riscv.h>
> +#include <linux/libfdt.h>
> +
> +static const struct efi_system_table *systemtab;
> +static const struct efi_boot_services *boottime;
> +static const char *fdt;
> +static const efi_guid_t riscv_efi_boot_protocol_guid = RISCV_EFI_BOOT_PROTOCOL_GUID;
> +static const efi_guid_t fdt_guid = EFI_FDT_GUID;
> +
> +/**
> + * efi_st_get_config_table() - get configuration table
> + *
> + * @guid:	GUID of the configuration table
> + * Return:	pointer to configuration table or NULL
> + */
> +static void *efi_st_get_config_table(const efi_guid_t *guid)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < systab.nr_tables; i++) {
> +		if (!guidcmp(guid, &systemtab->tables[i].guid))
> +			return systemtab->tables[i].table;
> +	}
> +	return NULL;
> +}
> +
> +/*
> + * Setup unit test.
> + *
> + * @handle:	handle of the loaded image
> + * @systable:	system table
> + * @return:	EFI_ST_SUCCESS for success
> + */
> +static int setup(const efi_handle_t img_handle,
> +		 const struct efi_system_table *systable)
> +{
> +	systemtab = systable;
> +	boottime = systable->boottime;
> +
> +	fdt = efi_st_get_config_table(&fdt_guid);
> +
> +	if (!fdt) {
> +		efi_st_error("Missing device tree\n");
> +		return EFI_ST_FAILURE;
> +	}
> +	return EFI_ST_SUCCESS;
> +}
> +
> +/*
> + * Execute unit test.
> + *
> + * @return:	EFI_ST_SUCCESS for success
> + */
> +static int execute(void)
> +{
> +	efi_status_t ret;
> +	struct riscv_efi_boot_protocol *prot;
> +	efi_uintn_t efi_hartid, fdt_hartid;
> +	int chosen_node, len;
> +	const fdt32_t *prop;
> +
> +	/* Get riscv boot protocol */
> +	ret = boottime->locate_protocol(&riscv_efi_boot_protocol_guid, NULL,
> +					(void **)&prot);
> +	if (ret != EFI_SUCCESS) {
> +		efi_st_error("RISC-V Boot Protocol not available\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	/* Get Boot Hart ID from EFI protocol */
> +	ret = prot->get_boot_hartid(prot, &efi_hartid);
> +	if (ret != EFI_SUCCESS) {
> +		efi_st_error("Could not retrieve boot hart ID\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	/* Get Boot Hart ID from FDT */
> +	chosen_node = fdt_path_offset(fdt, "/chosen");
> +	if (chosen_node < 0) {
> +		efi_st_error("/chosen node not found\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len);

Thanks a lot for providing a test.

In the UEFI selftests we are not having access to gd. We should not call
into U-Boot except using the UEFI API. Otherwise any printf() (e.g.
debug output) in the called function will cause a crash.

I will add a patch to move get_property() of efi_selftest_fdt.c to an
exported function and tweak your patch accordingly.

> +	if (!prop || len != sizeof(u32)) {
> +		efi_st_error("boot-hartid not found\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	fdt_hartid = fdt32_to_cpu(*prop);

We should do the same for f2h() in efi_selftest_fdt.c.

Otherwise looks good to me.

Best regards

Heinrich

> +
> +	/* Boot Hart ID should be same */
> +	if (efi_hartid != fdt_hartid) {
> +		efi_st_error("boot-hartid is not same in EFI and FDT\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	return EFI_ST_SUCCESS;
> +}
> +
> +EFI_UNIT_TEST(riscv) = {
> +	.name = "riscv",
> +	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
> +	.setup = setup,
> +	.execute = execute,
> +};