Hi,

I wasn't sure if I should also shim in here, but I just wanted to leave my opinion here. I agree with adding FDT as another method to handoff data to a payload. Despite all the advantages that Ron and others pointed out, I think something that passed data into another project e.g. payload here - whatever that is, should never be specific to one project. It should not be the status-quo that other projects, if they want to be loaded as a payload after coreboot need to implement other project specific implementations. What I mean by this is that hands-off's between different projects should always rely on an open standard that is *not* specific to the project.

In this particular case, my opinion is that other payloads implement coreboot tables should not be the defacto standard. coreboot should be encouraged to have a flexible option i.e. FDT to pass data into a payload - so that the payload would not care if that data is coming form coreboot - or any other underlying solution (oreboot?).

Of course I do understand the need for being backwards compatible, so I also would propose the approach to have these two solutions living site-by-site next to each other for some time - and then we as a community can decide how we want to move forward. However, we should never block those kind of approaches to make the open-source firmware would a bit more unified - and we should definitely not do it "just because Intel wants us to do it".

Best,

Chris

On 11/8/22 18:25, Jonathan Zhang (Infra) via coreboot wrote:

Hi, following is my perspective:

On FDT as a coreboot payload interface: I fully agree that coreboot table is well established, and we want to maintain the stability of the coreboot payload interface. That being said, FDT offers more than coreboot table does. Ron illustrated that very well, I would like to add that:
a. FDT is what Linux kernel expects on most of the embedded devices.
b. FDT is one of the two methods that ARM Linux server kernel expects. The other method is UEFI/ACPI. For coreboot to boot LinuxBoot on ARM server, coreboot need to provide either FDT or UEFI interface.
So my proposal is to add FDT as a parallel path, but do not retire coreboot table until FDT is very robust. I agree that 2 ways of doing same thing is generally not desired; I suppose that applies to more to coreboot internal, but not necessarily for external facing interfaces.

On FDT as an interface with FSP: such a discussion will not be effective without silicon vendor in the loop. When coreboot community and silicon vendor have trust and rapport with each other, such a discussion will go smoothly.

On FDT vs. ACPI: At least for target OS kernel on server, ACPI will be expected. 

Thanks,
Jonathan

On Nov 8, 2022, at 1:19 AM, Arthur Heymans <arthur@aheymans.xyz> wrote:

This Message Is From an External Sender
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

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

--

Christian Walter
Head of Firmware Development / Cyber Security

9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany

Email:  christian.walter@9elements.com

Phone:  +49 234 68 94 188

Mobile:  +49 176 70845047

Sitz der Gesellschaft: Bochum

Handelsregister: Amtsgericht Bochum, HRB 17519

Geschäftsführung: Sebastian Deutsch, Eray Basar

Datenschutzhinweise nach Art. 13 DSGVO