2023-07-05 11:59:26

by Yunhui Cui

[permalink] [raw]
Subject: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

Here's version 3 of patch series.

V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
consensus with the Maintainers.
Please refer to:
https://patches.linaro.org/project/linux-acpi/patch/[email protected]/

Changes v1->v2:
Adjusted the code structure, put the ACPI part under the RISC-V architecture,
and put the general part of obtaining SMBIOS entry through FFI
under driver/firmware/.
Please refer to:
https://lore.kernel.org/lkml/20230703-71f67eb66a037f5c0fb825c6@orel/T/

Changes v2->v3:
According to the suggestions of maintainers, the code has been modified as follows:
1. Modified the commit log.
2. Added description of "ffitbl" subnod in dt-bindings.
3. Add stub function to the function
4. arch/riscv/ and driver/firmware/ use CONFIG_FDT_FW_INTERFACE to control
5. Modified the ffi_smbios_root_pointer() function logic and printing
etc.

Yunhui Cui (4):
riscv: obtain ACPI RSDP from devicetree
firmware: introduce FFI for SMBIOS entry
riscv: obtain SMBIOS entry from FFI
dt-bindings: firmware: Document ffitbl binding

.../devicetree/bindings/firmware/ffitbl.txt | 27 ++++++
MAINTAINERS | 13 +++
arch/riscv/include/asm/acpi.h | 9 ++
arch/riscv/include/asm/ffi.h | 14 +++
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/ffi.c | 40 ++++++++
arch/riscv/kernel/setup.c | 2 +
drivers/firmware/Kconfig | 11 +++
drivers/firmware/Makefile | 1 +
drivers/firmware/dmi_scan.c | 97 +++++++++++--------
drivers/firmware/ffi.c | 42 ++++++++
include/linux/ffi.h | 29 ++++++
12 files changed, 246 insertions(+), 40 deletions(-)
create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
create mode 100644 arch/riscv/include/asm/ffi.h
create mode 100644 arch/riscv/kernel/ffi.c
create mode 100644 drivers/firmware/ffi.c
create mode 100644 include/linux/ffi.h

--
2.20.1



2023-07-05 12:02:03

by Yunhui Cui

[permalink] [raw]
Subject: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

Add the description for ffitbl subnode.

Signed-off-by: Yunhui Cui <[email protected]>
---
.../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt

diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
new file mode 100644
index 000000000000..c42368626199
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
@@ -0,0 +1,27 @@
+FFI(FDT FIRMWARE INTERFACE) driver
+
+Required properties:
+ - entry : acpi or smbios root pointer, u64
+ - reg : acpi or smbios version, u32
+
+Some bootloaders, such as Coreboot do not support EFI,
+only devicetree and some arches do not have a reserved
+address segment. Add "ffitbl" subnode to obtain ACPI RSDP
+and SMBIOS entry.
+This feature is known as FDT Firmware Interface (FFI).
+
+Example:
+ ffitbl {
+
+ smbios {
+ entry = "";
+ reg = < 0x03 >;
+
+ }
+ acpi {
+ entry = "";
+ reg = < 0x06 >;
+
+ }
+ }
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 9b886ef36587..008257e55062 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7874,6 +7874,7 @@ F: include/linux/efi*.h
FDT FIRMWARE INTERFACE (FFI)
M: Yunhui Cui [email protected]
S: Maintained
+F: Documentation/devicetree/bindings/firmware/ffitbl.txt
F: drivers/firmware/ffi.c
F: include/linux/ffi.h

--
2.20.1


2023-07-05 14:21:36

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

On Wed, 05 Jul 2023 04:42:47 PDT (-0700), [email protected] wrote:
> Here's version 3 of patch series.
>
> V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> consensus with the Maintainers.
> Please refer to:
> https://patches.linaro.org/project/linux-acpi/patch/[email protected]/

From looking at that thread it seems that the consensus is this is a bad
idea? Sorry if I'm just missing something...

> Changes v1->v2:
> Adjusted the code structure, put the ACPI part under the RISC-V architecture,
> and put the general part of obtaining SMBIOS entry through FFI
> under driver/firmware/.
> Please refer to:
> https://lore.kernel.org/lkml/20230703-71f67eb66a037f5c0fb825c6@orel/T/
>
> Changes v2->v3:
> According to the suggestions of maintainers, the code has been modified as follows:
> 1. Modified the commit log.
> 2. Added description of "ffitbl" subnod in dt-bindings.
> 3. Add stub function to the function
> 4. arch/riscv/ and driver/firmware/ use CONFIG_FDT_FW_INTERFACE to control
> 5. Modified the ffi_smbios_root_pointer() function logic and printing
> etc.
>
> Yunhui Cui (4):
> riscv: obtain ACPI RSDP from devicetree
> firmware: introduce FFI for SMBIOS entry
> riscv: obtain SMBIOS entry from FFI
> dt-bindings: firmware: Document ffitbl binding
>
> .../devicetree/bindings/firmware/ffitbl.txt | 27 ++++++
> MAINTAINERS | 13 +++
> arch/riscv/include/asm/acpi.h | 9 ++
> arch/riscv/include/asm/ffi.h | 14 +++
> arch/riscv/kernel/Makefile | 1 +
> arch/riscv/kernel/ffi.c | 40 ++++++++
> arch/riscv/kernel/setup.c | 2 +
> drivers/firmware/Kconfig | 11 +++
> drivers/firmware/Makefile | 1 +
> drivers/firmware/dmi_scan.c | 97 +++++++++++--------
> drivers/firmware/ffi.c | 42 ++++++++
> include/linux/ffi.h | 29 ++++++
> 12 files changed, 246 insertions(+), 40 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> create mode 100644 arch/riscv/include/asm/ffi.h
> create mode 100644 arch/riscv/kernel/ffi.c
> create mode 100644 drivers/firmware/ffi.c
> create mode 100644 include/linux/ffi.h

