On Tue, Jan 08, 2019 at 04:28:29PM +0100, Ard Biesheuvel wrote:
> The UEFI spec and EDK2 reference implementation both define EFI_GUID as
> struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
> is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
> this means that firmware services invoked by the kernel may assume that
> efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
> do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
>
> Note that the UEFI spec as well as some comments in the EDK2 code base
> suggest that EFI_GUID should be 64-bit aligned, but this appears to be
> a mistake, given that no code seems to exist that actually enforces that
> or relies on it.
Whereas code does exist that relies on it being 32-bit aligned...
> Reported-by: Heinrich Schuchardt <xypron.glpk@xxxxxx>,
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Reviewed-by: Leif Lindholm <leif.lindholm@xxxxxxxxxx>
> ---
> include/linux/efi.h | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 45ff763fba76..be08518c2553 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -48,7 +48,20 @@ typedef u16 efi_char16_t; /* UNICODE character */
> typedef u64 efi_physical_addr_t;
> typedef void *efi_handle_t;
>
> -typedef guid_t efi_guid_t;
> +/*
> + * The UEFI spec and EDK2 reference implementation both define EFI_GUID as
> + * struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
> + * is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
> + * this means that firmware services invoked by the kernel may assume that
> + * efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
> + * do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
> + *
> + * Note that the UEFI spec as well as some comments in the EDK2 code base
> + * suggest that EFI_GUID should be 64-bit aligned, but this appears to be
> + * a mistake, given that no code seems to exist that actually enforces that
> + * or relies on it.
> + */
> +typedef guid_t efi_guid_t __aligned(__alignof__(u32));
>
> #define EFI_GUID(a,b,c,d0,d1,d2,d3,d4,d5,d6,d7) \
> GUID_INIT(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)
> --
> 2.20.1
>
