Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33782 )
Change subject: vbe.h: Make use of __packed attribute in vbe_info_t structure ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33782/7/src/include/vbe.h File src/include/vbe.h:
https://review.coreboot.org/c/coreboot/+/33782/7/src/include/vbe.h@47 PS7, Line 47: } __packed vbe_info_t;
vbe_info_t structure is read from HW through int 10 0x4f00.
No it is not. `int 10 0x4f00` fills a structure in the given buffer. And that structure is then carefully converted to `vbe_info_t`. See the comment above, it is not meant to be the same structure. `vbe_info_t` is an internal representation of the VBE info, not an interface.
And why do compiler add any extra padding to make it align ?
Because it is mandated by C. The `struct` in C is not fit for ABIs or hardware interfaces that don't follow the same alignment rules as C. You can only rely on compiler extensions like `packed`, to make it work. But then you break other assumptions of the C language. That's why I believe, `packed` should only be used by the most experienced developers. And only in extreme cases.
I could see compiler alignment cause issue while reading video_mode_list[0].
Yes. But only because you used `vbe_info_t` in a way that it was not written for. You used memcpy() to copy data in an incompatible format into it.