Change in coreboot[master]: libpayload: Support x86 payload loaded via Linux Boot Protocol
Attention is currently required from: Tim Wawrzynczak, Julius Werner. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63748 )
Change subject: libpayload: Support x86 payload loaded via Linux Boot Protocol ......................................................................
Patch Set 1:
(2 comments)
File payloads/libpayload/arch/x86/head.S:
https://review.coreboot.org/c/coreboot/+/63748/comment/26591d80_89cd28e8 PS1, Line 86: jne 1f
(Note that as far as I can tell, this hasn't worked in a long time, maybe ever(?)... […]
Good to know. I only had a quick look at it and couldn't find that CB_MAGIC in coreboot. I also seem to remember that we simply pass a pointer to cbtables.
https://review.coreboot.org/c/coreboot/+/63748/comment/e24a1cd4_c9e93802 PS1, Line 105: esi
Where does %esi come from? Is the caller supposed to set it? That would be a problem for callers tha […]
The caller points to a configuration struct (aka. zero page) in %esi. If we always run into this path, I guess it's best to add a Kconfig so we always know what to expect?
I could also add some sanity check on the value, e.g. %esi <= 0xa0000 || 0x100000 <= %esi <= 0x2000000 (the C code assumes at least 32MiB are available, IIRC). But not sure if it's worth trying to get this perfect, as the whole thing feels like just a rather custom quirk.
-
Nico Huber (Code Review)