Hi

I appreciate all the solid input on this one!

If coreboot had to be rewritten, I'd argue FDT would be a nicer way of structuring a payload handoff.

However that is not the case, and coreboot tables are well established in many different payloads, supporting very different and specialized use cases.

I also share the scepticism about the universal payload concept: the reality is that hardware init and payload are tightly coupled.

The increased complexity for doing it otherwise may not be worth it, since it involves a lot of runtime decisions based on the payload.

Is it really worth overhauling coreboot tables, which only pass limited information like memory map, board id, framebuffer,... ?

Maybe...

Is it worth maintaining 2 payload handoffs in the tree? My answer is pretty solid to that: *no*.

So I won't be pursuing this FDT as a payload handoff as something that lives side by side with coreboot tables, as I've had my fill with multiple codepaths doing the same thing in the tree:

romcc vs. C env bootblock, CAR teardown in ramstage vs. postcar, resource allocation v3 vs. v4, legacy LAPIC_CPU_INIT vs. parallel_mp, ...

It's always a big mess to maintain multiple solutions.  I'd hate to introduce more of that...

Currently EDK2 as a USF payload is built with a layer (shimlayer) that translates coreboot tables and then loads the DXE stage as a separate program.

That shimlayer is just very poorly implemented: it tries to guess the flash memory mapping and then loads ELF at runtime.
A better way would be to use libpayload to parse coreboot tables and pass whatever format EDK2 wants and have cbfstool process the DXE stage ELF (the universal one ;-) ) so that it's easy to find and load.

As Libpayload is maintained with coreboot, using it to create a new specialized payload loader is the proper way to deal with payloads requiring a different format.

And completely unrelated to the payload handoff (so it indeed should not impact that decision/discussion)

- FDT as an interface to FSP would be a big improvement for reusability. Currently FSP is totally not reusable and you need code for each SOC, which is exactly the opposite of what Intel USF claims to want to achieve.

- FDT to replace ACPI... For static information that might work, but ACPI has a lot of real code in there that would require actual drivers in Linux to replace it with. It's a lot of work, so it would best be set up as a collaborative effort: let's pick a platform and go for it?

Kind regards

Arthur

On Tue, Nov 8, 2022 at 1:14 AM Julius Werner <jwerner@chromium.org> wrote:

> > 1. You said that coreboot tables were GPLv2 licensed, and this could
> > be an adoption problem for payloads...
>
> The only generator code I know is GPLv2.
>
> One could argue that only coreboot should ever generate this format,
> but I think one goal here is to enable or even encourage new situations.

Yes, 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'm trying to make the case for "unifying payload handoff
formats between different firmwares is generally not a useful idea and
not worth the cost".

> > I think the fundamental question here is still: what do we want to
> > achieve, and how does replacing coreboot tables with something else
> > help with that.
>
> This isn't just a technical change, for me there is also the more
> political goal to make more of what Intel does with USF going forward
> (please see https://github.com/UniversalScalableFirmware/Introduction/blob/main/USF_Overview.pdf )
> maximally useful for and within the coreboot context, ideally while
> encouraging reuse.
>
> Since Intel has planned to use CBOR and are unlikely to use coreboot
> tables for anything outside of coreboot I see using FDT as a way to
> improve the situation for everyone while making players come together
> more - that's an unusually rare win-win.
>
> I strongly prefer FDT over CBOR. FDT is simple on byte level, which I
> find important to not lose the simplicity of coreboot tables. FDT can
> easily be parsed without RAM, CBOR not so much.

I'm a bit confused by the hate for CBOR here to be honest, I think
it's a very simple format and not much different from FDT in terms of
complexity (maybe the CBOR tag parsing is slightly more complicated,
but in return FDT has weird quirks like the memory reserve map,
separate strings table and phandles you have to deal with), and I
don't think parsing one of them without RAM would be much more
cumbersome than the other. But I don't want to replace the coreboot
tables with either so it's probably not worth going off on that
tangent.

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

> > ending up with a solution that's less suited for either target
>
> I don't think FDT is less suited than coreboot tables where it
> matters the most - simplicity on byte level.

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

> > The way we use payloads nowadays is often very deeply tied to
> > coreboot,
>
> More so if "often" means number of shipped systems, less so if "often"
> means the number of individual payloads.

Well, if we want to open that line of argument we also have to
consider what's left in the handoff for the payloads where this isn't
true. If your payload isn't even accessing CBFS or printing logs to
the CBMEM console (both of which are supported directly within
libpayload), then what do you still need a handoff for at all? Just to
parse the mainboard vendor string?

> > can hopefully stay pretty isolated and self-contained in the code base
>
> I think there should be just one source of truth, from which other
> formats are translated/derived.

I mean I generally agree with that sentiment, but then we are at an
impasse because I disagree with abandoning coreboot tables or forcing
FDT generator code into coreboot images for those who aren't
interested in this project. I think keeping these things side-by-side
in the code base with a Kconfig to switch between them is the only way
we're going to let everyone achieve what they want here, even if it is
going to add extra maintenance burden.

> > 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. 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, and since the argument for
coreboot tables is mostly simplicity / lean code it's not a solution
for that side to just generate both.

> Julius Werner wrote:
> > > - reflection is easy
> > > - no problem of 'header mismatch' to have the right definition of a struct
> > > - easy to update with a text editor without needing to write code
> >
> > 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? When have you ever had a situation
where you'd want to edit coreboot table structures at runtime? (FWIW,
even if you wanted to do that for some reason it would still not be
very hard to write such a thing for coreboot tables -- the surrounding
UI and stuff of this shim payload would probably take the lion's share
of the effort, compared to the code to be able to delete/add/change
table entries for the known structures.)

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

> > Unless we wanted to replace ACPI with FDT¹
> > ¹ 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). We already have support
for FDT for kernel handoff in coreboot today, after all.



For the record, I just met with Lean from 9elements about this as well
and basically discussed that same position -- that I don't want to
block anyone from doing this but that I would like it to remain a
compile-time option and would like coreboot tables to continue being
available and supported as a first-party citizen going forward (and
not "soft deprecated" in favor of FDT, or have the FDT stuff seep into
something that can not be compiled out). As I understood, Lean (and
Arthur?) want to continue pursuing this implementation under that
premise, and then we'll have to see in practice how well we can make
them coexist side-by-side without stepping on each other's toes.
_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-leave@coreboot.org

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