On Wed, Oct 21, 2020 at 2:37 PM Tim Wawrzynczak twawrzynczak@google.com wrote:
On Wed, Oct 21, 2020 at 1:50 PM Aaron Durbin adurbin@google.com wrote:
On Wed, Oct 21, 2020 at 1:20 PM Tim Wawrzynczak via coreboot < coreboot@coreboot.org> wrote:
Hi coreboot community,
Currently there are 3 different "strapping" entries in the coreboot tables, and with the recent addition of fw_config ( https://review.coreboot.org/c/coreboot/+/41209), we would also like to add the 64-bit fw_config field (updated here https://review.coreboot.org/c/coreboot/+/45939) to the coreboot table as well.
In this patch (https://review.coreboot.org/c/coreboot/+/46605), I am proposing to deprecate the 3 current "strapping" entries (board ID, ram code and SKU ID), and add them all to 1 entry, containing board ID, ram code, SKU ID as well as fw_config. This saves the overhead of parsing 4 different entries to obtain board configuration information.
Not all of these entities are sourced through the same mechanism (gpios vs cbi). As such any time you want to read one of those fields you'll be unconditionally needing to obtain all of those while at the time the user may only want one field. And then this can happen from multiple stages.
The patch I'm proposing doesn't change whether or not the functions `board_id()`, `ram_code()`, `sku_id()` are called or not, or their definition. It will, however, currently waste 12 bytes of space if none of the weak functions are overridden. However, if a board uses all 4 IDs, then this patch saves 20 bytes of space in the table on redundant tags/sizes in the records. I guess this is the tradeoff here.
I would say that since these things aren't all related to answering the question someone may want to answer that we shouldn't go down this path. Width of fields, e.g., could be different across platforms since not everything is consistent.
I'm not sure I follow; board_id, ram_code and sku_id are all currently defined as 32 bits in coreboot/libpayload.
Sorry. I missed that this topic was pertaining to coreboot tables -- not runtime in coreboot. And I didn't read the patch. :)
I think it doesn't matter how the tables are encoded.
Would like to hear any thoughts on this,
- Tim
coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org