Re: Question regarding "boot-hartid" DT node

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

On Sun, Dec 05, 2021 at 02:21:34PM -0800, Atish Patra wrote:
> On Sun, Dec 5, 2021 at 8:38 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> > 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.
> >
> 
> There won’t be an SBI extension for “get current hart id” for the reasons
> explained by earlier. So EFI protocol should be the way forward to solve
> this issue.
> 
Thank you Atish for that confirmation. Let me draft the EFI protocol
with a PoC and will request for review. We can decide where to host it
(RVI or UEFI spec) later.

Thank you all for your inputs. This was very helpful.

Thanks
Sunil
> 
> > Thanks
> > Sunil
> >
> > > Jess
> > >
> >
> -- 
> Regards,
> Atish

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