2023-07-05 15:23:39

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

Hey,

On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> Add the description for ffitbl subnode.
>
> Signed-off-by: Yunhui Cui <[email protected]>
> ---
> .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 28 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>
> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> new file mode 100644
> index 000000000000..c42368626199
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt

Firstly, new dt-bindings need to be done in yaml, not in text form.
Secondly, you didn't re-run get_maintainer.pl after adding this binding,
so you have not CCed any of the other dt-binding maintainers nor the
devicetree mailing list.

> @@ -0,0 +1,27 @@

> +FFI(FDT FIRMWARE INTERFACE) driver
> +
> +Required properties:
> + - entry : acpi or smbios root pointer, u64
> + - reg : acpi or smbios version, u32

Please go look at any other dt-binding (or the example schema) as to how
these properties should be used. A "reg" certainly should not be being
used to store the revision...

Cheers,
Conor.

> +
> +Some bootloaders, such as Coreboot do not support EFI,
> +only devicetree and some arches do not have a reserved
> +address segment. Add "ffitbl" subnode to obtain ACPI RSDP
> +and SMBIOS entry.
> +This feature is known as FDT Firmware Interface (FFI).
> +
> +Example:
> + ffitbl {
> +
> + smbios {
> + entry = "";
> + reg = < 0x03 >;
> +
> + }
> + acpi {
> + entry = "";
> + reg = < 0x06 >;
> +
> + }
> + }
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9b886ef36587..008257e55062 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7874,6 +7874,7 @@ F: include/linux/efi*.h
> FDT FIRMWARE INTERFACE (FFI)
> M: Yunhui Cui [email protected]
> S: Maintained
> +F: Documentation/devicetree/bindings/firmware/ffitbl.txt
> F: drivers/firmware/ffi.c
> F: include/linux/ffi.h
>
> --
> 2.20.1
>


Attachments:
(No filename) (2.14 kB)
signature.asc (235.00 B)
Download all attachments

2023-07-05 16:20:56

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

Hey,

On Wed, Jul 05, 2023 at 07:17:29AM -0700, Palmer Dabbelt wrote:
> On Wed, 05 Jul 2023 04:42:47 PDT (-0700), [email protected] wrote:
> > Here's version 3 of patch series.
> >
> > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > consensus with the Maintainers.
> > Please refer to:
> > https://patches.linaro.org/project/linux-acpi/patch/[email protected]/
>
> From looking at that thread it seems that the consensus is this is a bad
> idea? Sorry if I'm just missing something...

"consensus" meaning that Ard told them what he was and was not prepared
to accept in common code, and left the decision on what he was not up to
the RISC-V maintainers.

While this version of the series seems to address some of my general
code comments on the v2 (although I have not yet looked more than skin
deep), there are some outstanding, higher level, questions that were
asked on v2 that do not seem to have been answered satisfactorily yet:

- "So, could you please bring this topic for discussion in on the riscv
tech-brs mailing list (https://lists.riscv.org/g/tech-brs) and get
agreement?" Sunil has asked this as RVI specs have an interest in
cross-os booting contracts. See:
https://lore.kernel.org/linux-riscv/CAEEQ3wnsedWJYEEg8z+3C_HuCca0nD50NGpCdU3scxavrrOucA@mail.gmail.com/

- "I am curious how do you handle EFI memory map dependencies." to
which the answer was "a memory node in DTS can solve it."
I don't see anything in this version of the patchset that actually
reads a DTS node when ACPI is enabled. If I am missing some codepath
that does this, please let point it out. See:
https://lore.kernel.org/linux-riscv/CAEEQ3wnsedWJYEEg8z+3C_HuCca0nD50NGpCdU3scxavrrOucA@mail.gmail.com/

- "I'm not a big fan of adding yet another interface. Have you considered
doing something like [1]?" where [1] was:
https://github.com/tianocore/tianocore.github.io/wiki/UefiPayloadPkg
The response to this question was "This has been discussed many times
with Ard, Please refer to <v1>". I don't see how this answers the
question to be honest & Andrew's follow-up question asking for
clarification went unanswered:
https://lore.kernel.org/linux-riscv/20230703-6ac90a2de15f1017bc0ced74@orel/
Jess, Emil and Bjorn have all also commented about you could load a
small EFI payload from Coreboot. I don't see any responses to those
questions.

运辉崔, please try to address all outstanding comments (and give people
a chance to reply to you) before sending new versions.

Cheers,
Conor.


Attachments:
(No filename) (2.56 kB)
signature.asc (235.00 B)
Download all attachments

2023-07-06 02:16:02

by Yunhui Cui

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

Hi Palmer,

On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <[email protected]> wrote:
>
> On Wed, 05 Jul 2023 04:42:47 PDT (-0700), [email protected] wrote:
> > Here's version 3 of patch series.
> >
> > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > consensus with the Maintainers.
> > Please refer to:
> > https://patches.linaro.org/project/linux-acpi/patch/[email protected]/
>
> From looking at that thread it seems that the consensus is this is a bad
> idea? Sorry if I'm just missing something...
>

First of all, Coreboot does not support EFI, Ron has expressed, as follows:
"I am wondering if we can focus on risc-v here, and not drag in ARM,
b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in 2013
;-) and it's clear we don't want to redo it.
In general, in my world, because of the many problems that come with
UEFI (security, code quality, performance), we'd like to avoid
requiring a dependency on UEFI just to get ACPI on RISC-V. It also
seems, from other discussions I'm having, that there is some belief
that ACPI will be wanted on RISC-V. It would be nice to separate those
pieces on RISC-V; certainly they were separate for a very long time in
the x86 world (we had ACPI+SMM on coreboot laptops without UEFI for
example)."

