Hi,
On 19. 04. 22 11:42, Arthur Heymans wrote:
Nice catch! Regardless of the upshot of this it's worth fixing this problem in the coreboot tables implementation. I'm not very knowledgeable on the topic but don't a lot of CPU ARCH support unaligned pointer access in hardware but it slows things down?
Yes a bit. But mostly for SIMD instructions.
The way you find coreboot table structs is by looping over ->size, since payloads may not know some tags. So aligning the size up to 8 bytes when generating the coreboot tables should do the trick.
Yes and no. Problem is if you have following layout in your struct:
u32 x u64 y
On 64-bit you will get "space" in the middle of your struct.
u32 x u32 hidden_padding u64 y (aligned to 8 byte boundary)
and on 32-bit you will get:
u32 x u64 y (aligned to 4 byte boundary)
And similar thing happens at the end of the struct where extra padding is inserted, but the size depends on the actual ABI/Architecture. Linux uses other ABI than UEFI for example. You can play with that using gcc. Just add -Wpadded to your compiler flags.
The problem above exactly happens with coreboot memory entries (when there is more then one). See libpayload/include/coreboot_tables.h which introduces for this reason struct cbuint64 which is 64-bit datatype split to 2 32-bit parts to fix this oversight.
Have a look what multiboot2 folks did. In general multiboot2 [1] got this "right". It aligns the start of the entries and I think there are no such issues. Also it defines the machine state at the point of handoff which is nice. Also, it has some infrastructure to pass various strange future memory lists in the memory tag.
Thanks, Rudolf
[1] https://www.gnu.org/software/grub/manual/multiboot2/multiboot.html
Kind regards Arthur
On Tue, Apr 19, 2022 at 10:44 AM Rudolf Marek <r.marek@assembler.cz mailto:r.marek@assembler.cz> wrote:
Dne 12. 04. 22 v 0:04 Peter Stuge napsal(a): > I propose that coreboot tables are a good alternative - fight me! :) Challenge accepted. They aren't because they are defined with ABI/compiler: - 64-bit data type alignment is different in 32-bit ABI (4 bytes) and different in 64-bit ABI (8 bytes). Not even sure how the other ABIs got this. - CB structures like framebuffer struct are not padded to the size compiler likes. The "packed" attribute would be bit dangerous because then compiler might complain like "error: taking address of packed member of 'struct cb_framebuffer' may result in an unaligned pointer value [-Werror=address-of-packed-member]" Alternatively one could add "pad[2]" member to the end to the fb structure to fix this, but maybe this will break some payloads which don't check the ->size of specific tag... Anyway, hope this illustrates the problem a bit. Thanks, Rudolf _______________________________________________ coreboot mailing list -- coreboot@coreboot.org <mailto:coreboot@coreboot.org> To unsubscribe send an email to coreboot-leave@coreboot.org <mailto:coreboot-leave@coreboot.org>