Re: [External] Re: [PATCH] firmware: added a firmware information passing method FFI

[Ard Biesheuvel]

(cc RISC-V maintainers and mailing list)

On Mon, 26 Jun 2023 at 12:20, 运辉崔 <cuiyunhui@xxxxxxxxxxxxx> wrote:
>
> Hi Ard, Mark,
>
> On Mon, Jun 26, 2023 at 4:23 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> > DT support for SMBIOS can live in generic code, but the binding has to
> > be sane. As I suggested before, it probably makes sense to supplant
> > the entrypoint rather than just carry its address - this means a 'reg'
> > property with base and size to describe the physical region, and at
> > least major/minor/docrev fields to describe the version.
>
> Regarding dts node binding, our current definition is as follows:
> /dts
> {
> ...
> cfgtables {
> acpi_phy_ptr = 0000000000000000; //u64
> smbios_phy_ptr = 0000000000000000; //u64
> ...
> }
> ...
> }
>
> x86 only gave a root_pointer entry address
> u64 x86_default_get_root_pointer(void)
> {
>        return boot_params.acpi_rsdp_addr;
> }
>
> Regarding the naming of the binding above, Mark,  do you have any suggestions?
>

I will defer to Mark or other DT experts to help decide on the naming
and general shape of these.

However, as I have indicated twice now, it would be better to describe
the SMBIOS structured data directly, instead of passing the physical
address of one of the existing entry points. This avoids the need for
mapping and reserving additional pages that don't carry any relevant
information.

So the node in question should have at least (base, size) and the
major, minor and docrev version fields.

>
> > For the ACPI side, you should just implement
> > acpi_arch_get_root_pointer() under arch/riscv, and wire it up in
> > whichever way you want. But please check with the RISC-V maintainers
> > if they are up for this, and whether they want to see this mechanism
> > contributed to one of the pertinent specifications.
>
> You suggest putting SMBIOS in general code instead of ACPI, why?

SMBIOS is a separate set of firmware tables that have little
significance to the kernel itself, and describing it via DT makes
sense.

ACPI serves a similar purpose as DT, and so having both at the same
time results in a maintenance burden, where the arch code is forced to
reason about whether they are consistent with each other, and if not,
which description has precedence.

> From the perspective of firmware information passing, they are a class.
>
> SMBIOS and ACPI are not related to ARCH, nor is DTS to obtain firmware
> information,
>
> Why do you have to put part of the ACPI code under arch/risc-v/?

Yes. And I don't think it should be using this FFI scheme either.

If the firmware uses DT as a conduit to deliver the ACPI system
description to the OS, it is probably better to pass this via the
/chosen node as a special boot argument.

> The scope of the previous discussion was limited to RISC-V because of
> historical reasons such as the binding with EFI on ARM64. We will only
> enable this function on RISC-V in subsequent patches.
>
> The realization of the FFI scheme itself is irrelevant to the arch.
>

I don't think we need a FFI scheme or framework for this.