Then, a consensus was reached with Ard, that FFI can be applied to RISC-V.

Please look at this patch again: [PATCH v3 1/4] riscv: obtain ACPI
RSDP from devicetree
Why do you think it is a bad idea?


> > Changes v1->v2:
> > Adjusted the code structure, put the ACPI part under the RISC-V architecture,
> > and put the general part of obtaining SMBIOS entry through FFI
> > under driver/firmware/.
> > Please refer to:
> > https://lore.kernel.org/lkml/20230703-71f67eb66a037f5c0fb825c6@orel/T/
> >
> > Changes v2->v3:
> > According to the suggestions of maintainers, the code has been modified as follows:
> > 1. Modified the commit log.
> > 2. Added description of "ffitbl" subnod in dt-bindings.
> > 3. Add stub function to the function
> > 4. arch/riscv/ and driver/firmware/ use CONFIG_FDT_FW_INTERFACE to control
> > 5. Modified the ffi_smbios_root_pointer() function logic and printing
> > etc.
> >
> > Yunhui Cui (4):
> > riscv: obtain ACPI RSDP from devicetree
> > firmware: introduce FFI for SMBIOS entry
> > riscv: obtain SMBIOS entry from FFI
> > dt-bindings: firmware: Document ffitbl binding
> >
> > .../devicetree/bindings/firmware/ffitbl.txt | 27 ++++++
> > MAINTAINERS | 13 +++
> > arch/riscv/include/asm/acpi.h | 9 ++
> > arch/riscv/include/asm/ffi.h | 14 +++
> > arch/riscv/kernel/Makefile | 1 +
> > arch/riscv/kernel/ffi.c | 40 ++++++++
> > arch/riscv/kernel/setup.c | 2 +
> > drivers/firmware/Kconfig | 11 +++
> > drivers/firmware/Makefile | 1 +
> > drivers/firmware/dmi_scan.c | 97 +++++++++++--------
> > drivers/firmware/ffi.c | 42 ++++++++
> > include/linux/ffi.h | 29 ++++++
> > 12 files changed, 246 insertions(+), 40 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> > create mode 100644 arch/riscv/include/asm/ffi.h
> > create mode 100644 arch/riscv/kernel/ffi.c
> > create mode 100644 drivers/firmware/ffi.c
> > create mode 100644 include/linux/ffi.h

Thanks,
Yunhui

2023-07-06 04:53:13

by Yunhui Cui

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

Hi Conor,

Added dts Maintainers,

On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <[email protected]> wrote:
>
> Hey,
>
> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> > Add the description for ffitbl subnode.
> >
> > Signed-off-by: Yunhui Cui <[email protected]>
> > ---
> > .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
> > MAINTAINERS | 1 +
> > 2 files changed, 28 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> > new file mode 100644
> > index 000000000000..c42368626199
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>
> Firstly, new dt-bindings need to be done in yaml, not in text form.
> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> so you have not CCed any of the other dt-binding maintainers nor the
> devicetree mailing list.

Re-run get_maintainer.pl and added maintainers into the maillist.
emm.. There is some *txt in
Documentation/devicetree/bindings/firmware/, isn't it?

>
> > @@ -0,0 +1,27 @@
>
> > +FFI(FDT FIRMWARE INTERFACE) driver
> > +
> > +Required properties:
> > + - entry : acpi or smbios root pointer, u64
> > + - reg : acpi or smbios version, u32
>
> Please go look at any other dt-binding (or the example schema) as to how
> these properties should be used. A "reg" certainly should not be being
> used to store the revision...

Okay, If so,I'll add a property "version" into the dts instead of
"reg", just like, WDYT?
ffitbl {
smbios {
entry = "";
version = < 0x02 >;
}
acpi {
entry = "";
version = < 0x06 >;
}
}

>
> Cheers,
> Conor.
>
> > +
> > +Some bootloaders, such as Coreboot do not support EFI,
> > +only devicetree and some arches do not have a reserved
> > +address segment. Add "ffitbl" subnode to obtain ACPI RSDP
> > +and SMBIOS entry.
> > +This feature is known as FDT Firmware Interface (FFI).
> > +
> > +Example:
> > + ffitbl {
> > +
> > + smbios {
> > + entry = "";
> > + reg = < 0x03 >;
> > +
> > + }
> > + acpi {
> > + entry = "";
> > + reg = < 0x06 >;
> > +
> > + }
> > + }
> > +
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 9b886ef36587..008257e55062 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7874,6 +7874,7 @@ F: include/linux/efi*.h
> > FDT FIRMWARE INTERFACE (FFI)
> > M: Yunhui Cui [email protected]
> > S: Maintained
> > +F: Documentation/devicetree/bindings/firmware/ffitbl.txt
> > F: drivers/firmware/ffi.c
> > F: include/linux/ffi.h
> >
> > --
> > 2.20.1
> >

Thanks,
Yunhui

2023-07-06 06:11:30

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

