I mostly agree with Peter here: 
"So a way forward where you can ignore FDT is for coreboot tables to
stay as-is, and a Kconfig option to make coreboot use its coreboot
tables to generate an FDT."

I suspect that, someday, the FDT may contain different information than is contained in coreboot tables. We can not anticipate everything: as needs change, we could find it is not possible to create the necessary FDT from coreboot tables.

For that reason, I think FDT creation should be in coreboot, not in a stage, because then the creation of the FDT will be easiest. And enabling FDT with a kconfig option makes sense. coreboot tables are small, so, thinking about it, it is hard to see a case where we'd ever not have coreboot tables as well.

On Thu, Nov 10, 2022 at 8:23 AM Peter Stuge <peter@stuge.se> wrote:
Julius Werner wrote:
> I think only coreboot should ever generate this format. I'm certainly
> not trying to make a case for "we should get other firmware projects
> to adopt coreboot tables" here, I don't think that makes any sense.

I think it makes some sense because coreboot does what we consider best.
Of course we want others to reuse the very best parts of that.

But I also recognize that it is unlikely to happen, so to improve the
situation overall (ie. also outside coreboot) I want to consider FDT
in coreboot.


> I'm trying to make the case for "unifying payload handoff formats
> between different firmwares is generally not a useful idea

MBR called to disagree.


> and not worth the cost".

This is the only interesting discussion.


> I understand that some people here want to instrumentalize coreboot
> to try to force Intel to change course on its USF specification, and
> honestly I don't really care about that one way or the other,

I find that silly polemic. I see this not as instrumentalizing
coreboot but as coreboot accomodating Intel a little while also
getting benefit - of course with the longer term goal of Intel
and coreboot becoming a better match.

You don't have to care about USF or Intel but do consider that
there probably would be no coreboot community within industry
were coreboot not to add value to Intel hardware.

Our community is obviously not in danger over this but the better
Intel and coreboot go together the more opportunities for really
good solutions we may have in the future.

If coreboot doesn't push Intel then Intel will push coreboot (not
neccessarily on purpose, just through inertia). You may want to
ignore that for now but the coreboot will be dealing with the outcome
for some time and now is when we can act.


> I'm just asking to please not let the fallout of that negatively
> affect those of us who still just care about using coreboot as a
> firmware and not as a political cudgel.

I understand if you and others want to ignore politics, I did too for
a long time.

That said, I wouldn't worry about negative effects. I certainly have
no interest to remove coreboot tables nor do I see any reason to do
so nor have I seen anyone mention anything in that direction.

Do not make up strawmen.


> > I don't think FDT is less suited than coreboot tables where it
> > matters the most - simplicity on byte level.
..
> I'm a bit confused by the hate for CBOR here to be honest,
..
> I don't think parsing one of them without RAM would be much more
> cumbersome than the other.
..
> FDT is a tokenized hierarchical key-value store with string keys (and
> a ton of extra quirks), whereas the coreboot tables are just a tagged
> list of C structures. That's an order of magnitude of difference in
> simplicity (and binary size).

A RAM-less FDT parser probably doesn't need to load the entire tree
but merely find one value at a time, perhaps without state between
values. The complexity of that in an FDT is low since FDT is flat and
has simple primitives. CBOR, well..

--8<-- RFC8949
   The initial byte of each encoded data item contains both information
   about the major type (the high-order 3 bits, described in
   Section 3.1) and additional information (the low-order 5 bits).  With
   a few exceptions, the additional information's value describes how to
   load an unsigned integer "argument":

   Less than 24:  The argument's value is the value of the additional
      information.

   24, 25, 26, or 27:  The argument's value is held in the following 1,
      2, 4, or 8 bytes, respectively, in network byte order.  For major
      type 7 and additional information value 25, 26, 27, these bytes
      are not used as an integer argument, but as a floating-point value
      (see Section 3.3).

   28, 29, 30:  These values are reserved for future additions to the
      CBOR format.  In the present version of CBOR, the encoded item is
      not well-formed.

   31:  No argument value is derived.  If the major type is 0, 1, or 6,
      the encoded item is not well-formed.  For major types 2 to 5, the
      item's length is indefinite, and for major type 7, the byte does
      not constitute a data item at all but terminates an indefinite-
      length item; all are described in Section 3.2.
