On Wed, Feb 05, 2020 at 12:37:03PM +0100, Heinrich Schuchardt wrote: > Hello Daniel, hello Leif, > > what is the GRUB view on this discussion?
Alex, could you chime in on this as a GRUB RISC-V maintainer? Daniel > Best regards > > Heinrich > > On 2/5/20 12:32 PM, Heinrich Schuchardt wrote: > > On 2/5/20 8:43 AM, Ard Biesheuvel wrote: > > > On Wed, 5 Feb 2020 at 05:53, Heinrich Schuchardt <xypron.g...@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.g...@gmx.de> > > > > Reviewed-by: Atish Patra <atish.pa...@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