Patch Set 4:

Patch Set 4:

Hello Mathew,

usually I'm asked to write documentation, but now I'm
the one that actually misses some information. I know
it is common for chromium firmware to have rendundant
FMAP entries for the descriptor regions. But, AFAICS,
this is nowhere documented. Especially the reason for
these entries would be nice to know, as coreboot it-
self doesn't care about them. Oh, and what does the
SI prefix stand for?

Nico, what are you requesting exactly? I'm not entirely sure, but it sounds like you are assuming coreboot shouldn't care about other regions covered by the descriptor. As system builders we definitely care about managing all regions/assets that comprise the spi flash. That may not be true for all users of coreboot, but it's definitely true for Chrome OS.


Yep this is exactly why I added SI_DESC and SI_ME initially. We debated just having one region to cover everything, but since the descriptor is always 4K it seemed easy to keep it separate. Now of course we've started having some systems with SI_GBE and SI_EC regions which are also needed for init but not provided by coreboot. Nothing says these all have to be within the same region of flash, but that is how it has ended up so far.


>
> As for SI prefix, I actually don't know. Silicon Initialization? That sounds like an intel term enough that it might be that. I never bothered to look up where that came from. Perhaps Duncan remembers?


Yes it is for Silicon Init, we wanted to categorize anything required by the silicon but not provided by coreboot.


>
> >
> > Without such documentation, a validation option can
> > be confusing. For instance, if I were told to imple-
> > ment such an option in ifdtool, I would rather check
> > that all FMAP regions are within the IFD BIOS region,
> > because I wouldn't expect redundancy.
>
> That's assuming one only cares about the BIOS region. And the check is that things are hierarchical within those regions w/o overlaps. If you care about covering every region then those same checks would apply to other regions covered in the descriptor.

In theory we could auto-generate the FMAP regions for this based on the descriptor provided at build time. But we would need to figure out how to mesh that with the static FMAP definition.

View Change

To view, visit change 34802. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idebf105dee1b8f829d54bd65c82867af7aa4aded
Gerrit-Change-Number: 34802
Gerrit-PatchSet: 4
Gerrit-Owner: Mathew King <mathewk@chromium.org>
Gerrit-Reviewer: Mathew King <mathewk@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin@chromium.org>
Gerrit-CC: Duncan Laurie <dlaurie@chromium.org>
Gerrit-CC: Nico Huber <nico.h@gmx.de>
Gerrit-Comment-Date: Tue, 13 Aug 2019 18:16:05 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment