Hi On Thu, 6 Apr 2023 at 09:16, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 4/6/23 01:48, Tom Rini wrote: > > When building with clang we see this warning: > > field guid within 'struct efi_hii_keyboard_layout' is less aligned than > > 'efi_guid_t' and is usually due to 'struct efi_hii_keyboard_layout' being > > packed, which can lead to unaligned accesses [-Wunaligned-access] > > > > Which is correct and true as EFI payloads are by specification allowed > > to do unaligned access. And so to silence the warning here and only > > here, we make use of #pragma to push/pop turning off the warning. > > > > Signed-off-by: Tom Rini <tr...@konsulko.com> > > --- > > Cc: Heinrich Schuchardt <xypron.g...@gmx.de> > > Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org> > > --- > > include/efi_api.h | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/include/efi_api.h b/include/efi_api.h > > index dc6e5ce236c9..f3bcbf593bea 100644 > > --- a/include/efi_api.h > > +++ b/include/efi_api.h > > @@ -1168,9 +1168,21 @@ struct efi_key_descriptor { > > u16 affected_attribute; > > } __packed; > > > > +/* The warniing: > > + * field guid within 'struct efi_hii_keyboard_layout' is less aligned than > > 'efi_guid_t' and is usually due to 'struct efi_hii_keyboard_layout' being > > packed, which can lead to unaligned accesses [-Wunaligned-access] > > + * is intentional due to EFI requiring unaligned access to be supported. > > + */ > > struct efi_hii_keyboard_layout { > > u16 layout_length; > > +#ifdef CONFIG_CC_IS_CLANG > > +#pragma clang diagnostic push > > +#pragma clang diagnostic ignored "-Wunaligned-access" > > +#endif > > efi_guid_t guid; > > +#ifdef CONFIG_CC_IS_CLANG > > +#pragma clang diagnostic pop > > +#endif > > I don't think that the clang warning should be suppressed. We should > replace efi_guid_t by u8[16] instead. This will force us to take the > missing alignment into account when accessing the component in future. > > - efi_guid_t guid; > + u8 guid[16];
I think what Heinrich suggests is fine, the EFI spec itself says that efi_guid must be 8-byte aligned unless otherwise specified. But we have another issue here. We aren't supposed to include structs that contain flexible length arrays into another array or struct. Quoting the C spec [0] Such a structure (and any union containing, possibly recursively, a member that is such a structure) shall not be a member of a structure or an element of an array. IOW efi_hii_keyboard_layout should not include struct efi_key_descriptor descriptors[]; since we include it at efi_hii_keyboard_package Regards /Ilias [0] https://www.dii.uchile.cl/~daespino/files/Iso_C_1999_definition.pdf chapter 6.7.2.1 > > Best regards > > Heinrich > > > + > > u32 layout_descriptor_string_offset; > > u8 descriptor_count; > > struct efi_key_descriptor descriptors[]; >