Re: Question regarding "boot-hartid" DT node

[Sunil V L <sunilvl@ventanamicro.com>]

On Sun, Dec 05, 2021 at 03:54:23PM +0000, Jessica Clarke wrote:
> On 5 Dec 2021, at 13:39, Sunil V L <sunilvl@ventanamicro.com> wrote:
> > 
> > On Sat, Dec 04, 2021 at 10:34:28AM -0800, Atish Patra wrote:
> >> On Fri, Dec 3, 2021 at 8:24 PM Anup Patel <anup@brainfault.org> wrote:
> >>> 
> >>> On Sat, Dec 4, 2021 at 7:17 AM Heinrich Schuchardt
> >>> <heinrich.schuchardt@canonical.com> wrote:
> >>>> 
> >>>> 
> >>>> 
> >>>> On 12/4/21 01:44, Atish Patra wrote:
> >>>>> On Fri, Dec 3, 2021 at 10:45 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>>>> 
> >>>>>> On 12/3/21 11:53 AM, Heinrich Schuchardt wrote:
> >>>>>>> On 12/3/21 11:13, Ard Biesheuvel wrote:
> >>>>>>>> On Thu, 2 Dec 2021 at 20:29, Atish Patra <atishp@atishpatra.org> wrote:
> >>>>>>>>> 
> >>>>>>>>> On Thu, Dec 2, 2021 at 9:05 AM Heinrich Schuchardt
> >>>>>>>>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>>>>>>> 
> >>>>>>>>>> On 12/2/21 17:58, Ard Biesheuvel wrote:
> >>>>>>>>>>> On Thu, 2 Dec 2021 at 17:53, Heinrich Schuchardt
> >>>>>>>>>>> <heinrich.schuchardt@canonical.com> wrote:
> >>>>>>>>>>>> 
> >>>>>>>>>>>> On 12/2/21 17:20, Ard Biesheuvel wrote:
> >>>>>>>>>>>>> On Thu, 2 Dec 2021 at 16:05, Sunil V L <sunilvl@ventanamicro.com>
> >>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>> 
> >>>>>>>>>>>>>> Hi All,
> >>>>>>>>>>>>>>       I am starting this thread to discuss about the
> >>>>>>>>>>>>>> "boot-hartid" DT node
> >>>>>>>>>>>>>>       that is being used in RISC-V Linux EFI stub.
> >>>>>>>>>>>>>> 
> >>>>>>>>>>>>>>       As you know, the boot Hart ID is passed in a0 register to
> >>>>>>>>>>>>>> the kernel
> >>>>>>>>>>>>>>       and hence there is actually no need to pass it via DT.
> >>>>>>>>>>>>>> However, since
> >>>>>>>>>>>>>>       EFI stub follows EFI application calling conventions, it
> >>>>>>>>>>>>>> needs to
> >>>>>>>>>>>>>>       know the boot Hart ID so that it can pass it to the proper
> >>>>>>>>>>>>>> kernel via
> >>>>>>>>>>>>>>       a0. For this issue, the solution was to add
> >>>>>>>>>>>>>> "/chosen/boot-hartid" in
> >>>>>>>>>>>>>>       DT. Both EDK2 and u-boot append this node in DT.
> >>>>>>>>>>>>>> 
> >>>>>>>>>>>>> 
> >>>>>>>>>>>>> I think this was a mistake tbh
> >>>>>>>>>>>>> 
> >>>>>>>>>>>>>>       But above approach causes issue for ACPI since ACPI
> >>>>>>>>>>>>>> initialization
> >>>>>>>>>>>>>>       happens late in the proper kernel. Same is true even if we
> >>>>>>>>>>>>>> pass this
> >>>>>>>>>>>>>>       information via SMBIOS.
> >>>>>>>>>>>>>> 
> >>>>>>>>>>>>> 
> >>>>>>>>>>>>> It would be better to define a RISCV specific EFI protocol that the
> >>>>>>>>>>>>> stub can call to discover the boot-hartid value. That wat, it can
> >>>>>>>>>>>>> pass
> >>>>>>>>>>>>> it directly, without having to rely on firmware tables.
> >>>>>>>>>>>> 
> >>>>>>>>>>>> As discovering the current process' hartid is not a UEFI specific
> >>>>>>>>>>>> task
> >>>>>>>>>>>> SBI would be a better fit.
> >>>>>>>>>>>> 
> >>>>>>>>>>> 
> >>>>>>>>>>> I disagree. The OS<->loader firmware interface is UEFI not SBI. So if
> >>>>>>>>>>> the EFI stub needs to ask the firmware which boot-hartid it should
> >>>>>>>>>>> pass in A0, it should use a EFI protocol and nothing else.
> >>>>>>>>>>> 
> >>>>>>>>>>> Whether or not the loader/firmware *implements* this EFI protocol by
> >>>>>>>>>>> calling into SBI is a different matter (and likely the best choice).
> >>>>>>>>>>> But that does not mean the EFI stub should make SBI calls directly.
> >>>>>>>>>>> 
> >>>>>>>>>> 
> >>>>>>>>>> The EFI stub does not need the boot-hartid. It is the main Linux kernel
> >>>>>>>>>> that does. And that kernel already implements SBI calls.
> >>>>>>>>>> 
> >>>>>>>>>> The main kernel could just try to read CSR mhartid which fails in
> >>>>>>>>>> S-mode
> >>>>>>>>>> and the SBI could emulate it.
> >>>>>>>>> 
> >>>>>>>>> New SBI extension should be added only if there is no other way to
> >>>>>>>>> solve a generic
> >>>>>>> 
> >>>>>>> I am not sure this feature would be implemented as SBI extension or as a
> >>>>>>> CSR emulation. Cf. sbi_emulate_csr_read(). But anyway it would require
> >>>>>>> an update of the SBI specification.
> >>>>>>> 
> >>>>>>>>> problem. The boot hartid issue is very specific to efi booting only.
> >>>>>>>>> Any system that doesn't require
> >>>>>>> 
> >>>>>>> The boot hartid is not EFI related at all. A firmware running single
> >>>>>>> threaded does not need this information.
> >>>>>>> 
> >>>>>>> Information about the boot hartid is a general OS need.
> >>>>>>> 
> >>>>>>> I am wondering why S-mode software should not have a generic means to
> >>>>>>> find out on which hart it is currently running.
> >>>>>>> 
> >>>>>>>>> EFI booting won't need it. Even for EFI booting, we have other
> >>>>>>>>> approaches already proposed
> >>>>>>>>> to solve it. That's why, SBI extension should be the last resort
> >>>>>>>>> rather than first.
> >>>>>>>>> 
> >>>>>>>>> I think an RISC-V specific EFI protocol as suggested by Ard should
> >>>>>>>>> work for all the cases.
> >>>>>>>>> Is there a case where you think it may not work ? U-Boot & EDK2
> >>>>>>>>> already stores the boot hartid.
> >>>>>>>>> They just implement that protocol and pass the hartid to the caller.
> >>>>>>>>> We do need to support it in the grub though.
> >>>>>>>>> 
> >>>>>>>> 
> >>>>>>>> Why would GRUB care about this? The EFI stub will call into the
> >>>>>>>> underlying firmware to invoke the protocol, GRUB is just a loader with
> >>>>>>>> a fancy menu that allows you to select which image to load, no?
> >>>>>>> 
> >>>>>>> This is a related discussion:
> >>>>>>> 
> >>>>>>> https://github.com/tekkamanninja/grub/commit/be9d4f1863a1fcb1cbbd2f867309457fade8be73#commitcomment-60851029
> >>>>>>> 
> >>>>> 
> >>>>> Yes!! Thanks for refreshing the memory. It seems after 2 years, we are
> >>>>> still debating the same topic :).
> >>>>> Let me summarize the thread. There are multiple ways for EFI stub code
> >>>>> to retrieve the boot hartid.
> >>>>> 
> >>>>> 1. EFI variables - This is what Henerich proposed last time. Ard
> >>>>> suggested that EFI configuration tables are better candidates than EFI
> >>>>> variables.
> >>>>> 2. DT modification - This was preferred over the configuration table
> >>>>> at that time given because RISC-V was DT only at that time.
> >>>>>                                  We already had all the infrastructure
> >>>>> around DT. Thus, DT seemed to be a natural choice then.
> >>>>>                                  It works now for existing setup
> >>>>> however, the DT approach will not work for systems with ACPI support.
> >>>>>                                  Adding a similar entry in ACPI tables
> >>>>> is possible but adding ACPI support in EFI stub is not trivial.
> >>>>> 3. SMBIOS - Only for platforms with SMBIOS support. SMBIOS is not
> >>>>> mandatory and adding SMBIOS support in EFI stub is not trivial.
> >>>>> 4. SBI         -  As I mentioned before, this is an EFI specific
> >>>>> problem because EFI stub doesn't know what the boot hartid is. Thus,
> >>>>> it should be solved
> >>>>>                       in an EFI specific way. An SBI extension for
> >>>>> such features may not be acceptable as the non-EFI booting method
> >>>>> works fine without the SBI extension.
> >>>>> 5. RISC-V specific EFI configuration table or protocol: Ard suggested
> >>>>> EFI configuration table last time. Earlier in this thread, EFI
> >>>>> protocol was suggested.
> >>>>>                       My personal preference has always been one of
> >>>>> these as it solves the problem for all EFI booting methods
> >>>>>                       for platforms-os
> >>>>> combination(DT/ACPI-Linux/FreeBSD) in an EFI specific way.
> >>>>> 
> >>>>> @Heinrich: Do you see any issue with the EFI configuration table or
> >>>>> protocol to retrieve the boot hartid?
> >>>> 
> >>>> There is nothing technical stopping us from implementing either option.
> >>>> 
> >>>> We could simply reuse the EFI Device Tree Fixup Protocol
> >>>> (https://github.com/U-Boot-EFI/EFI_DT_FIXUP_PROTOCOL) implemented in
> >>>> U-Boot and already used by systemd-boot. Pass a devicetree (which may be
> >>>> empty) to the Fixup() method and it will add the /chosen node with the
> >>>> boot-hartid parameter.
> >>>> 
> >>>> The EFI stub anyway creates a new device-tree to pass the memory map to
> >>>> the kernel in the ACPI case (function update_fdt()). Calling the EFI
> >>>> Device Tree Fixup Protocol could be easily added.
> >> 
> >> Thanks. Yes. We can solve the current problem for EFI stub in Linux.
> >> 
> >>> 
> >>> Are you suggesting that DTB (skeletal or full-blown) will always be there when
> >>> booting the kernel as an EFI application ? If yes then we are
> >>> indirectly mandating
> >>> skeletal DTB for UEFI+ACPI system.
> >> 
> >> Yes. EFI Stub tries to find a fdt from the command line (not a
> >> preferred method) or EFI configuration table[1]
> >> (currently used for RISC-V systems). If it can't find a device tree,
> >> it generates one [2]
> >> 
> >> [1] https://elixir.bootlin.com/linux/v5.16-rc3/source/drivers/firmware/efi/libstub/efi-stub.c#L231
> >> [2] https://elixir.bootlin.com/linux/v5.16-rc3/source/drivers/firmware/efi/libstub/fdt.c#L58
> >> 
> >> However, we may still need to define the RISC-V EFI protocol to
> >> support ACPI for other OS (FreeBSD) which doesn't have
> >> a stub like loader that uses DT.
> >> 
> > If RISC-V mandates that every OS (including FreeBSD)  should follow the
> > booting protocol that a0 should be used to pass the boot hartid, then
> > only Linux EFI stub becomes a special case which can reuse the existing
> > DT based EFI protocol (mentioned by Heinrich), right?
> 
> Absolutely not. What you are saying is “let’s forbid anyone from using
> UEFI”. This “booting protocol” thing is a RISC-V-specific low-level
> interface that is incompatible with UEFI; in it, every architecture,
> including RISC-V, passes exactly two arguments to the application, the
> handle to the image and a pointer to the system table, and puts the
> return address in the usual place. Linux’s EFI stub is not special;
> *any* EFI application, be it your EFI stub, FreeBSD’s bootloader,
> grub-efi, systemd-boot, OpenBSD’s bootloader, Haiku’s bootloader,
> whatever, follows the UEFI spec and so does not, and never will, follow
> the RISC-V booting protocol.
> 
Yes, understood. Thanks!.

> I wish everyone would stop talking about this “RISC-V booting
> protocol”. It’s low-level firmware details that, due to not having a
> mature firmware story at the time OSes were brought up, leaked up into
> the OS booting process. In FreeBSD the only time it gets used is if the
> kernel is booted directly from BBL/OpenSBI; if instead you boot the
> bootloader via UEFI then that code is completely bypassed (and we make
> the legacy direct boot method look like the bootloader method by
> creating a “fake” bootloader metadata array, rather than the other way
> round, which would be backwards and severely limiting). It’s not a good
> protocol, OSes shouldn’t be using it, and it should fade into obscurity
> outside of the M-mode firmware<->S-mode firmware interface. It’s just
> not relevant to an OS. What is relevant is that there is critical
> information in the firmware that the OS can’t get at if booting via the
> industry-standard portable rich firmware interface, and that
> information needs to be provided some way or another via a means
> compatible with the specification. I like the SBI “get current hartid”
> extension approach best, it’s the most flexible and there’s no harm in
> having it exist in a crummy non-UEFI situation. It’s trivial to
> implement, it’s trivial to use, it provides the interface that *should*
> have existed in the ISA just like how mpidr_el1 is a thing on AArch64,
> and it provides a way for even non-UEFI code to not have to care about
> a0 being hartid (outside of perhaps the hart lottery in the entry point
> if you need to support pre-HSM SBIs), it can just throw it away and ask
> for it later when it needs it, just like if booting via UEFI. You could
> also make it a UEFI protocol, but that’s more annoying to deal with,
> you now have two different ways to get the same information depending
> on how you were booted. You could also restrict it to only being “get
> the boot hartid”, but why, what does that achieve? It doesn’t really
> make it any simpler to implement, it’s just more restrictive for the
> sake of being minimal, even if it is sufficient. But at the end of the
> day all of them do work. So, please, can we stop wasting our lives on
> this and just do *something* rather than going round and round in
> circles and forgetting all the details in the process?
> 
> As for the “provide a dummy FDT to the proposed fixup protocol”
> proposal, no, that’s clearly not going to fly with an ACPI-only OS that
> doesn’t have, and never will include, a device tree parser. If your
> ACPI system needs a device tree then it’s not ACPI-only any more.
> 
Completely agree.

I can draft the EFI protocol but I also feel "SBI interface to get
current hartid" is better. Considering that the software ecosystem is
expanding for risc-v, this will cater to any future needs. I am
concerned that this EFI protocol will be of no use if SBI implements
"get current hart id" in future.

Thanks
Sunil

> Jess
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv