Re: Question regarding "boot-hartid" DT node

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

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?

Thanks
Sunil

> In that case, where should we document it ? UEFI spec or RISC-V platform spec ?
> 
> >
> > Regards,
> > Anup
> >
> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > >
> > > > My only concern with the RISC-V EFI protocol is that Ard suggested it
> > > > doesn't need a modification in UEFI spec.
> > > > Where should we document it in this case ? We can't document it in
> > > > Linux or EBBR.
> > > > Because, this is a protocol that server systems and other non-Linux OS
> > > > will also use.
> > > > We can define it in the RISC-V platform spec. But that's not the usual
> > > > place where somebody will look for the definition of such protocol.
> > > >
> > > > Wouldn't it be better to standardize it in UEFI spec ? The UEFI spec
> > > > already has ARCH specific protocols/config tables.
> > > >
> > > >>>
> > > >>> If GRUB loads a devicetree it will anyway have to call into the firmware
> > > >>> for fixups. These will include adding the boot-hartid.
> > > >>>
> > > >>>>
> > > >>>>> @Heinrich Schuchardt
> > > >>>>> I vaguely remember you proposed something similar when we discussed
> > > >>>>> this first during FOSDEM.
> > > >>>>> I can't recall why it was abandoned in favor of the DT approach which
> > > >>>>> works. But,
> > > >>>>> it is not an ideal solution considering RISC-V ACPI support is already
> > > >>>>> on the way.
> > > >>>>>
> > > >>>>> Do you have a link to the older thread where this thing was discussed ?
> > > >>>
> > > >>> Unfortunately I cannot find anything.
> > > >>
> > > >> I assume Atish referred to this thread:
> > > >>
> > > >> https://patchwork.ozlabs.org/project/uboot/patch/20200205055334.4072-1-xypron.glpk@gmx.de/
> > > >>
> > > >> Best regards
> > > >>
> > > >> Heinrich
> > > >>
> > > >> _______________________________________________
> > > >> linux-riscv mailing list
> > > >> linux-riscv@lists.infradead.org
> > > >> http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > >
> > > >
> > > >
> 
> 
> 
> -- 
> Regards,
> Atish

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