On 06/07/2023 05:43, 运辉崔 wrote:
> Hi Conor,
>
> Added dts Maintainers,
>
> On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <[email protected]> wrote:
>>
>> Hey,
>>
>> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
>>> Add the description for ffitbl subnode.
>>>
>>> Signed-off-by: Yunhui Cui <[email protected]>
>>> ---
>>> .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
>>> MAINTAINERS | 1 +
>>> 2 files changed, 28 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>> new file mode 100644
>>> index 000000000000..c42368626199
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>
>> Firstly, new dt-bindings need to be done in yaml, not in text form.
>> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
>> so you have not CCed any of the other dt-binding maintainers nor the
>> devicetree mailing list.
>
> Re-run get_maintainer.pl and added maintainers into the maillist.


This does not work like this.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least DT list (maybe more), so this won't be tested by our
tools. Performing review on untested code might be a waste of time, thus
I will skip this patch entirely till you follow the process allowing the
patch to be tested.

Please kindly resend and include all necessary To/Cc entries.

> emm.. There is some *txt in
> Documentation/devicetree/bindings/firmware/, isn't it?

And what about it? Do you claim they were added recently?

Best regards,
Krzysztof


2023-07-06 06:39:22

by Yunhui Cui

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

Hi Krzysztof,

On Thu, Jul 6, 2023 at 2:01 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 06/07/2023 05:43, 运辉崔 wrote:
> > Hi Conor,
> >
> > Added dts Maintainers,
> >
> > On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <[email protected]> wrote:
> >>
> >> Hey,
> >>
> >> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> >>> Add the description for ffitbl subnode.
> >>>
> >>> Signed-off-by: Yunhui Cui <[email protected]>
> >>> ---
> >>> .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
> >>> MAINTAINERS | 1 +
> >>> 2 files changed, 28 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>> new file mode 100644
> >>> index 000000000000..c42368626199
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>
> >> Firstly, new dt-bindings need to be done in yaml, not in text form.
> >> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> >> so you have not CCed any of the other dt-binding maintainers nor the
> >> devicetree mailing list.
> >
> > Re-run get_maintainer.pl and added maintainers into the maillist.
>
>
> This does not work like this.
>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> You missed at least DT list (maybe more), so this won't be tested by our
> tools. Performing review on untested code might be a waste of time, thus
> I will skip this patch entirely till you follow the process allowing the
> patch to be tested.
>
> Please kindly resend and include all necessary To/Cc entries.

This set of patches is applied on the tag next-20230706, and to
generate the mail list by scripts/get_maintainers.pl on the tag

