| Message ID | 20200205055334.4072-1-xypron.glpk@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/1] efi_loader: architecture specific UEFI setup | expand |
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 active hart via a property in the /chosen node? I'd assume the EFI 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 >
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 >>
Hello Daniel, hello Leif, what is the GRUB view on this discussion? 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.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 >>> >
On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel <ard.biesheuvel at linaro.org> 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 > active hart via a property in the /chosen node? Yes. RISC-V uses device tree. Technically, we can pass the active hart by a DT property but that means we have to modify the DT in OpenSBI (RISC-V specific run time service provider). We have been trying to avoid that if possible so that DT is not bounced around. This also limits U-Boot to have its own device tree. > I'd assume the EFI 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. Yes. EFI stub doesn't care about this information. However, it needs to save the information somewhere so that it can pass to the real kernel after exiting boot time services. > - 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. > Sounds good to me. I will experiment with configuration table and send the EFI stub patch series. > 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. > Sure. There is no kexec implementation available now. We will keep this in mind while implementing it. Thanks! > > > > > --- > > 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 > >
On 06.02.20 19:28, Atish Patra wrote: > On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > <ard.biesheuvel at linaro.org> 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 >> active hart via a property in the /chosen node? > Yes. RISC-V uses device tree. Technically, we can pass the active hart > by a DT property > but that means we have to modify the DT in OpenSBI (RISC-V specific > run time service provider). > We have been trying to avoid that if possible so that DT is not > bounced around. This also limits > U-Boot to have its own device tree. I don't understand how this is different from the UEFI variable scheme proposed here? If you want to create an SBI interface to propagate the active HART that U-Boot then uses to populate the /chosen property, that's probably fine as well. We already have hooks that allow you to modify the DT right before it gets delivered to the payload. Just add it there? > > >> I'd assume the EFI 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. > Yes. EFI stub doesn't care about this information. However, it needs > to save the information somewhere > so that it can pass to the real kernel after exiting boot time services. DT sounds like a pretty natural choice for that to me :). Alex
On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> wrote: > > > On 06.02.20 19:28, Atish Patra wrote: > > On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > <ard.biesheuvel at linaro.org> 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 > >> active hart via a property in the /chosen node? > > Yes. RISC-V uses device tree. Technically, we can pass the active hart > > by a DT property > > but that means we have to modify the DT in OpenSBI (RISC-V specific > > run time service provider). > > We have been trying to avoid that if possible so that DT is not > > bounced around. This also limits > > U-Boot to have its own device tree. > > > I don't understand how this is different from the UEFI variable scheme > proposed here? If you want to create an SBI interface to propagate the > active HART that U-Boot then uses to populate the /chosen property, > that's probably fine as well. > We don't want to create SBI interface to pass this information. > We already have hooks that allow you to modify the DT right before it > gets delivered to the payload. Just add it there? > Hmm. I guess it is true if the DT is loaded from MMC or network as well. How about EDK2 ? If we go DT route, it also has to modify the DT to pass the boot hart. As it requires DT modification in multiple projects, why not use efi configuration tables as suggested by Ard ? > > > > > >> I'd assume the EFI 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. > > Yes. EFI stub doesn't care about this information. However, it needs > > to save the information somewhere > > so that it can pass to the real kernel after exiting boot time services. > > > DT sounds like a pretty natural choice for that to me :). > > > Alex > >
> Am 06.02.2020 um 22:06 schrieb Atish Patra <atishp at atishpatra.org>: > > On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> wrote: >> >> >>> On 06.02.20 19:28, Atish Patra wrote: >>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel >>> <ard.biesheuvel at linaro.org> 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 >>>> active hart via a property in the /chosen node? >>> Yes. RISC-V uses device tree. Technically, we can pass the active hart >>> by a DT property >>> but that means we have to modify the DT in OpenSBI (RISC-V specific >>> run time service provider). >>> We have been trying to avoid that if possible so that DT is not >>> bounced around. This also limits >>> U-Boot to have its own device tree. >> >> >> I don't understand how this is different from the UEFI variable scheme >> proposed here? If you want to create an SBI interface to propagate the >> active HART that U-Boot then uses to populate the /chosen property, >> that's probably fine as well. >> > > We don't want to create SBI interface to pass this information. > >> We already have hooks that allow you to modify the DT right before it >> gets delivered to the payload. Just add it there? >> > > Hmm. I guess it is true if the DT is loaded from MMC or network as well. > How about EDK2 ? If we go DT route, it also has to modify the DT to > pass the boot hart. > > As it requires DT modification in multiple projects, why not use efi > configuration tables as > suggested by Ard ? How is assembling a configuration table better than modifying a DT? Alex > >>> >>> >>>> I'd assume the EFI 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. >>> Yes. EFI stub doesn't care about this information. However, it needs >>> to save the information somewhere >>> so that it can pass to the real kernel after exiting boot time services. >> >> >> DT sounds like a pretty natural choice for that to me :). >> >> >> Alex >> >> > > > -- > Regards, > Atish
On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: > > On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> wrote: > > > > > > On 06.02.20 19:28, Atish Patra wrote: > > > On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > > <ard.biesheuvel at linaro.org> 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 > > >> active hart via a property in the /chosen node? > > > Yes. RISC-V uses device tree. Technically, we can pass the active hart > > > by a DT property > > > but that means we have to modify the DT in OpenSBI (RISC-V specific > > > run time service provider). > > > We have been trying to avoid that if possible so that DT is not > > > bounced around. This also limits > > > U-Boot to have its own device tree. > > > > > > I don't understand how this is different from the UEFI variable scheme > > proposed here? If you want to create an SBI interface to propagate the > > active HART that U-Boot then uses to populate the /chosen property, > > that's probably fine as well. > > > > We don't want to create SBI interface to pass this information. > > > We already have hooks that allow you to modify the DT right before it > > gets delivered to the payload. Just add it there? > > > > Hmm. I guess it is true if the DT is loaded from MMC or network as well. > How about EDK2 ? If we go DT route, it also has to modify the DT to > pass the boot hart. > > As it requires DT modification in multiple projects, why not use efi > configuration tables as > suggested by Ard ? > Configuration tables are preferred over variables, but putting it in the DT makes even more sense, since in that case, nothing that runs in the UEFI context has to care about any of this. > > > > > > > > >> I'd assume the EFI 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. > > > Yes. EFI stub doesn't care about this information. However, it needs > > > to save the information somewhere > > > so that it can pass to the real kernel after exiting boot time services. > > > > > > DT sounds like a pretty natural choice for that to me :). > > Indeed. DT has a /chosen node that is set aside for this purpose. It does depend on how early you need the value (i.e., before or after you can run C code), but since you are passing the DT address to the core kernel, it makes way more sense to drop any additional information that you need to pass in there.
On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel <ard.biesheuvel at linaro.org> wrote: > > On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: > > > > On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> wrote: > > > > > > > > > On 06.02.20 19:28, Atish Patra wrote: > > > > On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > > > <ard.biesheuvel at linaro.org> 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 > > > >> active hart via a property in the /chosen node? > > > > Yes. RISC-V uses device tree. Technically, we can pass the active hart > > > > by a DT property > > > > but that means we have to modify the DT in OpenSBI (RISC-V specific > > > > run time service provider). > > > > We have been trying to avoid that if possible so that DT is not > > > > bounced around. This also limits > > > > U-Boot to have its own device tree. > > > > > > > > > I don't understand how this is different from the UEFI variable scheme > > > proposed here? If you want to create an SBI interface to propagate the > > > active HART that U-Boot then uses to populate the /chosen property, > > > that's probably fine as well. > > > > > > > We don't want to create SBI interface to pass this information. > > > > > We already have hooks that allow you to modify the DT right before it > > > gets delivered to the payload. Just add it there? > > > > > > > Hmm. I guess it is true if the DT is loaded from MMC or network as well. > > How about EDK2 ? If we go DT route, it also has to modify the DT to > > pass the boot hart. > > > > As it requires DT modification in multiple projects, why not use efi > > configuration tables as > > suggested by Ard ? > > > > Configuration tables are preferred over variables, but putting it in > the DT makes even more sense, since in that case, nothing that runs in > the UEFI context has to care about any of this. > > > > > > > > > > > > >> I'd assume the EFI 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. > > > > Yes. EFI stub doesn't care about this information. However, it needs > > > > to save the information somewhere > > > > so that it can pass to the real kernel after exiting boot time services. > > > > > > > > > DT sounds like a pretty natural choice for that to me :). > > > > > Indeed. DT has a /chosen node that is set aside for this purpose. It > does depend on how early you need the value (i.e., before or after you > can run C code), but since you are passing the DT address to the core > kernel, it makes way more sense to drop any additional information > that you need to pass in there. We don't need boot hart id until real kernel boots and parse DT. So that should be okay. I just looked at the efi stub code once more and realized that it is already parsing the DT to setup uefi memory maps from /chosen node. Adding boot hart id to the chosen node does seem much cleaner to me :). Thanks for all the explanations. I have not looked at EDK2 code. But I am assuming modifying the DT just before jumping to the payload won't be too hard for EDK2 as well. Added Leif and Abner for the opinion. -- Regards, Atish
> -----Original Message----- > From: Atish Patra [mailto:atishp at atishpatra.org] > Sent: Friday, February 7, 2020 6:56 AM > To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, Abner (HPS SW/FW > Technologist) <abner.chang at hpe.com> > Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt > <xypron.glpk at gmx.de>; U-Boot Mailing List <u-boot at lists.denx.de>; Atish > Patra <atish.patra at wdc.com>; leif at nuviainc.com > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel <ard.biesheuvel at linaro.org> > wrote: > > > > On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: > > > > > > On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> wrote: > > > > > > > > > > > > On 06.02.20 19:28, Atish Patra wrote: > > > > > On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > > > > <ard.biesheuvel at linaro.org> 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.a > > > > >>> doc > > > > >>> > > > > >>> 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 active hart via a property in the /chosen node? > > > > > Yes. RISC-V uses device tree. Technically, we can pass the > > > > > active hart by a DT property but that means we have to modify > > > > > the DT in OpenSBI (RISC-V specific run time service provider). > > > > > We have been trying to avoid that if possible so that DT is not > > > > > bounced around. This also limits U-Boot to have its own device > > > > > tree. > > > > > > > > > > > > I don't understand how this is different from the UEFI variable > > > > scheme proposed here? If you want to create an SBI interface to > > > > propagate the active HART that U-Boot then uses to populate the > > > > /chosen property, that's probably fine as well. > > > > > > > > > > We don't want to create SBI interface to pass this information. > > > > > > > We already have hooks that allow you to modify the DT right before > > > > it gets delivered to the payload. Just add it there? > > > > > > > > > > Hmm. I guess it is true if the DT is loaded from MMC or network as well. > > > How about EDK2 ? If we go DT route, it also has to modify the DT to > > > pass the boot hart. > > > > > > As it requires DT modification in multiple projects, why not use efi > > > configuration tables as suggested by Ard ? > > > > > > > Configuration tables are preferred over variables, but putting it in > > the DT makes even more sense, since in that case, nothing that runs in > > the UEFI context has to care about any of this. > > > > > > > > > > > > > > > > >> I'd assume the EFI 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. > > > > > Yes. EFI stub doesn't care about this information. However, it > > > > > needs to save the information somewhere so that it can pass to > > > > > the real kernel after exiting boot time services. > > > > > > > > > > > > DT sounds like a pretty natural choice for that to me :). > > > > > > > > Indeed. DT has a /chosen node that is set aside for this purpose. It > > does depend on how early you need the value (i.e., before or after you > > can run C code), but since you are passing the DT address to the core > > kernel, it makes way more sense to drop any additional information > > that you need to pass in there. > > We don't need boot hart id until real kernel boots and parse DT. So that > should be okay. > I just looked at the efi stub code once more and realized that it is already > parsing the DT to setup uefi memory maps from /chosen node. Adding boot > hart id to the chosen node does seem much cleaner to me :). Thanks for all > the explanations. > > I have not looked at EDK2 code. But I am assuming modifying the DT just > before jumping to the payload won't be too hard for EDK2 as well. We don’t use DT in edk2 RISC-V port and we pass boot HART ID in SMBIOS type 44h as it is spec out in below link, https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBIOS.md Abner > > Added Leif and Abner for the opinion. > > > > -- > Regards, > Atish
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.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
On 2/7/20 4:13 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > > >> -----Original Message----- >> From: Atish Patra [mailto:atishp at atishpatra.org] >> Sent: Friday, February 7, 2020 6:56 AM >> To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, Abner (HPS SW/FW >> Technologist) <abner.chang at hpe.com> >> Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt >> <xypron.glpk at gmx.de>; U-Boot Mailing List <u-boot at lists.denx.de>; Atish >> Patra <atish.patra at wdc.com>; leif at nuviainc.com >> Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup >> >> On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel <ard.biesheuvel at linaro.org> >> wrote: >>> >>> On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: >>>> >>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> wrote: >>>>> >>>>> >>>>> On 06.02.20 19:28, Atish Patra wrote: >>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel >>>>>> <ard.biesheuvel at linaro.org> 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.a >>>>>>>> doc >>>>>>>> >>>>>>>> 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 active hart via a property in the /chosen node? >>>>>> Yes. RISC-V uses device tree. Technically, we can pass the >>>>>> active hart by a DT property but that means we have to modify >>>>>> the DT in OpenSBI (RISC-V specific run time service provider). >>>>>> We have been trying to avoid that if possible so that DT is not >>>>>> bounced around. This also limits U-Boot to have its own device >>>>>> tree. >>>>> >>>>> >>>>> I don't understand how this is different from the UEFI variable >>>>> scheme proposed here? If you want to create an SBI interface to >>>>> propagate the active HART that U-Boot then uses to populate the >>>>> /chosen property, that's probably fine as well. >>>>> >>>> >>>> We don't want to create SBI interface to pass this information. >>>> >>>>> We already have hooks that allow you to modify the DT right before >>>>> it gets delivered to the payload. Just add it there? >>>>> >>>> >>>> Hmm. I guess it is true if the DT is loaded from MMC or network as well. >>>> How about EDK2 ? If we go DT route, it also has to modify the DT to >>>> pass the boot hart. >>>> >>>> As it requires DT modification in multiple projects, why not use efi >>>> configuration tables as suggested by Ard ? >>>> >>> >>> Configuration tables are preferred over variables, but putting it in >>> the DT makes even more sense, since in that case, nothing that runs in >>> the UEFI context has to care about any of this. >>> >>>>>> >>>>>> >>>>>>> I'd assume the EFI 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. >>>>>> Yes. EFI stub doesn't care about this information. However, it >>>>>> needs to save the information somewhere so that it can pass to >>>>>> the real kernel after exiting boot time services. >>>>> >>>>> >>>>> DT sounds like a pretty natural choice for that to me :). >>>>> >>> >>> Indeed. DT has a /chosen node that is set aside for this purpose. It >>> does depend on how early you need the value (i.e., before or after you >>> can run C code), but since you are passing the DT address to the core >>> kernel, it makes way more sense to drop any additional information >>> that you need to pass in there. >> >> We don't need boot hart id until real kernel boots and parse DT. So that >> should be okay. >> I just looked at the efi stub code once more and realized that it is already >> parsing the DT to setup uefi memory maps from /chosen node. Adding boot >> hart id to the chosen node does seem much cleaner to me :). Thanks for all >> the explanations. >> >> I have not looked at EDK2 code. But I am assuming modifying the DT just >> before jumping to the payload won't be too hard for EDK2 as well. > We don’t use DT in edk2 RISC-V port and we pass boot HART ID in SMBIOS type 44h as it is spec out in below link, > https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBIOS.md Thanks for the link. For 'RISC-V SMBIOS Type 44 Processor Additional Information' I find entry 0x13h 1: This is boot hart to boot system . But is '44' a hexadecimal number? The document does not indicate this. Best regards Heinrich > > Abner > >> >> Added Leif and Abner for the opinion. >> >> >> >> -- >> Regards, >> Atish
> -----Original Message----- > From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de] > Sent: Wednesday, February 12, 2020 2:26 AM > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com>; > Atish Patra <atishp at atishpatra.org>; Ard Biesheuvel > <ard.biesheuvel at linaro.org> > Cc: Alexander Graf <agraf at csgraf.de>; U-Boot Mailing List <u- > boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > leif at nuviainc.com > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > On 2/7/20 4:13 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > > > >> -----Original Message----- > >> From: Atish Patra [mailto:atishp at atishpatra.org] > >> Sent: Friday, February 7, 2020 6:56 AM > >> To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, Abner (HPS > >> SW/FW > >> Technologist) <abner.chang at hpe.com> > >> Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt > >> <xypron.glpk at gmx.de>; U-Boot Mailing List <u-boot at lists.denx.de>; > >> Atish Patra <atish.patra at wdc.com>; leif at nuviainc.com > >> Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI > >> setup > >> > >> On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel > >> <ard.biesheuvel at linaro.org> > >> wrote: > >>> > >>> On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: > >>>> > >>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> > wrote: > >>>>> > >>>>> > >>>>> On 06.02.20 19:28, Atish Patra wrote: > >>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > >>>>>> <ard.biesheuvel at linaro.org> 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.a > >>>>>>>> doc > >>>>>>>> > >>>>>>>> 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 active hart via a property in the /chosen node? > >>>>>> Yes. RISC-V uses device tree. Technically, we can pass the active > >>>>>> hart by a DT property but that means we have to modify the DT in > >>>>>> OpenSBI (RISC-V specific run time service provider). > >>>>>> We have been trying to avoid that if possible so that DT is not > >>>>>> bounced around. This also limits U-Boot to have its own device > >>>>>> tree. > >>>>> > >>>>> > >>>>> I don't understand how this is different from the UEFI variable > >>>>> scheme proposed here? If you want to create an SBI interface to > >>>>> propagate the active HART that U-Boot then uses to populate the > >>>>> /chosen property, that's probably fine as well. > >>>>> > >>>> > >>>> We don't want to create SBI interface to pass this information. > >>>> > >>>>> We already have hooks that allow you to modify the DT right before > >>>>> it gets delivered to the payload. Just add it there? > >>>>> > >>>> > >>>> Hmm. I guess it is true if the DT is loaded from MMC or network as well. > >>>> How about EDK2 ? If we go DT route, it also has to modify the DT to > >>>> pass the boot hart. > >>>> > >>>> As it requires DT modification in multiple projects, why not use > >>>> efi configuration tables as suggested by Ard ? > >>>> > >>> > >>> Configuration tables are preferred over variables, but putting it in > >>> the DT makes even more sense, since in that case, nothing that runs > >>> in the UEFI context has to care about any of this. > >>> > >>>>>> > >>>>>> > >>>>>>> I'd assume the EFI 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. > >>>>>> Yes. EFI stub doesn't care about this information. However, it > >>>>>> needs to save the information somewhere so that it can pass to > >>>>>> the real kernel after exiting boot time services. > >>>>> > >>>>> > >>>>> DT sounds like a pretty natural choice for that to me :). > >>>>> > >>> > >>> Indeed. DT has a /chosen node that is set aside for this purpose. It > >>> does depend on how early you need the value (i.e., before or after > >>> you can run C code), but since you are passing the DT address to the > >>> core kernel, it makes way more sense to drop any additional > >>> information that you need to pass in there. > >> > >> We don't need boot hart id until real kernel boots and parse DT. So > >> that should be okay. > >> I just looked at the efi stub code once more and realized that it is > >> already parsing the DT to setup uefi memory maps from /chosen node. > >> Adding boot hart id to the chosen node does seem much cleaner to me > >> :). Thanks for all the explanations. > >> > >> I have not looked at EDK2 code. But I am assuming modifying the DT > >> just before jumping to the payload won't be too hard for EDK2 as well. > > We don?t use DT in edk2 RISC-V port and we pass boot HART ID in SMBIOS > > type 44h as it is spec out in below link, > > https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBIOS.md > > Thanks for the link. > > For 'RISC-V SMBIOS Type 44 Processor Additional Information' I find entry > 0x13h 1: This is boot hart to boot system . > > But is '44' a hexadecimal number? The document does not indicate this. Type '44' is decimal format as it mentioned in SMBIOS spec, I had typo in above which said '44h'. However, that's good to mention this in RISCV_SMBIOS.md. Thanks for the recommendation. > > Best regards > > Heinrich > > > > > Abner > > > >> > >> Added Leif and Abner for the opinion. > >> > >> > >> > >> -- > >> Regards, > >> Atish
On Wed, 12 Feb 2020 at 06:49, Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com> wrote: > > > > > -----Original Message----- > > From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de] > > Sent: Wednesday, February 12, 2020 2:26 AM > > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com>; > > Atish Patra <atishp at atishpatra.org>; Ard Biesheuvel > > <ard.biesheuvel at linaro.org> > > Cc: Alexander Graf <agraf at csgraf.de>; U-Boot Mailing List <u- > > boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > > leif at nuviainc.com > > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > > > On 2/7/20 4:13 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > > > > > > >> -----Original Message----- > > >> From: Atish Patra [mailto:atishp at atishpatra.org] > > >> Sent: Friday, February 7, 2020 6:56 AM > > >> To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, Abner (HPS > > >> SW/FW > > >> Technologist) <abner.chang at hpe.com> > > >> Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt > > >> <xypron.glpk at gmx.de>; U-Boot Mailing List <u-boot at lists.denx.de>; > > >> Atish Patra <atish.patra at wdc.com>; leif at nuviainc.com > > >> Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI > > >> setup > > >> > > >> On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel > > >> <ard.biesheuvel at linaro.org> > > >> wrote: > > >>> > > >>> On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: > > >>>> > > >>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> > > wrote: > > >>>>> > > >>>>> > > >>>>> On 06.02.20 19:28, Atish Patra wrote: > > >>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > >>>>>> <ard.biesheuvel at linaro.org> 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.a > > >>>>>>>> doc > > >>>>>>>> > > >>>>>>>> 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 active hart via a property in the /chosen node? > > >>>>>> Yes. RISC-V uses device tree. Technically, we can pass the active > > >>>>>> hart by a DT property but that means we have to modify the DT in > > >>>>>> OpenSBI (RISC-V specific run time service provider). > > >>>>>> We have been trying to avoid that if possible so that DT is not > > >>>>>> bounced around. This also limits U-Boot to have its own device > > >>>>>> tree. > > >>>>> > > >>>>> > > >>>>> I don't understand how this is different from the UEFI variable > > >>>>> scheme proposed here? If you want to create an SBI interface to > > >>>>> propagate the active HART that U-Boot then uses to populate the > > >>>>> /chosen property, that's probably fine as well. > > >>>>> > > >>>> > > >>>> We don't want to create SBI interface to pass this information. > > >>>> > > >>>>> We already have hooks that allow you to modify the DT right before > > >>>>> it gets delivered to the payload. Just add it there? > > >>>>> > > >>>> > > >>>> Hmm. I guess it is true if the DT is loaded from MMC or network as well. > > >>>> How about EDK2 ? If we go DT route, it also has to modify the DT to > > >>>> pass the boot hart. > > >>>> > > >>>> As it requires DT modification in multiple projects, why not use > > >>>> efi configuration tables as suggested by Ard ? > > >>>> > > >>> > > >>> Configuration tables are preferred over variables, but putting it in > > >>> the DT makes even more sense, since in that case, nothing that runs > > >>> in the UEFI context has to care about any of this. > > >>> > > >>>>>> > > >>>>>> > > >>>>>>> I'd assume the EFI 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. > > >>>>>> Yes. EFI stub doesn't care about this information. However, it > > >>>>>> needs to save the information somewhere so that it can pass to > > >>>>>> the real kernel after exiting boot time services. > > >>>>> > > >>>>> > > >>>>> DT sounds like a pretty natural choice for that to me :). > > >>>>> > > >>> > > >>> Indeed. DT has a /chosen node that is set aside for this purpose. It > > >>> does depend on how early you need the value (i.e., before or after > > >>> you can run C code), but since you are passing the DT address to the > > >>> core kernel, it makes way more sense to drop any additional > > >>> information that you need to pass in there. > > >> > > >> We don't need boot hart id until real kernel boots and parse DT. So > > >> that should be okay. > > >> I just looked at the efi stub code once more and realized that it is > > >> already parsing the DT to setup uefi memory maps from /chosen node. > > >> Adding boot hart id to the chosen node does seem much cleaner to me > > >> :). Thanks for all the explanations. > > >> > > >> I have not looked at EDK2 code. But I am assuming modifying the DT > > >> just before jumping to the payload won't be too hard for EDK2 as well. > > > We don’t use DT in edk2 RISC-V port and we pass boot HART ID in SMBIOS > > > type 44h as it is spec out in below link, > > > https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBIOS.md > > > > Thanks for the link. > > > > For 'RISC-V SMBIOS Type 44 Processor Additional Information' I find entry > > 0x13h 1: This is boot hart to boot system . > > > > But is '44' a hexadecimal number? The document does not indicate this. > Type '44' is decimal format as it mentioned in SMBIOS spec, I had typo in above which said '44h'. However, that's good to mention this in RISCV_SMBIOS.md. Thanks for the recommendation. > > SMBIOS data is intended to describe the hardware to system administrators, not to the OS loader, and I don't think it makes sense to rely on it for booting. I'd assume that SMBIOS tables are not mandatory to begin with. For EFI boot, it is acceptable if the stub loader in Linux itself needs to obtain the value from something like a device tree and pass it in a CPU register at handover time, although I would still prefer it if the kernel simply gets it from the device tree directly if one is guaranteed to be available. Adding a new ABI between the firmware and the stub loader in Linux to use EFI specific conduits like config tables or EFI variables should really be avoided, though, as it affects every EFI loader while the code that runs in the EFI context doesn't even care (note that beyond u-boot and GRUB, there are other EFI loaders such as systemd-boot that need to be taken into account).
On Tue, Feb 11, 2020 at 11:28 PM Ard Biesheuvel <ard.biesheuvel at linaro.org> wrote: > > On Wed, 12 Feb 2020 at 06:49, Chang, Abner (HPS SW/FW Technologist) > <abner.chang at hpe.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de] > > > Sent: Wednesday, February 12, 2020 2:26 AM > > > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com>; > > > Atish Patra <atishp at atishpatra.org>; Ard Biesheuvel > > > <ard.biesheuvel at linaro.org> > > > Cc: Alexander Graf <agraf at csgraf.de>; U-Boot Mailing List <u- > > > boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > > > leif at nuviainc.com > > > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > > > > > On 2/7/20 4:13 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > > > > > > > > > >> -----Original Message----- > > > >> From: Atish Patra [mailto:atishp at atishpatra.org] > > > >> Sent: Friday, February 7, 2020 6:56 AM > > > >> To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, Abner (HPS > > > >> SW/FW > > > >> Technologist) <abner.chang at hpe.com> > > > >> Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt > > > >> <xypron.glpk at gmx.de>; U-Boot Mailing List <u-boot at lists.denx.de>; > > > >> Atish Patra <atish.patra at wdc.com>; leif at nuviainc.com > > > >> Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI > > > >> setup > > > >> > > > >> On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel > > > >> <ard.biesheuvel at linaro.org> > > > >> wrote: > > > >>> > > > >>> On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: > > > >>>> > > > >>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> > > > wrote: > > > >>>>> > > > >>>>> > > > >>>>> On 06.02.20 19:28, Atish Patra wrote: > > > >>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > > >>>>>> <ard.biesheuvel at linaro.org> 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.a > > > >>>>>>>> doc > > > >>>>>>>> > > > >>>>>>>> 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 active hart via a property in the /chosen node? > > > >>>>>> Yes. RISC-V uses device tree. Technically, we can pass the active > > > >>>>>> hart by a DT property but that means we have to modify the DT in > > > >>>>>> OpenSBI (RISC-V specific run time service provider). > > > >>>>>> We have been trying to avoid that if possible so that DT is not > > > >>>>>> bounced around. This also limits U-Boot to have its own device > > > >>>>>> tree. > > > >>>>> > > > >>>>> > > > >>>>> I don't understand how this is different from the UEFI variable > > > >>>>> scheme proposed here? If you want to create an SBI interface to > > > >>>>> propagate the active HART that U-Boot then uses to populate the > > > >>>>> /chosen property, that's probably fine as well. > > > >>>>> > > > >>>> > > > >>>> We don't want to create SBI interface to pass this information. > > > >>>> > > > >>>>> We already have hooks that allow you to modify the DT right before > > > >>>>> it gets delivered to the payload. Just add it there? > > > >>>>> > > > >>>> > > > >>>> Hmm. I guess it is true if the DT is loaded from MMC or network as well. > > > >>>> How about EDK2 ? If we go DT route, it also has to modify the DT to > > > >>>> pass the boot hart. > > > >>>> > > > >>>> As it requires DT modification in multiple projects, why not use > > > >>>> efi configuration tables as suggested by Ard ? > > > >>>> > > > >>> > > > >>> Configuration tables are preferred over variables, but putting it in > > > >>> the DT makes even more sense, since in that case, nothing that runs > > > >>> in the UEFI context has to care about any of this. > > > >>> > > > >>>>>> > > > >>>>>> > > > >>>>>>> I'd assume the EFI 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. > > > >>>>>> Yes. EFI stub doesn't care about this information. However, it > > > >>>>>> needs to save the information somewhere so that it can pass to > > > >>>>>> the real kernel after exiting boot time services. > > > >>>>> > > > >>>>> > > > >>>>> DT sounds like a pretty natural choice for that to me :). > > > >>>>> > > > >>> > > > >>> Indeed. DT has a /chosen node that is set aside for this purpose. It > > > >>> does depend on how early you need the value (i.e., before or after > > > >>> you can run C code), but since you are passing the DT address to the > > > >>> core kernel, it makes way more sense to drop any additional > > > >>> information that you need to pass in there. > > > >> > > > >> We don't need boot hart id until real kernel boots and parse DT. So > > > >> that should be okay. > > > >> I just looked at the efi stub code once more and realized that it is > > > >> already parsing the DT to setup uefi memory maps from /chosen node. > > > >> Adding boot hart id to the chosen node does seem much cleaner to me > > > >> :). Thanks for all the explanations. > > > >> > > > >> I have not looked at EDK2 code. But I am assuming modifying the DT > > > >> just before jumping to the payload won't be too hard for EDK2 as well. > > > > We don?t use DT in edk2 RISC-V port and we pass boot HART ID in SMBIOS > > > > type 44h as it is spec out in below link, > > > > https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBIOS.md > > > > > > Thanks for the link. > > > > > > For 'RISC-V SMBIOS Type 44 Processor Additional Information' I find entry > > > 0x13h 1: This is boot hart to boot system . > > > > > > But is '44' a hexadecimal number? The document does not indicate this. > > Type '44' is decimal format as it mentioned in SMBIOS spec, I had typo in above which said '44h'. However, that's good to mention this in RISCV_SMBIOS.md. Thanks for the recommendation. > > > > > SMBIOS data is intended to describe the hardware to system > administrators, not to the OS loader, and I don't think it makes sense > to rely on it for booting. I'd assume that SMBIOS tables are not > mandatory to begin with. > > For EFI boot, it is acceptable if the stub loader in Linux itself > needs to obtain the value from something like a device tree and pass > it in a CPU register at handover time, That's what I am planning to do for now. We can add SMBIOS parsing as well if required in future. although I would still prefer > it if the kernel simply gets it from the device tree directly if one > is guaranteed to be available. > That would break current booting protocol in RISC-V where register "a0" should contain the booting hartid. If we have to move away for that method, changes need to be in multiple places (to modify the DT) and it has to be done in a backward compatible way. > Adding a new ABI between the firmware and the stub loader in Linux to > use EFI specific conduits like config tables or EFI variables should > really be avoided, though, as it affects every EFI loader while the > code that runs in the EFI context doesn't even care (note that beyond > u-boot and GRUB, there are other EFI loaders such as systemd-boot that > need to be taken into account). Which booting stage should be responsible for changing the DT for those EFI loaders ?
On Thu, 13 Feb 2020 at 19:59, Atish Patra <atishp at atishpatra.org> wrote: > > On Tue, Feb 11, 2020 at 11:28 PM Ard Biesheuvel > <ard.biesheuvel at linaro.org> wrote: > > > > On Wed, 12 Feb 2020 at 06:49, Chang, Abner (HPS SW/FW Technologist) > > <abner.chang at hpe.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de] > > > > Sent: Wednesday, February 12, 2020 2:26 AM > > > > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com>; > > > > Atish Patra <atishp at atishpatra.org>; Ard Biesheuvel > > > > <ard.biesheuvel at linaro.org> > > > > Cc: Alexander Graf <agraf at csgraf.de>; U-Boot Mailing List <u- > > > > boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > > > > leif at nuviainc.com > > > > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > > > > > > > On 2/7/20 4:13 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > > > > > > > > > > > > >> -----Original Message----- > > > > >> From: Atish Patra [mailto:atishp at atishpatra.org] > > > > >> Sent: Friday, February 7, 2020 6:56 AM > > > > >> To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, Abner (HPS > > > > >> SW/FW > > > > >> Technologist) <abner.chang at hpe.com> > > > > >> Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt > > > > >> <xypron.glpk at gmx.de>; U-Boot Mailing List <u-boot at lists.denx.de>; > > > > >> Atish Patra <atish.patra at wdc.com>; leif at nuviainc.com > > > > >> Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI > > > > >> setup > > > > >> > > > > >> On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel > > > > >> <ard.biesheuvel at linaro.org> > > > > >> wrote: > > > > >>> > > > > >>> On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: > > > > >>>> > > > > >>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> > > > > wrote: > > > > >>>>> > > > > >>>>> > > > > >>>>> On 06.02.20 19:28, Atish Patra wrote: > > > > >>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > > > >>>>>> <ard.biesheuvel at linaro.org> 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.a > > > > >>>>>>>> doc > > > > >>>>>>>> > > > > >>>>>>>> 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 active hart via a property in the /chosen node? > > > > >>>>>> Yes. RISC-V uses device tree. Technically, we can pass the active > > > > >>>>>> hart by a DT property but that means we have to modify the DT in > > > > >>>>>> OpenSBI (RISC-V specific run time service provider). > > > > >>>>>> We have been trying to avoid that if possible so that DT is not > > > > >>>>>> bounced around. This also limits U-Boot to have its own device > > > > >>>>>> tree. > > > > >>>>> > > > > >>>>> > > > > >>>>> I don't understand how this is different from the UEFI variable > > > > >>>>> scheme proposed here? If you want to create an SBI interface to > > > > >>>>> propagate the active HART that U-Boot then uses to populate the > > > > >>>>> /chosen property, that's probably fine as well. > > > > >>>>> > > > > >>>> > > > > >>>> We don't want to create SBI interface to pass this information. > > > > >>>> > > > > >>>>> We already have hooks that allow you to modify the DT right before > > > > >>>>> it gets delivered to the payload. Just add it there? > > > > >>>>> > > > > >>>> > > > > >>>> Hmm. I guess it is true if the DT is loaded from MMC or network as well. > > > > >>>> How about EDK2 ? If we go DT route, it also has to modify the DT to > > > > >>>> pass the boot hart. > > > > >>>> > > > > >>>> As it requires DT modification in multiple projects, why not use > > > > >>>> efi configuration tables as suggested by Ard ? > > > > >>>> > > > > >>> > > > > >>> Configuration tables are preferred over variables, but putting it in > > > > >>> the DT makes even more sense, since in that case, nothing that runs > > > > >>> in the UEFI context has to care about any of this. > > > > >>> > > > > >>>>>> > > > > >>>>>> > > > > >>>>>>> I'd assume the EFI 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. > > > > >>>>>> Yes. EFI stub doesn't care about this information. However, it > > > > >>>>>> needs to save the information somewhere so that it can pass to > > > > >>>>>> the real kernel after exiting boot time services. > > > > >>>>> > > > > >>>>> > > > > >>>>> DT sounds like a pretty natural choice for that to me :). > > > > >>>>> > > > > >>> > > > > >>> Indeed. DT has a /chosen node that is set aside for this purpose. It > > > > >>> does depend on how early you need the value (i.e., before or after > > > > >>> you can run C code), but since you are passing the DT address to the > > > > >>> core kernel, it makes way more sense to drop any additional > > > > >>> information that you need to pass in there. > > > > >> > > > > >> We don't need boot hart id until real kernel boots and parse DT. So > > > > >> that should be okay. > > > > >> I just looked at the efi stub code once more and realized that it is > > > > >> already parsing the DT to setup uefi memory maps from /chosen node. > > > > >> Adding boot hart id to the chosen node does seem much cleaner to me > > > > >> :). Thanks for all the explanations. > > > > >> > > > > >> I have not looked at EDK2 code. But I am assuming modifying the DT > > > > >> just before jumping to the payload won't be too hard for EDK2 as well. > > > > > We don?t use DT in edk2 RISC-V port and we pass boot HART ID in SMBIOS > > > > > type 44h as it is spec out in below link, > > > > > https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBIOS.md > > > > > > > > Thanks for the link. > > > > > > > > For 'RISC-V SMBIOS Type 44 Processor Additional Information' I find entry > > > > 0x13h 1: This is boot hart to boot system . > > > > > > > > But is '44' a hexadecimal number? The document does not indicate this. > > > Type '44' is decimal format as it mentioned in SMBIOS spec, I had typo in above which said '44h'. However, that's good to mention this in RISCV_SMBIOS.md. Thanks for the recommendation. > > > > > > > > SMBIOS data is intended to describe the hardware to system > > administrators, not to the OS loader, and I don't think it makes sense > > to rely on it for booting. I'd assume that SMBIOS tables are not > > mandatory to begin with. > > > > For EFI boot, it is acceptable if the stub loader in Linux itself > > needs to obtain the value from something like a device tree and pass > > it in a CPU register at handover time, > > That's what I am planning to do for now. We can add SMBIOS parsing as well > if required in future. > > although I would still prefer > > it if the kernel simply gets it from the device tree directly if one > > is guaranteed to be available. > > > > That would break current booting protocol in RISC-V where register "a0" should > contain the booting hartid. If we have to move away for that method, > changes need to > be in multiple places (to modify the DT) and it has to be done in a > backward compatible > way. > How do you pass the device tree address? > > Adding a new ABI between the firmware and the stub loader in Linux to > > use EFI specific conduits like config tables or EFI variables should > > really be avoided, though, as it affects every EFI loader while the > > code that runs in the EFI context doesn't even care (note that beyond > > u-boot and GRUB, there are other EFI loaders such as systemd-boot that > > need to be taken into account). > > Which booting stage should be responsible for changing the DT for > those EFI loaders ? > If the EFI stub for RISC-V needs to read the hart id from somewhere and pass it in a register when it enters the startup code of the core kernel, that is fine. Since DT is mandatory on your systems, and EFI is not, defining some EFI specific way of conveying this information seems like a bad idea to me. I'd say the firmware stage that incorporates the DT stuffs the hartid in /chosen so that any later stage can find it there if it needs to, without the software having to be aware of this. That way, you can use intermediate loaders like GRUB or systemd-boot without any changes. (This would actually be true when using a EFI variable for the same purpose, but I still prefer DT for this)
On Thu, Feb 13, 2020 at 2:11 PM Ard Biesheuvel <ard.biesheuvel at linaro.org> wrote: > > On Thu, 13 Feb 2020 at 19:59, Atish Patra <atishp at atishpatra.org> wrote: > > > > On Tue, Feb 11, 2020 at 11:28 PM Ard Biesheuvel > > <ard.biesheuvel at linaro.org> wrote: > > > > > > On Wed, 12 Feb 2020 at 06:49, Chang, Abner (HPS SW/FW Technologist) > > > <abner.chang at hpe.com> wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de] > > > > > Sent: Wednesday, February 12, 2020 2:26 AM > > > > > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com>; > > > > > Atish Patra <atishp at atishpatra.org>; Ard Biesheuvel > > > > > <ard.biesheuvel at linaro.org> > > > > > Cc: Alexander Graf <agraf at csgraf.de>; U-Boot Mailing List <u- > > > > > boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > > > > > leif at nuviainc.com > > > > > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > > > > > > > > > On 2/7/20 4:13 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > > > > > > > > > > > > > > > >> -----Original Message----- > > > > > >> From: Atish Patra [mailto:atishp at atishpatra.org] > > > > > >> Sent: Friday, February 7, 2020 6:56 AM > > > > > >> To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, Abner (HPS > > > > > >> SW/FW > > > > > >> Technologist) <abner.chang at hpe.com> > > > > > >> Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt > > > > > >> <xypron.glpk at gmx.de>; U-Boot Mailing List <u-boot at lists.denx.de>; > > > > > >> Atish Patra <atish.patra at wdc.com>; leif at nuviainc.com > > > > > >> Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI > > > > > >> setup > > > > > >> > > > > > >> On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel > > > > > >> <ard.biesheuvel at linaro.org> > > > > > >> wrote: > > > > > >>> > > > > > >>> On Thu, 6 Feb 2020 at 21:06, Atish Patra <atishp at atishpatra.org> wrote: > > > > > >>>> > > > > > >>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <agraf at csgraf.de> > > > > > wrote: > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> On 06.02.20 19:28, Atish Patra wrote: > > > > > >>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > > > > >>>>>> <ard.biesheuvel at linaro.org> 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.a > > > > > >>>>>>>> doc > > > > > >>>>>>>> > > > > > >>>>>>>> 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 active hart via a property in the /chosen node? > > > > > >>>>>> Yes. RISC-V uses device tree. Technically, we can pass the active > > > > > >>>>>> hart by a DT property but that means we have to modify the DT in > > > > > >>>>>> OpenSBI (RISC-V specific run time service provider). > > > > > >>>>>> We have been trying to avoid that if possible so that DT is not > > > > > >>>>>> bounced around. This also limits U-Boot to have its own device > > > > > >>>>>> tree. > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> I don't understand how this is different from the UEFI variable > > > > > >>>>> scheme proposed here? If you want to create an SBI interface to > > > > > >>>>> propagate the active HART that U-Boot then uses to populate the > > > > > >>>>> /chosen property, that's probably fine as well. > > > > > >>>>> > > > > > >>>> > > > > > >>>> We don't want to create SBI interface to pass this information. > > > > > >>>> > > > > > >>>>> We already have hooks that allow you to modify the DT right before > > > > > >>>>> it gets delivered to the payload. Just add it there? > > > > > >>>>> > > > > > >>>> > > > > > >>>> Hmm. I guess it is true if the DT is loaded from MMC or network as well. > > > > > >>>> How about EDK2 ? If we go DT route, it also has to modify the DT to > > > > > >>>> pass the boot hart. > > > > > >>>> > > > > > >>>> As it requires DT modification in multiple projects, why not use > > > > > >>>> efi configuration tables as suggested by Ard ? > > > > > >>>> > > > > > >>> > > > > > >>> Configuration tables are preferred over variables, but putting it in > > > > > >>> the DT makes even more sense, since in that case, nothing that runs > > > > > >>> in the UEFI context has to care about any of this. > > > > > >>> > > > > > >>>>>> > > > > > >>>>>> > > > > > >>>>>>> I'd assume the EFI 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. > > > > > >>>>>> Yes. EFI stub doesn't care about this information. However, it > > > > > >>>>>> needs to save the information somewhere so that it can pass to > > > > > >>>>>> the real kernel after exiting boot time services. > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> DT sounds like a pretty natural choice for that to me :). > > > > > >>>>> > > > > > >>> > > > > > >>> Indeed. DT has a /chosen node that is set aside for this purpose. It > > > > > >>> does depend on how early you need the value (i.e., before or after > > > > > >>> you can run C code), but since you are passing the DT address to the > > > > > >>> core kernel, it makes way more sense to drop any additional > > > > > >>> information that you need to pass in there. > > > > > >> > > > > > >> We don't need boot hart id until real kernel boots and parse DT. So > > > > > >> that should be okay. > > > > > >> I just looked at the efi stub code once more and realized that it is > > > > > >> already parsing the DT to setup uefi memory maps from /chosen node. > > > > > >> Adding boot hart id to the chosen node does seem much cleaner to me > > > > > >> :). Thanks for all the explanations. > > > > > >> > > > > > >> I have not looked at EDK2 code. But I am assuming modifying the DT > > > > > >> just before jumping to the payload won't be too hard for EDK2 as well. > > > > > > We don?t use DT in edk2 RISC-V port and we pass boot HART ID in SMBIOS > > > > > > type 44h as it is spec out in below link, > > > > > > https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBIOS.md > > > > > > > > > > Thanks for the link. > > > > > > > > > > For 'RISC-V SMBIOS Type 44 Processor Additional Information' I find entry > > > > > 0x13h 1: This is boot hart to boot system . > > > > > > > > > > But is '44' a hexadecimal number? The document does not indicate this. > > > > Type '44' is decimal format as it mentioned in SMBIOS spec, I had typo in above which said '44h'. However, that's good to mention this in RISCV_SMBIOS.md. Thanks for the recommendation. > > > > > > > > > > > SMBIOS data is intended to describe the hardware to system > > > administrators, not to the OS loader, and I don't think it makes sense > > > to rely on it for booting. I'd assume that SMBIOS tables are not > > > mandatory to begin with. > > > > > > For EFI boot, it is acceptable if the stub loader in Linux itself > > > needs to obtain the value from something like a device tree and pass > > > it in a CPU register at handover time, > > > > That's what I am planning to do for now. We can add SMBIOS parsing as well > > if required in future. > > > > although I would still prefer > > > it if the kernel simply gets it from the device tree directly if one > > > is guaranteed to be available. > > > > > > > That would break current booting protocol in RISC-V where register "a0" should > > contain the booting hartid. If we have to move away for that method, > > changes need to > > be in multiple places (to modify the DT) and it has to be done in a > > backward compatible > > way. > > > > How do you pass the device tree address? in register "a1" > > > Adding a new ABI between the firmware and the stub loader in Linux to > > > use EFI specific conduits like config tables or EFI variables should > > > really be avoided, though, as it affects every EFI loader while the > > > code that runs in the EFI context doesn't even care (note that beyond > > > u-boot and GRUB, there are other EFI loaders such as systemd-boot that > > > need to be taken into account). > > > > Which booting stage should be responsible for changing the DT for > > those EFI loaders ? > > > > If the EFI stub for RISC-V needs to read the hart id from somewhere > and pass it in a register when it enters the startup code of the core > kernel, that is fine. > > Since DT is mandatory on your systems, and EFI is not, defining some > EFI specific way of conveying this information seems like a bad idea > to me. > > I'd say the firmware stage that incorporates the DT stuffs the hartid > in /chosen so that any later stage can find it there if it needs to, > without the software having to be aware of this. That way, you can use > intermediate loaders like GRUB or systemd-boot without any changes. > (This would actually be true when using a EFI variable for the same > purpose, but I still prefer DT for this) I am fine with this. For now, U-Boot can append the chosen node. @Abner: I might have missed something. But I couldn't find anything other than boot hartid in SMBIOS table that EFI stub need to parse in order to boot kernel. How difficult is to modify the DT in EDK2 ?
> -----Original Message----- > From: Atish Patra [mailto:atishp at atishpatra.org] > Sent: Friday, February 14, 2020 7:57 AM > To: Ard Biesheuvel <ard.biesheuvel at linaro.org> > Cc: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com>; > Heinrich Schuchardt <xypron.glpk at gmx.de>; Alexander Graf > <agraf at csgraf.de>; U-Boot Mailing List <u-boot at lists.denx.de>; Atish Patra > <atish.patra at wdc.com>; leif at nuviainc.com > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > On Thu, Feb 13, 2020 at 2:11 PM Ard Biesheuvel <ard.biesheuvel at linaro.org> > wrote: > > > > On Thu, 13 Feb 2020 at 19:59, Atish Patra <atishp at atishpatra.org> wrote: > > > > > > On Tue, Feb 11, 2020 at 11:28 PM Ard Biesheuvel > > > <ard.biesheuvel at linaro.org> wrote: > > > > > > > > On Wed, 12 Feb 2020 at 06:49, Chang, Abner (HPS SW/FW > > > > Technologist) <abner.chang at hpe.com> wrote: > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de] > > > > > > Sent: Wednesday, February 12, 2020 2:26 AM > > > > > > To: Chang, Abner (HPS SW/FW Technologist) > > > > > > <abner.chang at hpe.com>; Atish Patra <atishp at atishpatra.org>; > > > > > > Ard Biesheuvel <ard.biesheuvel at linaro.org> > > > > > > Cc: Alexander Graf <agraf at csgraf.de>; U-Boot Mailing List <u- > > > > > > boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > > > > > > leif at nuviainc.com > > > > > > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific > > > > > > UEFI setup > > > > > > > > > > > > On 2/7/20 4:13 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > > > > > > > > > > > > > > > > > > >> -----Original Message----- > > > > > > >> From: Atish Patra [mailto:atishp at atishpatra.org] > > > > > > >> Sent: Friday, February 7, 2020 6:56 AM > > > > > > >> To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, > > > > > > >> Abner (HPS SW/FW > > > > > > >> Technologist) <abner.chang at hpe.com> > > > > > > >> Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt > > > > > > >> <xypron.glpk at gmx.de>; U-Boot Mailing List > > > > > > >> <u-boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > > > > > > >> leif at nuviainc.com > > > > > > >> Subject: Re: [PATCH v2 1/1] efi_loader: architecture > > > > > > >> specific UEFI setup > > > > > > >> > > > > > > >> On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel > > > > > > >> <ard.biesheuvel at linaro.org> > > > > > > >> wrote: > > > > > > >>> > > > > > > >>> On Thu, 6 Feb 2020 at 21:06, Atish Patra > <atishp at atishpatra.org> wrote: > > > > > > >>>> > > > > > > >>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf > > > > > > >>>> <agraf at csgraf.de> > > > > > > wrote: > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> On 06.02.20 19:28, Atish Patra wrote: > > > > > > >>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > > > > > >>>>>> <ard.biesheuvel at linaro.org> 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/ri > > > > > > >>>>>>>> scv-sbi.a > > > > > > >>>>>>>> doc > > > > > > >>>>>>>> > > > > > > >>>>>>>> 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 active hart via a property in the /chosen node? > > > > > > >>>>>> Yes. RISC-V uses device tree. Technically, we can pass > > > > > > >>>>>> the active hart by a DT property but that means we have > > > > > > >>>>>> to modify the DT in OpenSBI (RISC-V specific run time > service provider). > > > > > > >>>>>> We have been trying to avoid that if possible so that > > > > > > >>>>>> DT is not bounced around. This also limits U-Boot to > > > > > > >>>>>> have its own device tree. > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> I don't understand how this is different from the UEFI > > > > > > >>>>> variable scheme proposed here? If you want to create an > > > > > > >>>>> SBI interface to propagate the active HART that U-Boot > > > > > > >>>>> then uses to populate the /chosen property, that's probably > fine as well. > > > > > > >>>>> > > > > > > >>>> > > > > > > >>>> We don't want to create SBI interface to pass this information. > > > > > > >>>> > > > > > > >>>>> We already have hooks that allow you to modify the DT > > > > > > >>>>> right before it gets delivered to the payload. Just add it there? > > > > > > >>>>> > > > > > > >>>> > > > > > > >>>> Hmm. I guess it is true if the DT is loaded from MMC or > network as well. > > > > > > >>>> How about EDK2 ? If we go DT route, it also has to modify > > > > > > >>>> the DT to pass the boot hart. > > > > > > >>>> > > > > > > >>>> As it requires DT modification in multiple projects, why > > > > > > >>>> not use efi configuration tables as suggested by Ard ? > > > > > > >>>> > > > > > > >>> > > > > > > >>> Configuration tables are preferred over variables, but > > > > > > >>> putting it in the DT makes even more sense, since in that > > > > > > >>> case, nothing that runs in the UEFI context has to care about > any of this. > > > > > > >>> > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>>>> I'd assume the EFI 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. > > > > > > >>>>>> Yes. EFI stub doesn't care about this information. > > > > > > >>>>>> However, it needs to save the information somewhere so > > > > > > >>>>>> that it can pass to the real kernel after exiting boot time > services. > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> DT sounds like a pretty natural choice for that to me :). > > > > > > >>>>> > > > > > > >>> > > > > > > >>> Indeed. DT has a /chosen node that is set aside for this > > > > > > >>> purpose. It does depend on how early you need the value > > > > > > >>> (i.e., before or after you can run C code), but since you > > > > > > >>> are passing the DT address to the core kernel, it makes > > > > > > >>> way more sense to drop any additional information that you > need to pass in there. > > > > > > >> > > > > > > >> We don't need boot hart id until real kernel boots and > > > > > > >> parse DT. So that should be okay. > > > > > > >> I just looked at the efi stub code once more and realized > > > > > > >> that it is already parsing the DT to setup uefi memory maps from > /chosen node. > > > > > > >> Adding boot hart id to the chosen node does seem much > > > > > > >> cleaner to me :). Thanks for all the explanations. > > > > > > >> > > > > > > >> I have not looked at EDK2 code. But I am assuming modifying > > > > > > >> the DT just before jumping to the payload won't be too hard for > EDK2 as well. > > > > > > > We don?t use DT in edk2 RISC-V port and we pass boot HART ID > > > > > > > in SMBIOS type 44h as it is spec out in below link, > > > > > > > https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBI > > > > > > > OS.md > > > > > > > > > > > > Thanks for the link. > > > > > > > > > > > > For 'RISC-V SMBIOS Type 44 Processor Additional Information' I > > > > > > find entry 0x13h 1: This is boot hart to boot system . > > > > > > > > > > > > But is '44' a hexadecimal number? The document does not indicate > this. > > > > > Type '44' is decimal format as it mentioned in SMBIOS spec, I had typo > in above which said '44h'. However, that's good to mention this in > RISCV_SMBIOS.md. Thanks for the recommendation. > > > > > > > > > > > > > > SMBIOS data is intended to describe the hardware to system > > > > administrators, not to the OS loader, and I don't think it makes > > > > sense to rely on it for booting. I'd assume that SMBIOS tables are > > > > not mandatory to begin with. > > > > > > > > For EFI boot, it is acceptable if the stub loader in Linux itself > > > > needs to obtain the value from something like a device tree and > > > > pass it in a CPU register at handover time, > > > > > > That's what I am planning to do for now. We can add SMBIOS parsing > > > as well if required in future. > > > > > > although I would still prefer > > > > it if the kernel simply gets it from the device tree directly if > > > > one is guaranteed to be available. > > > > > > > > > > That would break current booting protocol in RISC-V where register > > > "a0" should contain the booting hartid. If we have to move away for > > > that method, changes need to be in multiple places (to modify the > > > DT) and it has to be done in a backward compatible way. > > > > > > > How do you pass the device tree address? > > in register "a1" > > > > > Adding a new ABI between the firmware and the stub loader in Linux > > > > to use EFI specific conduits like config tables or EFI variables > > > > should really be avoided, though, as it affects every EFI loader > > > > while the code that runs in the EFI context doesn't even care > > > > (note that beyond u-boot and GRUB, there are other EFI loaders > > > > such as systemd-boot that need to be taken into account). > > > > > > Which booting stage should be responsible for changing the DT for > > > those EFI loaders ? > > > > > > > If the EFI stub for RISC-V needs to read the hart id from somewhere > > and pass it in a register when it enters the startup code of the core > > kernel, that is fine. > > > > Since DT is mandatory on your systems, and EFI is not, defining some > > EFI specific way of conveying this information seems like a bad idea > > to me. > > > > I'd say the firmware stage that incorporates the DT stuffs the hartid > > in /chosen so that any later stage can find it there if it needs to, > > without the software having to be aware of this. That way, you can use > > intermediate loaders like GRUB or systemd-boot without any changes. > > (This would actually be true when using a EFI variable for the same > > purpose, but I still prefer DT for this) > > I am fine with this. For now, U-Boot can append the chosen node. > > @Abner: I might have missed something. But I couldn't find anything other > than boot hartid in SMBIOS table that EFI stub need to parse in order to boot The table from this link https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBIOS.md Offset 3 is HART ID, and offset 13h is the boolean indicates this hart is the boot hart. > kernel. How difficult is to modify the DT in EDK2 ? > I never used DT before on PC/Server project. However, the DT code is over there in edk2 repo which mostly used by ARM platforms. I don?t think it is difficult to adopt it though. > -- > Regards, > Atish
> -----Original Message----- > From: Alexander Graf [mailto:agraf at csgraf.de] > Sent: Friday, February 14, 2020 4:07 PM > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com> > Cc: Atish Patra <atishp at atishpatra.org>; Ard Biesheuvel > <ard.biesheuvel at linaro.org>; Heinrich Schuchardt <xypron.glpk at gmx.de>; > U-Boot Mailing List <u-boot at lists.denx.de>; Atish Patra > <Atish.Patra at wdc.com>; leif at nuviainc.com > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > > > > Am 14.02.2020 um 05:21 schrieb Chang, Abner (HPS SW/FW Technologist) > <abner.chang at hpe.com>: > > > > > > > >> -----Original Message----- > >> From: Atish Patra [mailto:atishp at atishpatra.org] > >> Sent: Friday, February 14, 2020 7:57 AM > >> To: Ard Biesheuvel <ard.biesheuvel at linaro.org> > >> Cc: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com>; > >> Heinrich Schuchardt <xypron.glpk at gmx.de>; Alexander Graf > >> <agraf at csgraf.de>; U-Boot Mailing List <u-boot at lists.denx.de>; Atish > Patra > >> <atish.patra at wdc.com>; leif at nuviainc.com > >> Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > >> > >>> On Thu, Feb 13, 2020 at 2:11 PM Ard Biesheuvel > <ard.biesheuvel at linaro.org> > >>> wrote: > >>> > >>> On Thu, 13 Feb 2020 at 19:59, Atish Patra <atishp at atishpatra.org> wrote: > >>>> > >>>> On Tue, Feb 11, 2020 at 11:28 PM Ard Biesheuvel > >>>> <ard.biesheuvel at linaro.org> wrote: > >>>>> > >>>>> On Wed, 12 Feb 2020 at 06:49, Chang, Abner (HPS SW/FW > >>>>> Technologist) <abner.chang at hpe.com> wrote: > >>>>>> > >>>>>> > >>>>>> > >>>>>>> -----Original Message----- > >>>>>>> From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de] > >>>>>>> Sent: Wednesday, February 12, 2020 2:26 AM > >>>>>>> To: Chang, Abner (HPS SW/FW Technologist) > >>>>>>> <abner.chang at hpe.com>; Atish Patra <atishp at atishpatra.org>; > >>>>>>> Ard Biesheuvel <ard.biesheuvel at linaro.org> > >>>>>>> Cc: Alexander Graf <agraf at csgraf.de>; U-Boot Mailing List <u- > >>>>>>> boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > >>>>>>> leif at nuviainc.com > >>>>>>> Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific > >>>>>>> UEFI setup > >>>>>>> > >>>>>>> On 2/7/20 4:13 AM, Chang, Abner (HPS SW/FW Technologist) wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>>> -----Original Message----- > >>>>>>>>> From: Atish Patra [mailto:atishp at atishpatra.org] > >>>>>>>>> Sent: Friday, February 7, 2020 6:56 AM > >>>>>>>>> To: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Chang, > >>>>>>>>> Abner (HPS SW/FW > >>>>>>>>> Technologist) <abner.chang at hpe.com> > >>>>>>>>> Cc: Alexander Graf <agraf at csgraf.de>; Heinrich Schuchardt > >>>>>>>>> <xypron.glpk at gmx.de>; U-Boot Mailing List > >>>>>>>>> <u-boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; > >>>>>>>>> leif at nuviainc.com > >>>>>>>>> Subject: Re: [PATCH v2 1/1] efi_loader: architecture > >>>>>>>>> specific UEFI setup > >>>>>>>>> > >>>>>>>>> On Thu, Feb 6, 2020 at 2:07 PM Ard Biesheuvel > >>>>>>>>> <ard.biesheuvel at linaro.org> > >>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>> On Thu, 6 Feb 2020 at 21:06, Atish Patra > >> <atishp at atishpatra.org> wrote: > >>>>>>>>>>> > >>>>>>>>>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf > >>>>>>>>>>> <agraf at csgraf.de> > >>>>>>> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> On 06.02.20 19:28, Atish Patra wrote: > >>>>>>>>>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > >>>>>>>>>>>>> <ard.biesheuvel at linaro.org> 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/ri > >>>>>>>>>>>>>>> scv-sbi.a > >>>>>>>>>>>>>>> doc > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> 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 active hart via a property in the /chosen node? > >>>>>>>>>>>>> Yes. RISC-V uses device tree. Technically, we can pass > >>>>>>>>>>>>> the active hart by a DT property but that means we have > >>>>>>>>>>>>> to modify the DT in OpenSBI (RISC-V specific run time > >> service provider). > >>>>>>>>>>>>> We have been trying to avoid that if possible so that > >>>>>>>>>>>>> DT is not bounced around. This also limits U-Boot to > >>>>>>>>>>>>> have its own device tree. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> I don't understand how this is different from the UEFI > >>>>>>>>>>>> variable scheme proposed here? If you want to create an > >>>>>>>>>>>> SBI interface to propagate the active HART that U-Boot > >>>>>>>>>>>> then uses to populate the /chosen property, that's probably > >> fine as well. > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> We don't want to create SBI interface to pass this information. > >>>>>>>>>>> > >>>>>>>>>>>> We already have hooks that allow you to modify the DT > >>>>>>>>>>>> right before it gets delivered to the payload. Just add it there? > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Hmm. I guess it is true if the DT is loaded from MMC or > >> network as well. > >>>>>>>>>>> How about EDK2 ? If we go DT route, it also has to modify > >>>>>>>>>>> the DT to pass the boot hart. > >>>>>>>>>>> > >>>>>>>>>>> As it requires DT modification in multiple projects, why > >>>>>>>>>>> not use efi configuration tables as suggested by Ard ? > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> Configuration tables are preferred over variables, but > >>>>>>>>>> putting it in the DT makes even more sense, since in that > >>>>>>>>>> case, nothing that runs in the UEFI context has to care about > >> any of this. > >>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>>> I'd assume the EFI 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. > >>>>>>>>>>>>> Yes. EFI stub doesn't care about this information. > >>>>>>>>>>>>> However, it needs to save the information somewhere so > >>>>>>>>>>>>> that it can pass to the real kernel after exiting boot time > >> services. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> DT sounds like a pretty natural choice for that to me :). > >>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> Indeed. DT has a /chosen node that is set aside for this > >>>>>>>>>> purpose. It does depend on how early you need the value > >>>>>>>>>> (i.e., before or after you can run C code), but since you > >>>>>>>>>> are passing the DT address to the core kernel, it makes > >>>>>>>>>> way more sense to drop any additional information that you > >> need to pass in there. > >>>>>>>>> > >>>>>>>>> We don't need boot hart id until real kernel boots and > >>>>>>>>> parse DT. So that should be okay. > >>>>>>>>> I just looked at the efi stub code once more and realized > >>>>>>>>> that it is already parsing the DT to setup uefi memory maps from > >> /chosen node. > >>>>>>>>> Adding boot hart id to the chosen node does seem much > >>>>>>>>> cleaner to me :). Thanks for all the explanations. > >>>>>>>>> > >>>>>>>>> I have not looked at EDK2 code. But I am assuming modifying > >>>>>>>>> the DT just before jumping to the payload won't be too hard for > >> EDK2 as well. > >>>>>>>> We don?t use DT in edk2 RISC-V port and we pass boot HART ID > >>>>>>>> in SMBIOS type 44h as it is spec out in below link, > >>>>>>>> https://github.com/riscv/riscv-smbios/blob/master/RISCV-SMBI > >>>>>>>> OS.md > >>>>>>> > >>>>>>> Thanks for the link. > >>>>>>> > >>>>>>> For 'RISC-V SMBIOS Type 44 Processor Additional Information' I > >>>>>>> find entry 0x13h 1: This is boot hart to boot system . > >>>>>>> > >>>>>>> But is '44' a hexadecimal number? The document does not indicate > >> this. > >>>>>> Type '44' is decimal format as it mentioned in SMBIOS spec, I had > typo > >> in above which said '44h'. However, that's good to mention this in > >> RISCV_SMBIOS.md. Thanks for the recommendation. > >>>>>>> > >>>>> > >>>>> SMBIOS data is intended to describe the hardware to system > >>>>> administrators, not to the OS loader, and I don't think it makes > >>>>> sense to rely on it for booting. I'd assume that SMBIOS tables are > >>>>> not mandatory to begin with. > >>>>> > >>>>> For EFI boot, it is acceptable if the stub loader in Linux itself > >>>>> needs to obtain the value from something like a device tree and > >>>>> pass it in a CPU register at handover time, > >>>> > >>>> That's what I am planning to do for now. We can add SMBIOS parsing > >>>> as well if required in future. > >>>> > >>>> although I would still prefer > >>>>> it if the kernel simply gets it from the device tree directly if > >>>>> one is guaranteed to be available. > >>>>> > >>>> > >>>> That would break current booting protocol in RISC-V where register > >>>> "a0" should contain the booting hartid. If we have to move away for > >>>> that method, changes need to be in multiple places (to modify the > >>>> DT) and it has to be done in a backward compatible way. > >>>> > >>> > >>> How do you pass the device tree address? > >> > >> in register "a1" > >> > >>>>> Adding a new ABI between the firmware and the stub loader in Linux > >>>>> to use EFI specific conduits like config tables or EFI variables > >>>>> should really be avoided, though, as it affects every EFI loader > >>>>> while the code that runs in the EFI context doesn't even care > >>>>> (note that beyond u-boot and GRUB, there are other EFI loaders > >>>>> such as systemd-boot that need to be taken into account). > >>>> > >>>> Which booting stage should be responsible for changing the DT for > >>>> those EFI loaders ? > >>>> > >>> > >>> If the EFI stub for RISC-V needs to read the hart id from somewhere > >>> and pass it in a register when it enters the startup code of the core > >>> kernel, that is fine. > >>> > >>> Since DT is mandatory on your systems, and EFI is not, defining some > >>> EFI specific way of conveying this information seems like a bad idea > >>> to me. > >>> > >>> I'd say the firmware stage that incorporates the DT stuffs the hartid > >>> in /chosen so that any later stage can find it there if it needs to, > >>> without the software having to be aware of this. That way, you can use > >>> intermediate loaders like GRUB or systemd-boot without any changes. > >>> (This would actually be true when using a EFI variable for the same > >>> purpose, but I still prefer DT for this) > >> > >> I am fine with this. For now, U-Boot can append the chosen node. > >> > >> @Abner: I might have missed something. But I couldn't find anything > other > >> than boot hartid in SMBIOS table that EFI stub need to parse in order to > boot > > The table from this link https://github.com/riscv/riscv- > smbios/blob/master/RISCV-SMBIOS.md > > Offset 3 is HART ID, and offset 13h is the boolean indicates this hart is the > boot hart. > > > >> kernel. How difficult is to modify the DT in EDK2 ? > >> > > I never used DT before on PC/Server project. However, the DT code is over > there in edk2 repo which mostly used by ARM platforms. I don?t think it is > difficult to adopt it though. > > Yes, some arm platforms already transform the DT just fine. Let's really > please not use SMBIOS for anything boot or system configuration related: its > purpose is in general purely informational. As DT to embedded system, SMBIOS provides system information/configuration on most PC/Server platforms. Especially to pre-OS applications such as EFI diagnostic tool, factory/customer deployment tools, pre-OS system configuration, network boot image and EFI OS boot loader as well. The intention of RISC-V SMBIOS is support above applications using single image for the RISC-V core variants, Non ACPI-aware OS is also part of this scope, but it is rare though. If you are booting to OS which is actually well understand DT then just use DT, but for PC/Server industry, SMBIOS would be still our choice before OS. We may have to define the corresponding syntax If DT doesn't have it for boot HART information. RISC-V System Description Task Group (f it formed) would be a good place to bring this topic. Maybe you can support both DT or SMBIOS to retrieve the information you need on demand... > > So yes, unless I hear really good arguments against passing it via /chosen in > DT, I'd strongly prefer that mechanism. For ACPI (if it ever happens), there > would be a special ACPI table for it anyway. Yes, we do have an ECR for ACPI spec to report the RISC-V core characteristic which is similar to what we done for SMBIOS. > > > Thanks, > > Alex >
> -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel at linaro.org] > Sent: Friday, February 14, 2020 7:33 PM > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com> > Cc: Alexander Graf <agraf at csgraf.de>; Atish Patra <atishp at atishpatra.org>; > Heinrich Schuchardt <xypron.glpk at gmx.de>; U-Boot Mailing List <u- > boot at lists.denx.de>; Atish Patra <Atish.Patra at wdc.com>; > leif at nuviainc.com > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > On Fri, 14 Feb 2020 at 12:27, Chang, Abner (HPS SW/FW Technologist) > <abner.chang at hpe.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Alexander Graf [mailto:agraf at csgraf.de] > > > Sent: Friday, February 14, 2020 4:07 PM > > > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com> > > > Cc: Atish Patra <atishp at atishpatra.org>; Ard Biesheuvel > > > <ard.biesheuvel at linaro.org>; Heinrich Schuchardt > > > <xypron.glpk at gmx.de>; U-Boot Mailing List <u-boot at lists.denx.de>; > > > Atish Patra <Atish.Patra at wdc.com>; leif at nuviainc.com > > > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI > > > setup > > > > > > > > > > > > > Am 14.02.2020 um 05:21 schrieb Chang, Abner (HPS SW/FW > > > > Technologist) > > > <abner.chang at hpe.com>: > > > > > > ... > > > The table from this link https://github.com/riscv/riscv- > > > smbios/blob/master/RISCV-SMBIOS.md > > > > Offset 3 is HART ID, and offset 13h is the boolean indicates this > > > > hart is the > > > boot hart. > > > > > > > >> kernel. How difficult is to modify the DT in EDK2 ? > > > >> > > > > I never used DT before on PC/Server project. However, the DT code > > > > is over > > > there in edk2 repo which mostly used by ARM platforms. I don?t think > > > it is difficult to adopt it though. > > > > > > Yes, some arm platforms already transform the DT just fine. Let's > > > really please not use SMBIOS for anything boot or system > > > configuration related: its purpose is in general purely informational. > > As DT to embedded system, SMBIOS provides system > information/configuration on most PC/Server platforms. Especially to pre-OS > applications such as EFI diagnostic tool, factory/customer deployment tools, > pre-OS system configuration, network boot image and EFI OS boot loader as > well. The intention of RISC-V SMBIOS is support above applications using > single image for the RISC-V core variants, Non ACPI-aware OS is also part of > this scope, but it is rare though. > > If you are booting to OS which is actually well understand DT then just use > DT, but for PC/Server industry, SMBIOS would be still our choice before OS. > > We may have to define the corresponding syntax If DT doesn't have it for > boot HART information. RISC-V System Description Task Group (f it formed) > would be a good place to bring this topic. > > Maybe you can support both DT or SMBIOS to retrieve the information you > need on demand... > > > > SMBIOS is an informational protocol. Firmware or OS loaders should not rely > on it for low-level things like the hart id. Hart ID is just one of the information in type 44 which is the same as the processor information provided in type 4. > > > > > > > So yes, unless I hear really good arguments against passing it via > > > /chosen in DT, I'd strongly prefer that mechanism. For ACPI (if it > > > ever happens), there would be a special ACPI table for it anyway. > > Yes, we do have an ECR for ACPI spec to report the RISC-V core > characteristic which is similar to what we done for SMBIOS. > > > > So we'll end up with a DT+SMBIOS or ACPI+SMBIOS boot protocol, and we'll > always have to parse two sets of tables, just to discover the hart id. I don't > think that makes sense at all, to be honest. As I said, SMBIOS is mostly for pre OS applications, Type 42 is a good example, the Host interface used to talk to BMC and Redfish service in pre-OS environment (also runtime OS). For OS boot, maybe OS (like Windows) still retrieves information from it before ACPI enable. I have no preference of using which way to get boot hard ID for the boot process, either to get it from DT, SMBIOS or ACPI seems to me the same... just the information is provided over there > > SMBIOS is wonderful for the sysadmin to look at the model numbers of the > installed DIMMs etc. But for core boot stuff, we really should avoid it. Security consideration?
> -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel at linaro.org] > Sent: Friday, February 14, 2020 8:07 PM > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com> > Cc: Alexander Graf <agraf at csgraf.de>; Atish Patra <atishp at atishpatra.org>; > Heinrich Schuchardt <xypron.glpk at gmx.de>; U-Boot Mailing List <u- > boot at lists.denx.de>; Atish Patra <Atish.Patra at wdc.com>; > leif at nuviainc.com > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI setup > > On Fri, 14 Feb 2020 at 13:04, Chang, Abner (HPS SW/FW Technologist) > <abner.chang at hpe.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Ard Biesheuvel [mailto:ard.biesheuvel at linaro.org] > > > Sent: Friday, February 14, 2020 7:33 PM > > > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang at hpe.com> > > > Cc: Alexander Graf <agraf at csgraf.de>; Atish Patra > > > <atishp at atishpatra.org>; Heinrich Schuchardt <xypron.glpk at gmx.de>; > > > U-Boot Mailing List <u- boot at lists.denx.de>; Atish Patra > > > <Atish.Patra at wdc.com>; leif at nuviainc.com > > > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific UEFI > > > setup > > > > > > On Fri, 14 Feb 2020 at 12:27, Chang, Abner (HPS SW/FW Technologist) > > > <abner.chang at hpe.com> wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Alexander Graf [mailto:agraf at csgraf.de] > > > > > Sent: Friday, February 14, 2020 4:07 PM > > > > > To: Chang, Abner (HPS SW/FW Technologist) > <abner.chang at hpe.com> > > > > > Cc: Atish Patra <atishp at atishpatra.org>; Ard Biesheuvel > > > > > <ard.biesheuvel at linaro.org>; Heinrich Schuchardt > > > > > <xypron.glpk at gmx.de>; U-Boot Mailing List > > > > > <u-boot at lists.denx.de>; Atish Patra <Atish.Patra at wdc.com>; > > > > > leif at nuviainc.com > > > > > Subject: Re: [PATCH v2 1/1] efi_loader: architecture specific > > > > > UEFI setup > > > > > > > > > > > > > > > > > > > > > Am 14.02.2020 um 05:21 schrieb Chang, Abner (HPS SW/FW > > > > > > Technologist) > > > > > <abner.chang at hpe.com>: > > > > > > > > > > > > ... > > > > > The table from this link https://github.com/riscv/riscv- > > > > > smbios/blob/master/RISCV-SMBIOS.md > > > > > > Offset 3 is HART ID, and offset 13h is the boolean indicates > > > > > > this hart is the > > > > > boot hart. > > > > > > > > > > > >> kernel. How difficult is to modify the DT in EDK2 ? > > > > > >> > > > > > > I never used DT before on PC/Server project. However, the DT > > > > > > code is over > > > > > there in edk2 repo which mostly used by ARM platforms. I don?t > > > > > think it is difficult to adopt it though. > > > > > > > > > > Yes, some arm platforms already transform the DT just fine. > > > > > Let's really please not use SMBIOS for anything boot or system > > > > > configuration related: its purpose is in general purely informational. > > > > As DT to embedded system, SMBIOS provides system > > > information/configuration on most PC/Server platforms. Especially to > > > pre-OS applications such as EFI diagnostic tool, factory/customer > > > deployment tools, pre-OS system configuration, network boot image > > > and EFI OS boot loader as well. The intention of RISC-V SMBIOS is > > > support above applications using single image for the RISC-V core > > > variants, Non ACPI-aware OS is also part of this scope, but it is rare > though. > > > > If you are booting to OS which is actually well understand DT then > > > > just use > > > DT, but for PC/Server industry, SMBIOS would be still our choice before > OS. > > > > We may have to define the corresponding syntax If DT doesn't have > > > > it for > > > boot HART information. RISC-V System Description Task Group (f it > > > formed) would be a good place to bring this topic. > > > > Maybe you can support both DT or SMBIOS to retrieve the > > > > information you > > > need on demand... > > > > > > > > > > SMBIOS is an informational protocol. Firmware or OS loaders should > > > not rely on it for low-level things like the hart id. > > Hart ID is just one of the information in type 44 which is the same as the > processor information provided in type 4. > > > > Fine. But that doesn't mean we should be parsing SMBIOS tables in the Linux > startup code. Ok, this is not my familiar area. You guys are better than me. > > > > > > > > > > > > > > So yes, unless I hear really good arguments against passing it > > > > > via /chosen in DT, I'd strongly prefer that mechanism. For ACPI > > > > > (if it ever happens), there would be a special ACPI table for it anyway. > > > > Yes, we do have an ECR for ACPI spec to report the RISC-V core > > > characteristic which is similar to what we done for SMBIOS. > > > > > > > > > > So we'll end up with a DT+SMBIOS or ACPI+SMBIOS boot protocol, and > > > we'll always have to parse two sets of tables, just to discover the > > > hart id. I don't think that makes sense at all, to be honest. > > As I said, SMBIOS is mostly for pre OS applications, Type 42 is a good > example, the Host interface used to talk to BMC and Redfish service in pre- > OS environment (also runtime OS). > > For OS boot, maybe OS (like Windows) still retrieves information from it > before ACPI enable. > > > > I have no preference of using which way to get boot hard ID for the > > boot process, either to get it from DT, SMBIOS or ACPI seems to me > > the same... just the information is provided over there > > > > > > > > SMBIOS is wonderful for the sysadmin to look at the model numbers of > > > the installed DIMMs etc. But for core boot stuff, we really should avoid it. > > Security consideration? > > What security considerations did you have in mind? Hah this is my question for you. Can someone under Pre-OS environment and steal boot hard ID and do something bad? Or change it to run malicious code from another HART?
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;