Marty E. Plummer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36346 )
Change subject: cbfs: store header offset as absolute value BE from fmap_top ......................................................................
Patch Set 1:
Patch Set 1:
Well, as you mentioned, there is the consistency issue, and to be frank I am not aware of any things other than src/lib/cbfs.c which look at this particular offset. I could be wrong but I think the others just look at the header it points to.
Here are just two other CBFS parsers I'm aware of that are using this pointer:
https://github.com/coreboot/seabios/blob/master/src/fw/coreboot.c#L432 https://github.com/coreos/grub/blob/2.02-coreos/grub-core/fs/cbfs.c#L345
I know there are more and I'm pretty sure we couldn't find all of them if we tried.
This has worked this way since basically forever and I don't think we should break it just for some minor design cleanliness concern. I understand the problem of supporting big-endian systems if we want that, bug big-endian systems can still explicitly convert a value from little-endian to CPU byte order, so I think that's how we should handle this one to avoid breaking older parsers.
True, it is fairly simple (in c-land, at least) to convert be to le. Problem is, a coreboot.rom (as it currently stands) built on a big endian system would store this pointer as be, so conversion in cbfs.c would need even more logic to determine whether or not to leave it alone.