On 15/02/2024 8:08 am, Ard Biesheuvel wrote: > On Wed, 14 Feb 2024 at 23:31, Ross Philipson <ross.philipson@xxxxxxxxxx> wrote: >> +/* >> + * Primary SLR Table Header I know it's just a comment, but SLR ought to be written in longhand here. >> + */ >> +struct slr_table { >> + u32 magic; >> + u16 revision; >> + u16 architecture; >> + u32 size; >> + u32 max_size; >> + /* entries[] */ >> +} __packed; > Packing this struct has no effect on the layout so better drop the > __packed here. If this table is part of a structure that can appear > misaligned in memory, better to pack the outer struct or deal with it > there in another way. As you note, __packed does two things not one. The consumer of the random integer that is expected to be a pointer to a struct lsr_table doesn't know whether it was invoked by a 16bit bootloader or a 32bit bootloader, and this really does make a difference for an ABI described only in C. Then again, we're holding off on setting the spec in stone until there's an agreement in principle, so we could retrofit a statement about the expected alignment of this structure in memory. The sane choices are either 8b alignment (there are uint64_t's in entires[], but I also see there are some misaligned uint64_t's too, which is dull), or using the good old x86 fallback or paragraph alignment just in case we really want to extend it with a uint128_t in future. Thoughts? ~Andrew _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec