On Thu, Feb 13, 2020 at 2:11 PM Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: > > On Thu, 13 Feb 2020 at 19:59, Atish Patra <ati...@atishpatra.org> wrote: > > > > On Tue, Feb 11, 2020 at 11:28 PM Ard Biesheuvel > > <ard.biesheu...@linaro.org> wrote: > > > > > > On Wed, 12 Feb 2020 at 06:49, Chang, Abner (HPS SW/FW Technologist) > > > <abner.ch...@hpe.com> wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Heinrich Schuchardt [mailto:xypron.g...@gmx.de] > > > > > Sent: Wednesday, February 12, 2020 2:26 AM > > > > > To: Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com>; > > > > > Atish Patra <ati...@atishpatra.org>; Ard Biesheuvel > > > > > <ard.biesheu...@linaro.org> > > > > > Cc: Alexander Graf <ag...@csgraf.de>; U-Boot Mailing List <u- > > > > > b...@lists.denx.de>; Atish Patra <atish.pa...@wdc.com>; > > > > > l...@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:ati...@atishpatra.org] > > > > > >> Sent: Friday, February 7, 2020 6:56 AM > > > > > >> To: Ard Biesheuvel <ard.biesheu...@linaro.org>; Chang, Abner (HPS > > > > > >> SW/FW > > > > > >> Technologist) <abner.ch...@hpe.com> > > > > > >> Cc: Alexander Graf <ag...@csgraf.de>; Heinrich Schuchardt > > > > > >> <xypron.g...@gmx.de>; U-Boot Mailing List <u-boot@lists.denx.de>; > > > > > >> Atish Patra <atish.pa...@wdc.com>; l...@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.biesheu...@linaro.org> > > > > > >> wrote: > > > > > >>> > > > > > >>> On Thu, 6 Feb 2020 at 21:06, Atish Patra <ati...@atishpatra.org> > > > > > >>> wrote: > > > > > >>>> > > > > > >>>> On Thu, Feb 6, 2020 at 12:10 PM Alexander Graf <ag...@csgraf.de> > > > > > wrote: > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> On 06.02.20 19:28, Atish Patra wrote: > > > > > >>>>>> On Tue, Feb 4, 2020 at 11:43 PM Ard Biesheuvel > > > > > >>>>>> <ard.biesheu...@linaro.org> wrote: > > > > > >>>>>>> On Wed, 5 Feb 2020 at 05:53, Heinrich Schuchardt > > > > > >> <xypron.g...@gmx.de> wrote: > > > > > >>>>>>>> RISC-V booting currently is based on a per boot stage lottery > > > > > >>>>>>>> to determine the active CPU. The Hart State Management (HSM) > > > > > >>>>>>>> SBI extension replaces this lottery by using a dedicated > > > > > >>>>>>>> primary > > > > > >> CPU. > > > > > >>>>>>>> > > > > > >>>>>>>> Cf. Hart State Management Extension, Extension ID: 0x48534D > > > > > >>>>>>>> (HSM) > > > > > >>>>>>>> https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.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.g...@gmx.de> > > > > > >>>>>>>> Reviewed-by: Atish Patra <atish.pa...@wdc.com> > > > > > >>>>>>> OK, so I have a couple of questions: > > > > > >>>>>>> > > > > > >>>>>>> - does RISC-V use device tree? if so, why are you not passing > > > > > >>>>>>> the 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 ? -- Regards, Atish