linux-kernel - Re: [PATCH] dt-bindings: firmware: coreboot: Convert to YAML

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAODwPW_LfeC7UFnDMxJFfKCFBoGBvu2O6S=otD0Sj7ssOSJVgw@mail.gmail.com>
Date: Tue, 25 Nov 2025 16:36:07 -0800
From: Julius Werner <jwerner@...omium.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Chen-Yu Tsai <wenst@...omium.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org, 
	chrome-platform@...ts.linux.dev, Julius Werner <jwerner@...omium.org>, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: firmware: coreboot: Convert to YAML

> >>> +title: COREBOOT firmware information
> >>
> >> Coreboot
> >
> > OK. Side note, coreboot is stylized in all lowercase letters.
> > Should I follow that or just use standard English rules?
>
> Just choose one. Here was capitals, but in description not. Preferably
> Coreboot or coreboot

coreboot should always be written all lowercase, even at the start of
a sentence (i.e. "...bookkeeping structures to the kernel. coreboot's
FIT image payload can insert..."). Not sure why I wrote it in caps in
the heading here, that was never really correct, not even back then.

> >>> +  The device tree node to communicate the location of coreboot's
> >>> +  memory-resident bookkeeping structures to the kernel. Coreboot's
> >>> +  FIT image payload can insert the node into the device tree. If a

nit: Not really sure what "coreboot's FIT image payload" is supposed
to be? Do you mean "coreboot's FIT image code"? (The file is just
called fit_payload.c because the FIT image takes the place of the
usual coreboot payload, but the code that does the device tree
stitching itself is still coreboot proper.) To make the point you're
trying to make here, why not just say: "This node may be inserted
either by coreboot itself or by a second-stage bootloader (a coreboot
"payload"), depending on where the kernel device tree is finalized."

> >>> +        /* Firmware actually emits "coreboot" node without unit name */
> >>> +        coreboot@...ea000 {
> >>> +            compatible = "coreboot";
> >>> +            reg = <0xfdfea000 0x264>, <0xfdfea000 0x16000>;
> >>
> >> That's the same address in both places, so the same one entry. You need
> >> two distinctive addresses or binding needs changes to have only one item
> >> as well.
> >
> > The description does mention that the latter block can include the
> > former. It's really up to the firmware. If you like I can include
> > two examples to cover both cases.
>
> Lovely, I don't think we should accept growing this binding at all until
> coreboot fixes this mess (duplicated entry and missing unit address).

As Chen-Yu said, the second region usually contains the first. It is
possible (in fact tends to be common on many platforms, because the
coreboot table is often the last region added to the downward-growing
CBMEM) for the start addresses to match. That is not an error or a
"duplicate", they're still describing two different things.

We can start adding the @<address> part if you want but it hasn't
really been relevant in practice since there will always be only one
coreboot in the system anyway. As far as I understand, node names are
just informational in the device tree and the compatible string is
what identifies the actual binding. Since bootloaders on older
platforms often don't get updated, it is probably still a good idea to
 mention that in a comment like Chen-Yu did here even if we change it.