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

Thu Feb 13 23:10:59 CET 2020

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)