[PATCH v2 1/1] efi_loader: architecture specific UEFI setup

[Heinrich Schuchardt]

On 2/5/20 8:43 AM, Ard Biesheuvel wrote:
> On Wed, 5 Feb 2020 at 05:53, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> RISC-V booting currently is based on a per boot stage lottery to determine
>> the active CPU. The Hart State Management (HSM) SBI extension replaces
>> this lottery by using a dedicated primary CPU.
>>
>> Cf. Hart State Management Extension, Extension ID: 0x48534D (HSM)
>> https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
>>
>> In this scenario the id of the active hart has to be passed from boot stage
>> to boot stage. Using a UEFI variable would provide an easy implementation.
>>
>> This patch provides a weak function that is called at the end of the setup
>> of U-Boot's UEFI sub-system. By overriding this function architecture
>> specific UEFI variables or configuration tables can be created.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> Reviewed-by: Atish Patra <atish.patra at wdc.com>
>
> OK, so I have a couple of questions:
>
> - does RISC-V use device tree? if so, why are you not passing the

In the Linux kernel tree you can find the SiFive HiFive Unleashed device
tree: arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts

Some of the QEMU emulated RISC-V boards provide device trees, cf.
https://github.com/riscv/riscv-qemu/wiki#machines

> active hart via a property in the /chosen node? I'd assume the EFI

There is a hart (core) that calls the entry point of the next
boot-stage. Could this define the active hart?

Best regards

Heinrich

> stub would not care at all about this information, and it would give
> you a Linux/RISC-V specific way to convey this information that is
> independent of EFI.
> - using variables to pass information from firmware to OS only is
> overkill, and config tables are preferred, given that they only
> require access to the system table. If required, a RISC-V specific
> data structure containing boot parameters could be installed as a
> configuration table, and the address passed to the startup code in the
> kernel proper [rather than just a hart id], allowing you to put any
> piece of information you like in there.
>
> Config tables work fine with kexec, btw. It is up to the first OS to
> memblock_reserve() the table to guarantee that it is still there at
> kexec time, but this applies equally to all other data structures
> passed as config tables. Alternatively, in this case, you can
> stipulate that it is passed as AcpiReclaim [ignore the 'Acpi' in the
> name] which is intended for firmware tables (and we never reclaim it
> in linux)
>
> I'd also recommend that RISC-V adopt the same principle as ARM does
> when it comes to EFI: call SetVirtualAddressMap in the stub, so that
> the kernel proper always sees the same handover state, regardless of
> kexec. Additionally, you shouldn't ever modify the EFI memory map
> provided by the firmware, so that the kexec kernel sees the exact same
> version.
>
>
>
>
>> ---
>> v2:
>>          reference the Hart State Management Extension in the commit message
>> ---
>>   include/efi_loader.h       |  3 +++
>>   lib/efi_loader/efi_setup.c | 16 ++++++++++++++++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index d4c59b54c4..d87de85e83 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -116,6 +116,9 @@ extern efi_uintn_t efi_memory_map_key;
>>   extern struct efi_runtime_services efi_runtime_services;
>>   extern struct efi_system_table systab;
>>
>> +/* Architecture specific initialization of the UEFI system */
>> +efi_status_t efi_setup_arch_specific(void);
>> +
>>   extern struct efi_simple_text_output_protocol efi_con_out;
>>   extern struct efi_simple_text_input_protocol efi_con_in;
>>   extern struct efi_console_control_protocol efi_console_control;
>> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>> index de7b616c6d..8469f0f43c 100644
>> --- a/lib/efi_loader/efi_setup.c
>> +++ b/lib/efi_loader/efi_setup.c
>> @@ -22,6 +22,17 @@ void __weak allow_unaligned(void)
>>   {
>>   }
>>
>> +/**
>> + * efi_setup_arch_specific() - architecture specific UEFI setup
>> + *
>> + * This routine can be used to define architecture specific variables
>> + * or configuration tables, e.g. HART id for RISC-V
>> + */
>> +efi_status_t __weak efi_setup_arch_specific(void)
>> +{
>> +       return EFI_SUCCESS;
>> +}
>> +
>>   /**
>>    * efi_init_platform_lang() - define supported languages
>>    *
>> @@ -179,6 +190,11 @@ efi_status_t efi_init_obj_list(void)
>>          if (ret != EFI_SUCCESS)
>>                  goto out;
>>
>> +       /* Architecture specific setup */
>> +       ret = efi_setup_arch_specific();
>> +       if (ret != EFI_SUCCESS)
>> +               goto out;
>> +
>>   out:
>>          efi_obj_list_initialized = ret;
>>          return ret;
>> --
>> 2.24.1
>>