./scripts/get_maintainer.pl
../riscv/linux/v3-0004-dt-bindings-firmware-Document-ffitbl-binding.patch
Yunhui Cui [email protected] (maintainer:FDT FIRMWARE INTERFACE (FFI))
Rob Herring <[email protected]> (maintainer:OPEN FIRMWARE AND
FLATTENED DEVICE TREE BINDINGS)
Krzysztof Kozlowski <[email protected]>
(maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
Conor Dooley <[email protected]> (maintainer:OPEN FIRMWARE AND
FLATTENED DEVICE TREE BINDINGS)
[email protected] (open list:OPEN FIRMWARE AND FLATTENED
DEVICE TREE BINDINGS)
[email protected] (open list)

What am I missing ?


> > emm.. There is some *txt in
> > Documentation/devicetree/bindings/firmware/, isn't it?
>
> And what about it? Do you claim they were added recently?
>
> Best regards,
> Krzysztof
>

Thanks,
Yunhui

2023-07-06 07:09:25

by Conor Dooley

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

On Thu, Jul 06, 2023 at 11:43:55AM +0800, 运辉崔 wrote:
> On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <[email protected]> wrote:
> > On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> > > Add the description for ffitbl subnode.
> > >
> > > Signed-off-by: Yunhui Cui <[email protected]>
> > > ---
> > > .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
> > > MAINTAINERS | 1 +
> > > 2 files changed, 28 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> > > new file mode 100644
> > > index 000000000000..c42368626199
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >
> > Firstly, new dt-bindings need to be done in yaml, not in text form.
> > Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> > so you have not CCed any of the other dt-binding maintainers nor the
> > devicetree mailing list.
>
> Re-run get_maintainer.pl and added maintainers into the maillist.
> emm.. There is some *txt in
> Documentation/devicetree/bindings/firmware/, isn't it?

There might be, but that's not an excuse for adding _new_ ones, sorry.

> > > +FFI(FDT FIRMWARE INTERFACE) driver
> > > +
> > > +Required properties:
> > > + - entry : acpi or smbios root pointer, u64
> > > + - reg : acpi or smbios version, u32
> >
> > Please go look at any other dt-binding (or the example schema) as to how
> > these properties should be used. A "reg" certainly should not be being
> > used to store the revision...
>
> Okay, If so,I'll add a property "version" into the dts instead of
> "reg", just like, WDYT?
> ffitbl {

Firstly, I'd much rather you spelt this out, like "ffi-table".

> smbios {
> entry = "";

I still don't understand why "entry", which is an address, is being
represented by an empty string.
I also don't really get why you have not used "reg" to describe its
start address and size.

> version = < 0x02 >;

Probably missing a vendor prefix, and the spaces are unusual, but better
than it was, yes.

> }
> acpi {
> entry = "";
> version = < 0x06 >;
> }
> }

Thanks,
Conor.


Attachments:
(No filename) (2.39 kB)
signature.asc (235.00 B)
Download all attachments

2023-07-06 07:10:08

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

On 06/07/2023 08:24, 运辉崔 wrote:
> Hi Krzysztof,
>
> On Thu, Jul 6, 2023 at 2:01 PM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 06/07/2023 05:43, 运辉崔 wrote:
>>> Hi Conor,
>>>
>>> Added dts Maintainers,
>>>
>>> On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <[email protected]> wrote:
>>>>
>>>> Hey,
>>>>
>>>> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
>>>>> Add the description for ffitbl subnode.
>>>>>
>>>>> Signed-off-by: Yunhui Cui <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
>>>>> MAINTAINERS | 1 +
>>>>> 2 files changed, 28 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>>>> new file mode 100644
>>>>> index 000000000000..c42368626199
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
>>>>
>>>> Firstly, new dt-bindings need to be done in yaml, not in text form.
>>>> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
>>>> so you have not CCed any of the other dt-binding maintainers nor the
>>>> devicetree mailing list.
>>>
>>> Re-run get_maintainer.pl and added maintainers into the maillist.
>>
>>
>> This does not work like this.
>>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC. It might happen, that command when run on an older
>> kernel, gives you outdated entries. Therefore please be sure you base
>> your patches on recent Linux kernel.
>>
>> You missed at least DT list (maybe more), so this won't be tested by our
>> tools. Performing review on untested code might be a waste of time, thus
>> I will skip this patch entirely till you follow the process allowing the
>> patch to be tested.
>>
>> Please kindly resend and include all necessary To/Cc entries.
>
> This set of patches is applied on the tag next-20230706, and to
> generate the mail list by scripts/get_maintainers.pl on the tag
>
> ./scripts/get_maintainer.pl
> ../riscv/linux/v3-0004-dt-bindings-firmware-Document-ffitbl-binding.patch
> Yunhui Cui [email protected] (maintainer:FDT FIRMWARE INTERFACE (FFI))
> Rob Herring <[email protected]> (maintainer:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS)
> Krzysztof Kozlowski <[email protected]>
> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> Conor Dooley <[email protected]> (maintainer:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS)
> [email protected] (open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS)
> [email protected] (open list)
>
> What am I missing ?

I did not receive the original patch. Neither did Patchwork. You cannot
just reply to some comment and hope it will fix something. We don't have
this patch simply.



Best regards,
Krzysztof


2023-07-06 07:18:06

by Yunhui Cui

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

Hi Krzysztof,


On Thu, Jul 6, 2023 at 2:41 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 06/07/2023 08:24, 运辉崔 wrote:
> > Hi Krzysztof,
> >
> > On Thu, Jul 6, 2023 at 2:01 PM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 06/07/2023 05:43, 运辉崔 wrote:
> >>> Hi Conor,
> >>>
> >>> Added dts Maintainers,
> >>>
> >>> On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <[email protected]> wrote:
> >>>>
> >>>> Hey,
> >>>>
> >>>> On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> >>>>> Add the description for ffitbl subnode.
> >>>>>
> >>>>> Signed-off-by: Yunhui Cui <[email protected]>
> >>>>> ---
> >>>>> .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
> >>>>> MAINTAINERS | 1 +
> >>>>> 2 files changed, 28 insertions(+)
> >>>>> create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..c42368626199
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> >>>>
> >>>> Firstly, new dt-bindings need to be done in yaml, not in text form.
> >>>> Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> >>>> so you have not CCed any of the other dt-binding maintainers nor the
> >>>> devicetree mailing list.
> >>>
> >>> Re-run get_maintainer.pl and added maintainers into the maillist.
> >>
> >>
> >> This does not work like this.
> >>
> >> Please use scripts/get_maintainers.pl to get a list of necessary people
> >> and lists to CC. It might happen, that command when run on an older
> >> kernel, gives you outdated entries. Therefore please be sure you base
> >> your patches on recent Linux kernel.
> >>
> >> You missed at least DT list (maybe more), so this won't be tested by our
> >> tools. Performing review on untested code might be a waste of time, thus
> >> I will skip this patch entirely till you follow the process allowing the
> >> patch to be tested.
> >>
> >> Please kindly resend and include all necessary To/Cc entries.
> >
> > This set of patches is applied on the tag next-20230706, and to
> > generate the mail list by scripts/get_maintainers.pl on the tag
> >
> > ./scripts/get_maintainer.pl
> > ../riscv/linux/v3-0004-dt-bindings-firmware-Document-ffitbl-binding.patch
> > Yunhui Cui [email protected] (maintainer:FDT FIRMWARE INTERFACE (FFI))
> > Rob Herring <[email protected]> (maintainer:OPEN FIRMWARE AND
> > FLATTENED DEVICE TREE BINDINGS)
> > Krzysztof Kozlowski <[email protected]>
> > (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> > Conor Dooley <[email protected]> (maintainer:OPEN FIRMWARE AND
> > FLATTENED DEVICE TREE BINDINGS)
> > [email protected] (open list:OPEN FIRMWARE AND FLATTENED
> > DEVICE TREE BINDINGS)
> > [email protected] (open list)
> >
> > What am I missing ?
>
> I did not receive the original patch. Neither did Patchwork. You cannot
> just reply to some comment and hope it will fix something. We don't have
> this patch simply.

Oh, I see, you only received the middle mail, and did not receive the patch.
Okay, I'll post it after the next version is updated.

>
> Best regards,
> Krzysztof
>

Thanks,
Yunhui

2023-07-06 09:21:50

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

On Thu, 6 Jul 2023 at 04:04, 运辉崔 <[email protected]> wrote:
>
> Hi Palmer,
>
> On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <[email protected]> wrote:
> >
> > On Wed, 05 Jul 2023 04:42:47 PDT (-0700), [email protected] wrote:
> > > Here's version 3 of patch series.
> > >
> > > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
> > > consensus with the Maintainers.
> > > Please refer to:
> > > https://patches.linaro.org/project/linux-acpi/patch/[email protected]/
> >
> > From looking at that thread it seems that the consensus is this is a bad
> > idea? Sorry if I'm just missing something...
> >
>
> First of all, Coreboot does not support EFI, Ron has expressed, as follows:
> "I am wondering if we can focus on risc-v here, and not drag in ARM,
> b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in 2013
> ;-) and it's clear we don't want to redo it.
> In general, in my world, because of the many problems that come with
> UEFI (security, code quality, performance), we'd like to avoid
> requiring a dependency on UEFI just to get ACPI on RISC-V. It also
> seems, from other discussions I'm having, that there is some belief
> that ACPI will be wanted on RISC-V. It would be nice to separate those
> pieces on RISC-V; certainly they were separate for a very long time in
> the x86 world (we had ACPI+SMM on coreboot laptops without UEFI for
> example)."
>

There appears to be a bit of cargo cult going on here.

I agree that the traditional BIOS vendors did a terrible job pivoting
to (U)EFI when it became a requirement for booting Windows on x86 PCs,
and coreboot did an excellent job providing a retrofit alternative
that was more secure and robust.

However, it makes sense to distinguish between
a) the UEFI specification
b) the UEFI reference implementation (edk2)
c) commercial implementations created by BIOS vendors for x86 PC OEMs
that do not perform any testing beyond booting Windows.

