Re: [RFC PATCH 1/1] efi_loader: Enable RISCV_EFI_BOOT_PROTOCOL support
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu Jan 27 07:35:02 CET 2022
Am 26. Januar 2022 15:01:56 MEZ schrieb Jessica Clarke <jrtc27 at jrtc27.com>:
>On 26 Jan 2022, at 11:06, Sunil V L <sunilvl at ventanamicro.com> wrote:
>>
>> This adds support for new RISCV_EFI_BOOT_PROTOCOL to
>> communicate the boot hart ID to bootloader/kernel on RISC-V
>> UEFI platforms.
>>
>> Signed-off-by: Sunil V L <sunilvl at ventanamicro.com>
>> ---
>> include/efi_api.h | 4 +++
>> include/efi_loader.h | 2 ++
>> include/efi_riscv.h | 16 +++++++++
>> lib/efi_loader/Kconfig | 7 ++++
>> lib/efi_loader/Makefile | 1 +
>> lib/efi_loader/efi_riscv.c | 69 ++++++++++++++++++++++++++++++++++++++
>> lib/efi_loader/efi_setup.c | 6 ++++
>> 7 files changed, 105 insertions(+)
>> create mode 100644 include/efi_riscv.h
>> create mode 100644 lib/efi_loader/efi_riscv.c
>>
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index 8d5d835bd0..f123d0557c 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -438,6 +438,10 @@ struct efi_runtime_services {
>> EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, \
>> 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
>>
>> +#define RISCV_EFI_BOOT_PROTOCOL_GUID \
>> + EFI_GUID(0xccd15fec, 0x6f73, 0x4eec, 0x83, \
>> + 0x95, 0x3e, 0x69, 0xe4, 0xb9, 0x40, 0xbf)
>> +
>> /**
>> * struct efi_configuration_table - EFI Configuration Table
>> *
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 701efcd2b6..1fa75b40fe 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -527,6 +527,8 @@ efi_status_t efi_disk_register(void);
>> efi_status_t efi_rng_register(void);
>> /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
>> efi_status_t efi_tcg2_register(void);
>> +/* Called by efi_init_obj_list() to install RISCV_EFI_BOOT_PROTOCOL */
>> +efi_status_t efi_riscv_register(void);
>> /* Called by efi_init_obj_list() to do initial measurement */
>> efi_status_t efi_tcg2_do_initial_measurement(void);
>> /* measure the pe-coff image, extend PCR and add Event Log */
>> diff --git a/include/efi_riscv.h b/include/efi_riscv.h
>> new file mode 100644
>> index 0000000000..6beb2637f6
>> --- /dev/null
>> +++ b/include/efi_riscv.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * RISCV_EFI_BOOT_PROTOCOL
>> + *
>> + * Copyright (c) 2022 Ventana Micro Systems Inc
>> + */
>> +
>> +#include <efi_api.h>
>> +
>> +#define RISCV_EFI_BOOT_PROTOCOL_REVISION 0x00010000
>> +
>> +struct riscv_efi_boot_protocol {
>> + u64 revision;
>> + efi_status_t (EFIAPI *get_boot_hartid) (struct riscv_efi_boot_protocol *this,
>> + efi_uintn_t *boot_hartid);
>> +};
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index 24f9a2bb75..77ba6a7ea1 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -369,4 +369,11 @@ config EFI_ESRT
>> help
>> Enabling this option creates the ESRT UEFI system table.
>>
>> +config EFI_RISCV_BOOT_PROTOCOL
>> + bool "RISCV_EFI_BOOT_PROTOCOL support"
>> + default y
>
>Why would you ever turn it off for a RISC-V EFI system?
We don't need the protocol on ARM. So we want a configuration symbol for it.
The feature is experimental as there is no ratified specification requiring it. So it is ok to be able to disable it. Anyway it defaults to yes on RISC-V.
We still put the boot-hartid into the device-tree as elder kernels need it and future kernels may support this due to the legacy entry point. To minimize the code size on constrained devices it may be necessary to disable the protocol.
>
>> + depends on RISCV
>> + help
>> + Provide a RISCV_EFI_BOOT_PROTOCOL implementation on RISC-V platform.
>> +
>> endif
>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>> index fd344cea29..b2c664d108 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -62,6 +62,7 @@ obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
>> obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>> obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
>> obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o
>> +obj-$(CONFIG_EFI_RISCV_BOOT_PROTOCOL) += efi_riscv.o
>> obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
>> obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o
>>
>> diff --git a/lib/efi_loader/efi_riscv.c b/lib/efi_loader/efi_riscv.c
>> new file mode 100644
>> index 0000000000..91b8d2b927
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_riscv.c
>> @@ -0,0 +1,69 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Defines APIs that allow an OS to interact with UEFI firmware to query
>> + * information about the boot hart ID.
>> + *
>> + * Copyright (c) 2022, Ventana Micro Systems Inc
>> + */
>> +
>> +#define LOG_CATEGORY LOGC_EFI
>> +#include <common.h>
>> +#include <efi_loader.h>
>> +#include <efi_variable.h>
>> +#include <log.h>
>> +#include <asm/global_data.h>
>> +#include <efi_riscv.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static const efi_guid_t efi_guid_riscv_boot_protocol = RISCV_EFI_BOOT_PROTOCOL_GUID;
>> +static efi_uintn_t hartid;
>> +
>> +/**
>> + * efi_riscv_get_boot_hartid() - return boot hart ID
>> + *
>> + * @this: RISCV_EFI_BOOT_PROTOCOL instance
>> + * @boot_hartid caller allocated memory to return boot hart id
>> +
>> + * Return: status code
>> + */
>> +static efi_status_t EFIAPI
>> +efi_riscv_get_boot_hartid(struct riscv_efi_boot_protocol *this,
>> + efi_uintn_t *boot_hartid)
>> +{
>> + log_err(" efi_riscv_get_boot_hartid ENTER\n");
>> + if ((this == NULL) || (boot_hartid == NULL))
>> + return EFI_INVALID_PARAMETER;
>> +
>> + *boot_hartid = hartid;
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> +static const struct riscv_efi_boot_protocol riscv_efi_boot_prot = {
>> + .revision = RISCV_EFI_BOOT_PROTOCOL_REVISION,
>> + .get_boot_hartid = efi_riscv_get_boot_hartid
>> +};
>> +
>> +/**
>> + * efi_riscv_register() - register RISCV_EFI_BOOT_PROTOCOL
>> + *
>> + *
>> + * Return: status code
>> + */
>> +efi_status_t efi_riscv_register(void)
>> +{
>> + efi_status_t ret = EFI_SUCCESS;
>> +
>> + /* save boot hart id since gd is not accessible after launching
>> + * the kernel
>> + */
>> + hartid = gd->arch.boot_hart;
>> +
>> + ret = efi_add_protocol(efi_root, &efi_guid_riscv_boot_protocol,
>> + (void *)&riscv_efi_boot_prot);
>> + if (ret != EFI_SUCCESS) {
>> + log_err("Cannot install RISCV_EFI_BOOT_PROTOCOL\n");
>> + }
>> + return ret;
>> +}
>> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>> index 49172e3579..380adc15c8 100644
>> --- a/lib/efi_loader/efi_setup.c
>> +++ b/lib/efi_loader/efi_setup.c
>> @@ -247,6 +247,12 @@ efi_status_t efi_init_obj_list(void)
>> goto out;
>> }
>>
>> + if (IS_ENABLED(CONFIG_EFI_RISCV_BOOT_PROTOCOL)) {
>> + ret = efi_riscv_register();
>> + if (ret != EFI_SUCCESS)
>> + goto out;
>> + }
>> +
>
>I don’t think this will link if EFI_RISCV_BOOT_PROTOCOL is disabled. At
>least not at -O0, at -O1 and above it’ll probably optimise away the
>dead code and thus not reference the undefined symbol.
U-Boot is built with -O2 (or -Os). The project wants to avoid #ifdef. See the warning in scripts/checkpatch.pl. With this coding style we catch more coding issues in our CI without increasing the binary size.
Best regards
Heinrich
>
>Jess
>
>> /* Secure boot */
>> ret = efi_init_secure_boot();
>> if (ret != EFI_SUCCESS)
>> --
>> 2.25.1
>>
>
More information about the U-Boot
mailing list