[coreboot] coreboot specific ACPI table

Julius Werner jwerner at chromium.org
Mon Apr 25 22:02:41 CEST 2016


> On 04/18/2016 08:17 PM, Julius Werner wrote:
>> For comparison, the coreboot device tree interface on ARM
>> (https://lkml.org/lkml/2014/6/16/622, unfortunately never picked up by
>> the maintainers but still used on Chromebooks today) only exports
>> start address and size of the coreboot table and the whole CBMEM area,
>> so maybe ACPI should just match that. (I guess if CBMEM is referenced
>> by the coreboot table like Aaron said, we could also drop that part.)
>>
> Did you get other feedback than the one mail by Rob Herring?
> We should keep pushing this.

No, I just never heard from them again (as I so often do trying to
work with upstream Linux), got frustrated and gave up. Maybe we should
try again... another two years of Chromebooks shipped with that
interface might now convince them that there's no point bikeshedding
the tiniest naming details anymore.

>>> Here's the 1 million dollar question: Do we want to get rid of coreboot
>>> table and only have a coreboot> specific table?

I don't know enough about how ACPI tables look like to comment on the
technical aspects for this, but please also keep the legacy support
costs in mind when considering to change the whole format. There are
millions of Chromebooks out there which still produce the old tables
(they could be updated, but that would probably not be worth the
effort), so the kernel driver and the cbmem utility would probably
need to be able to detect and parse the old format anyway. At that
point, haven't we just made it more complicated without really gaining
anything?

> This is somewhat orthogonal, but the coreboot tables are not really
> defined all that well. There's no specific endianness for each of the
> fields aside from whatever the endianness the CPUs was in during the
> time of the write. The structs used are also not  marked as packed so
> there's not much of a guarantee on field alignment -- especially as
> they are allocated one after another within the table itself.

I think the alignment is actually standardized by C (both start and
size get aligned to the largest-width data type in the struct). Since
it looks like we always used explicit-width types and always wrote the
structs in a way that avoids any need for padding, it should be pretty
well-defined and match the reader's expectation. We just need to
continue paying attention to that when we add entry types... marking
the structs as packed shouldn't change anything but reduce
performance. (There are a few hazards for unaligned uint64_t values,
but we're already making sure our architectures can handle that for
CBFS anyway.)

For the endianness issue, I would suggest to just declare that all the
fields which aren't explicit right now should be little-endian (both
in the coreboot tables and CBFS), and reflect that in the code the
next time we try to add a big-endian architecture. It's unfortunate
that some of the CBFS fields started out locked to the wrong
endianness (and maybe we could even change that at some point), but
little-endian rules the world these days and it's easiest to just use
it throughout.



More information about the coreboot mailing list