coreboot decided not to implement EFI at all, which on x86 means
booting in a mode that is similar to BIOS boot. Given how the ACPI and
DMTF (for SMBIOS) specifications were already under development when
UEFI was being rolled out on x86, those specs contain provisions
defining how to obtain the ACPI and SMBIOS tables by scanning regions
of memory and looking for magic strings. But this is only defined for
x86, and only works on x86 because all x86 machines are essentially
PCs with a highly uniform system topology.

The ARM case is very different, and while I am no expect on RISC-V,
the following probably applies to it as well:
- there is no need to work around buggy proprietary firmware that can
boot Windows but not Linux
- there is no 'prior art' when it comes to pre-EFI boot interfaces
except for embedded style bare metal boot where all initialization is
done by the kernel (e.g., PCI enumeration and resource assignment
etc), and this is fundamentally arch specific
- ACPI is a rich firmware interface, and the ACPI specification layers
it on top of UEFI so the OS can make certain assumptions about the
extent to which the platform has been initialized by the time it hands
over.

This is why the maintainers of the arm64 and RISC-V ports appear to
agree that ACPI will only be supported when booting from firmware that
implements the EFI specification. Note that this does not impose any
requirement at all regarding which EFI implementation is going to be
used: suggestions have been made on the thread to use a) a coreboot
specific minimal EFI shim that describes the firmware tables and the
EFI memory map, b) the UPL payload for coreboot, and c) U-Boot's EFI
implementation.

I will also note that booting according to the EFI spec is not
fundamentally more secure or faster: I have done some experiments on
arm64 comparing bare metal boot with EFI boot using a minimal
implementation in Rust, for booting virtual machines under KVM. Due to
cache maintenance overhead and execution with the MMU disabled, bare
metal boot is actually slightly slower. And due to the fact that the
minimal EFI firmware enables the MMU and caches straight out of reset,
it is also arguably more secure, given that all memory permission
based protections and other page table based hardening measures (e.g.,
BTI) are always enabled.

In summary, I think it may be time to stop extrapolating from bad
experiences with buggy proprietary x86 PC firmware created by
traditional BIOS vendors for booting Windows (and nothing else) 15+
years ago. The situation is very different for non-x86 Linux
architectures, where we are trying hard to beat some sense into the
fragmented embedded ecosystem, where every SoC vendor used to have its
own fork of u-boot that booted in a slightly different manner,
requiring a lot of effort on the part of the distros to track all
those moving targets.


> Then, a consensus was reached with Ard, that FFI can be applied to RISC-V.
>

For the record, I would not characterize this as consensus. What I said was
- SMBIOS has very little significance to the kernel itself or impact
on its internal operation, and so it can be exposed via DT in a
generic manner;
- ACPI without UEFI on non-x86 is a) a bad idea, and b) fundamentally
broken on arm64. So b) is out of the question, but it is not up to me
to decide whether or not the RISC-V maintainers should entertain bad
ideas.

2023-07-06 09:36:57

by Yunhui Cui

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

Hi Conor,

On Thu, Jul 6, 2023 at 2:45 PM Conor Dooley <[email protected]> wrote:
>
> On Thu, Jul 06, 2023 at 11:43:55AM +0800, 运辉崔 wrote:
> > On Wed, Jul 5, 2023 at 11:07 PM Conor Dooley <[email protected]> wrote:
> > > On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> > > > Add the description for ffitbl subnode.
> > > >
> > > > Signed-off-by: Yunhui Cui <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
> > > > MAINTAINERS | 1 +
> > > > 2 files changed, 28 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> > > > new file mode 100644
> > > > index 000000000000..c42368626199
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> > >
> > > Firstly, new dt-bindings need to be done in yaml, not in text form.
> > > Secondly, you didn't re-run get_maintainer.pl after adding this binding,
> > > so you have not CCed any of the other dt-binding maintainers nor the
> > > devicetree mailing list.
> >
> > Re-run get_maintainer.pl and added maintainers into the maillist.
> > emm.. There is some *txt in
> > Documentation/devicetree/bindings/firmware/, isn't it?
>
> There might be, but that's not an excuse for adding _new_ ones, sorry.

Okay, I'll update it on v4.


> > > > +FFI(FDT FIRMWARE INTERFACE) driver
> > > > +
> > > > +Required properties:
> > > > + - entry : acpi or smbios root pointer, u64
> > > > + - reg : acpi or smbios version, u32
> > >
> > > Please go look at any other dt-binding (or the example schema) as to how
> > > these properties should be used. A "reg" certainly should not be being
> > > used to store the revision...
> >
> > Okay, If so,I'll add a property "version" into the dts instead of
> > "reg", just like, WDYT?
> > ffitbl {
>
> Firstly, I'd much rather you spelt this out, like "ffi-table".
>
> > smbios {
> > entry = "";
>
> I still don't understand why "entry", which is an address, is being
> represented by an empty string.
> I also don't really get why you have not used "reg" to describe its
> start address and size.
>
> > version = < 0x02 >;
>
> Probably missing a vendor prefix, and the spaces are unusual, but better
> than it was, yes.

Based on your review, I plan to modify it as follows:

ffi-table {
#address-cells = <2>;
#size-cells = <0>;
smbios@entry1 {
compatible = "smbios";
reg = <entry1>;
version = <2>;
};
smbios@entry2 {
compatible = "smbios";
reg = <entry2>;
version = <3>;
};
acpi@entry {
compatible = "acpi";
reg = <entry>;
version = <6>;
};
}

The reason why #size-cells is 0 is because only one item is needed,
#address-cells = <2>; because two u32 are needed to express the 64-bit
address.
The memory for the SMBIOS table itself will be preserved in "reserved
memory" , so we don't have to worry about this piece of memory getting
corrupted. ACPI as well. WDYT?

> > }
> > acpi {
> > entry = "";
> > version = < 0x06 >;
> > }
> > }
>
> Thanks,
> Conor.

Thanks,
Yunhui

2023-07-06 15:39:29

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [External] Re: [PATCH v3 0/4] Obtain SMBIOS and ACPI entry from FFI

On Thu, 06 Jul 2023 01:53:47 PDT (-0700), Ard Biesheuvel wrote:
> On Thu, 6 Jul 2023 at 04:04, 运辉崔 <[email protected]> wrote:
>>
>> Hi Palmer,
>>
>> On Wed, Jul 5, 2023 at 10:17 PM Palmer Dabbelt <[email protected]> wrote:
>> >
>> > On Wed, 05 Jul 2023 04:42:47 PDT (-0700), [email protected] wrote:
>> > > Here's version 3 of patch series.
>> > >
>> > > V1: The FFI (FDT FIRMWARE INTERFACE) scheme has reached a
>> > > consensus with the Maintainers.
>> > > Please refer to:
>> > > https://patches.linaro.org/project/linux-acpi/patch/[email protected]/
>> >
>> > From looking at that thread it seems that the consensus is this is a bad
>> > idea? Sorry if I'm just missing something...
>> >
>>
>> First of all, Coreboot does not support EFI, Ron has expressed, as follows:
>> "I am wondering if we can focus on risc-v here, and not drag in ARM,
>> b/c the ARM ACPI+UEFI ship has sailed. I had that discussion in 2013
>> ;-) and it's clear we don't want to redo it.
>> In general, in my world, because of the many problems that come with
>> UEFI (security, code quality, performance), we'd like to avoid
>> requiring a dependency on UEFI just to get ACPI on RISC-V. It also
>> seems, from other discussions I'm having, that there is some belief
>> that ACPI will be wanted on RISC-V. It would be nice to separate those
>> pieces on RISC-V; certainly they were separate for a very long time in
>> the x86 world (we had ACPI+SMM on coreboot laptops without UEFI for
>> example)."
>>
>
> There appears to be a bit of cargo cult going on here.
>
> I agree that the traditional BIOS vendors did a terrible job pivoting
> to (U)EFI when it became a requirement for booting Windows on x86 PCs,
> and coreboot did an excellent job providing a retrofit alternative
> that was more secure and robust.
>
> However, it makes sense to distinguish between
> a) the UEFI specification
> b) the UEFI reference implementation (edk2)
> c) commercial implementations created by BIOS vendors for x86 PC OEMs
> that do not perform any testing beyond booting Windows.
>
> coreboot decided not to implement EFI at all, which on x86 means
> booting in a mode that is similar to BIOS boot. Given how the ACPI and
> DMTF (for SMBIOS) specifications were already under development when
> UEFI was being rolled out on x86, those specs contain provisions
> defining how to obtain the ACPI and SMBIOS tables by scanning regions
> of memory and looking for magic strings. But this is only defined for

In theory we have that in RISC-V as well: on boot we don't actually have
a DT pointer, but instead a "config string" pointer. That's a bit of a
retcon from when we were planning on adding our own firmware probing
interface, but in order to appear to have never made a mistake we just
said that config strings can be anything and have magic numbers to
differentiate between the flavors.

IIUC we don't take advantage of that in Linux, though, so maybe let's
just pretend it doesn't exist?

