Re: Question regarding "boot-hartid" DT node

[Heinrich Schuchardt <heinrich.schuchardt@canonical.com>]

On 12/4/21 19:34, 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.
> 
> In that case, where should we document it ? UEFI spec or RISC-V platform spec ?

Defining EFI protocols outside the UEFI spec has precedents, e.g. the 
EFI_TCG2_PROTOCOL is defined in a specification hosted by the Trusted 
Computing Group.

The UEFI Forum prefers an implemention first approach. We should 
demonstrate with a patched EDK II or U-Boot and a patched Linux that 
what we define is working before creating a change request.

Let's start with a draft protocol specification on Github and then 
develop the necessary patches.

Best regards

Heinrich

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

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