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

Fri Feb 14 04:50:41 CET 2020


> -----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