> x86, and only works on x86 because all x86 machines are essentially
> PCs with a highly uniform system topology.
>
> The ARM case is very different, and while I am no expect on RISC-V,
> the following probably applies to it as well:
> - there is no need to work around buggy proprietary firmware that can
> boot Windows but not Linux
> - there is no 'prior art' when it comes to pre-EFI boot interfaces
> except for embedded style bare metal boot where all initialization is
> done by the kernel (e.g., PCI enumeration and resource assignment
> etc), and this is fundamentally arch specific
> - ACPI is a rich firmware interface, and the ACPI specification layers
> it on top of UEFI so the OS can make certain assumptions about the
> extent to which the platform has been initialized by the time it hands
> over.
>
> This is why the maintainers of the arm64 and RISC-V ports appear to
> agree that ACPI will only be supported when booting from firmware that

Yes, we're basically in the same spot as arm64 is here -- or at least
we're aiming to be, we've yet to even release a kernel that boots with
ACPI so we have no legacy compatibility yet.

> implements the EFI specification. Note that this does not impose any
> requirement at all regarding which EFI implementation is going to be
> used: suggestions have been made on the thread to use a) a coreboot
> specific minimal EFI shim that describes the firmware tables and the
> EFI memory map, b) the UPL payload for coreboot, and c) U-Boot's EFI
> implementation.
>
> I will also note that booting according to the EFI spec is not
> fundamentally more secure or faster: I have done some experiments on
> arm64 comparing bare metal boot with EFI boot using a minimal
> implementation in Rust, for booting virtual machines under KVM. Due to
> cache maintenance overhead and execution with the MMU disabled, bare
> metal boot is actually slightly slower. And due to the fact that the
> minimal EFI firmware enables the MMU and caches straight out of reset,
> it is also arguably more secure, given that all memory permission
> based protections and other page table based hardening measures (e.g.,
> BTI) are always enabled.
>
> In summary, I think it may be time to stop extrapolating from bad
> experiences with buggy proprietary x86 PC firmware created by
> traditional BIOS vendors for booting Windows (and nothing else) 15+
> years ago. The situation is very different for non-x86 Linux
> architectures, where we are trying hard to beat some sense into the
> fragmented embedded ecosystem, where every SoC vendor used to have its
> own fork of u-boot that booted in a slightly different manner,
> requiring a lot of effort on the part of the distros to track all
> those moving targets.

That's roughly where we're trying to go in RISC-V land, at least for
most software people. Everyone gets their own ISA, which obviously
causes a ton of fragmentation, but not really anything we can do about
that. At least we can avoid adding additional sources of fragmentation
from the software side of things, though.

>> Then, a consensus was reached with Ard, that FFI can be applied to RISC-V.
>>
>
> For the record, I would not characterize this as consensus. What I said was
> - SMBIOS has very little significance to the kernel itself or impact
> on its internal operation, and so it can be exposed via DT in a
> generic manner;
> - ACPI without UEFI on non-x86 is a) a bad idea, and b) fundamentally
> broken on arm64. So b) is out of the question, but it is not up to me
> to decide whether or not the RISC-V maintainers should entertain bad
> ideas.

IMO we have enough bad ideas in RISC-V already and thus should avoid
adding more.

2023-07-07 16:43:42

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] dt-bindings: firmware: Document ffitbl binding

Hey,

On Wed, Jul 05, 2023 at 07:42:51PM +0800, Yunhui Cui wrote:
> Add the description for ffitbl subnode.
>
> Signed-off-by: Yunhui Cui <[email protected]>
> ---
> .../devicetree/bindings/firmware/ffitbl.txt | 27 +++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 28 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/firmware/ffitbl.txt
>
> diff --git a/Documentation/devicetree/bindings/firmware/ffitbl.txt b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> new file mode 100644
> index 000000000000..c42368626199
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/ffitbl.txt
> @@ -0,0 +1,27 @@
> +FFI(FDT FIRMWARE INTERFACE) driver
> +
> +Required properties:
> + - entry : acpi or smbios root pointer, u64
> + - reg : acpi or smbios version, u32
> +
> +Some bootloaders, such as Coreboot do not support EFI,
> +only devicetree and some arches do not have a reserved
> +address segment. Add "ffitbl" subnode to obtain ACPI RSDP
> +and SMBIOS entry.

Since the conversation on this stuff all seems to be going absolutely
nowhere, the ACPI portion of this is intended for use on RISC-V in
violation of the RISC-V ACPI specs. It also goes against the
requirements of the platform spec. Quoting from [1]:

| > Just so we're all on the same page, I just now asked Mark Himelstein
| > of RISC-V International if there is anything in RISC-V standards that
| > requires UEFI, and the answer is a solid "no."
|
| Huh? Firstly, running off to invoke RVI is not productive - they don't
| maintain the various operating system kernels etc.
| Secondly, that does not seem to be true. The platform spec mandates UEFI
| for the OS-A server platform, alongside ACPI:
| https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
| and the OS-A embedded platform needs to comply with EBBR & use DT:
| https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#32-boot-process
|
| EBBR does say that systems must not provide both ACPI and DT to the OS
| loader, but I am far from an expert on these kind of things & am not
| sure where something like this where the DT "contains" ACPI would stand.
|
| The RISC-V ACPI spec also says "UEFI firmware is mandatory to support
| ACPI":
| https://github.com/riscv-non-isa/riscv-acpi/blob/master/riscv-acpi-guidance.adoc

NAKed-by: Conor Dooley <[email protected]>

Cheers,
Conor.

[1] - https://lore.kernel.org/linux-riscv/20230707-attach-conjuror-306d967347ce@wendy/


Attachments:
(No filename) (2.57 kB)
signature.asc (235.00 B)
Download all attachments