Messages in this thread | | | Date | Wed, 21 Feb 2024 18:10:43 -0800 | Subject | Re: [PATCH v8 04/15] x86: Secure Launch Resource Table header file | From | ross.philipson@oracle ... |
| |
On 2/21/24 6:03 PM, 'Andrew Cooper' via trenchboot-devel wrote: > On 15/02/2024 8:08 am, Ard Biesheuvel wrote: >> On Wed, 14 Feb 2024 at 23:31, Ross Philipson <ross.philipson@oracle.com> wrote: >>> +/* >>> + * Primary SLR Table Header > > I know it's just a comment, but SLR ought to be written in longhand here.
Will do, thanks. Ross.
> >>> + */ >>> +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 >
| |