-->8--

This is bizarrely complex for storing a simple scalar.


> what do you still need a handoff for at all?
> Just to parse the mainboard vendor string?

Memory map at a minimum.


> I disagree with abandoning coreboot tables or forcing FDT generator code
> into coreboot images for those who aren't interested in this project.

So a way forward where you can ignore FDT is for coreboot tables to
stay as-is, and a Kconfig option to make coreboot use its coreboot
tables to generate an FDT.


> I think keeping these things side-by-side in the code base with a
> Kconfig to switch between them

Like Arthur I don't appreciate the "side-by-side" narrative, nor an
expectation for two independent data sets.


> > > I'd like to make sure that we can agree to keep coreboot tables as
> > > a first-class citizen now and in the future, and don't force a new
> > > format on anyone.
> >
> > I don't think anyone proposed to not generate coreboot tables anymore.
>
> I'm talking about where this is eventually supposed to be headed, not
> just the first experimental patch.

This is supposed to be headed to additional benefit for both coreboot
and USF (users). Others will use coreboot in different ways, e.g.
without FDT.


> I think we can all agree that there's no point in having two
> firmware handoff structures that both need to be parsed to get the
> full picture,

Noone suggested anything like that. Don't waste our time.


> and since the argument for coreboot tables is mostly simplicity /
> lean code it's not a solution for that side to just generate both.

If you mean "it's not a solution for coreboot to just generate both"
then I ask why not? Two things can be true at the same time: coreboot
tables are simple to generate and the code to generate FDT from
coreboot tables is also simple.


> > > Well, Arthur also admitted that none of these points are really
> > > relevant to coreboot's payload handoff, ;) which I fully agree with.
> >
> > Currently that may be true, but reflection would pretty much be
> > required for a new payload for manual parameter tweaking during boot.
>
> Yeah, but what's the use case?

There are several, both outside and within the boot process.

FDT can be meaningfully persisted and so be useful both before and after boot.

> When have you ever had a situation where you'd want to edit
> coreboot table structures at runtime?

You can also consider FDT in coreboot as plumbing to enable new
situations that were best avoided with coreboot tables, then of
course noone ever wanted to edit them. That doesn't mean that noone
would ever want to edit firmware handoff information, especially if
we also consider FSP.

Please be a bit more imaginative here.


> it would still not be very hard to write such a thing for coreboot
> tables

Of course. One goal is to have more freedom than that allows, or at a
very minimum fill a namespace not (exclusively) defined by coreboot
or Intel.


> > As I understand it, cbtable member alignment is currently undefined.
> > That matters when building firmware and payload with different compilers.
> > "Just use the coreboot compiler" is not a good answer.
>
> Can we please stop perpetuating this myth? It's just wrong, like I
> already explained in my first email here.

I don't think it's wrong. I used to, but not anymore.

This is not about 64-bit values which you mentioned in your first
email, this is about struct member offsets in the struct.

The structs are not packed so the compiler decides.

This is why both Nico and I are so strong proponents of explicit
serialization of the tables. (I'd also like to create new tags for
coreboot tables with packed values but that's another matter.)


> > > ¹ I wouldn't mind that btw., replacing ACPI.
> >
> > That would be fantastic! But maybe it's more of a stretch goal. :)
>
> FWIW I think this is a completely separate discussion and has nothing
> to do with what we should do for payload handoff (again, just like
> with the FSP thing, kernel handoff is a completely different thing
> that would have very little to share with it).

If all handoff interfaces were to use a single format (this would be
great) then obviously it can be a single data set, which would make
life simpler for a lot of different people.


> (and not "soft deprecated" in favor of FDT,

Since this your narrative is baseless it makes it look like you are
intentionally acting in bad faith. That's not a good look. Be careful.


> or have the FDT stuff seep into something that can not be compiled out).

Like vboot has been for maybe a decade now yes, I agree strongly that
this is important to avoid.


> without stepping on each other's toes.

I wouldn't worry.


//Peter
_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-leave@coreboot.org