| Message ID | 20230128045615.8216-1-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] efi: use 32-bit alignment for efi_guid_t | expand |
On Sat, 28 Jan 2023 at 06:56, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > Current U-Boot implements 64-bit boundary for efi_guid_t structure. > It follows the UEFI specification, page 21 of the UEFI Specification v2.10 > says about EFI_GUID: > 128-bit buffer containing a unique identifier value. Unless > otherwise specified, aligned on a 64-bit boundary. > > On the other hand, page 163 of the UEFI specification v2.10 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-bit not 64-bit like U-Boot efi_guid_t. > > Due to this alignment difference, EDK2 application "CapsuleApp.efi -P" > does not work as expected. > This calls EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo() > and dump the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure, > offsetof(EFI_FIRMWARE_IMAGE_DESCRIPTOR, ImageTypeId) is different, > 8 in U-Boot and 4 in EDK2(CapsuleApp.efi). > Here is the wrong EFI_GUID dump. > wrong dump : ImageTypeId - 00000000-7D83-058B-D550-474CA19560D8 > expected : ImageTypeId - 058B7D83-50D5-4C47-A195-60D86AD341C4 > > EFI_FIRMWARE_IMAGE_DESCRIPTOR structure is defined in UEFI specification: > typedef struct { > UINT8 ImageIndex; > EFI_GUID ImageTypeId; > UINT64 ImageId > <snip> > > } EFI_FIRMWARE_IMAGE_DESCRIPTOR; > > There was the relevant patch for linux kernel to use 32-bit alignment > for efi_guid_t [1]. > U-Boot should get aligned to EDK2 reference implementation and > linux kernel. > > Due to this alignment change, efi_hii_ref structure in include/efi_api.h > is affected, but it is not used in the current U-Boot code. > > [1] https://lore.kernel.org/all/20190202094119.13230-5-ard.biesheuvel@linaro.org/ > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > --- > Changes in v2: > - add source code comment to describe why the efi_guid_t structure is > 4-byte aligned > > include/efi.h | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/include/efi.h b/include/efi.h > index 42f4e58a91..c3087d3da2 100644 > --- a/include/efi.h > +++ b/include/efi.h > @@ -54,9 +54,18 @@ > > struct efi_device_path; > > +/* > + * The EFI spec defines the EFI_GUID as > + * "128-bit buffer containing a unique identifier value. Unless otherwise specified, > + * aligned on a 64-bit boundary". > + * Page 163 of the UEFI specification v2.10 and > + * EDK2 reference implementation both define EFI_GUID as > + * struct { u32 a; u16; b; u16 c; u8 d[8]; }; which is 4-byte > + * aligned. > + */ > typedef struct { > u8 b[16]; > -} efi_guid_t __attribute__((aligned(8))); > +} efi_guid_t __attribute__((aligned(4))); > > #define EFI_BITS_PER_LONG (sizeof(long) * 8) > > -- > 2.17.1 > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff --git a/include/efi.h b/include/efi.h index 42f4e58a91..c3087d3da2 100644 --- a/include/efi.h +++ b/include/efi.h @@ -54,9 +54,18 @@ struct efi_device_path; +/* + * The EFI spec defines the EFI_GUID as + * "128-bit buffer containing a unique identifier value. Unless otherwise specified, + * aligned on a 64-bit boundary". + * Page 163 of the UEFI specification v2.10 and + * EDK2 reference implementation both define EFI_GUID as + * struct { u32 a; u16; b; u16 c; u8 d[8]; }; which is 4-byte + * aligned. + */ typedef struct { u8 b[16]; -} efi_guid_t __attribute__((aligned(8))); +} efi_guid_t __attribute__((aligned(4))); #define EFI_BITS_PER_LONG (sizeof(long) * 8)
Current U-Boot implements 64-bit boundary for efi_guid_t structure. It follows the UEFI specification, page 21 of the UEFI Specification v2.10 says about EFI_GUID: 128-bit buffer containing a unique identifier value. Unless otherwise specified, aligned on a 64-bit boundary. On the other hand, page 163 of the UEFI specification v2.10 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-bit not 64-bit like U-Boot efi_guid_t. Due to this alignment difference, EDK2 application "CapsuleApp.efi -P" does not work as expected. This calls EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo() and dump the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure, offsetof(EFI_FIRMWARE_IMAGE_DESCRIPTOR, ImageTypeId) is different, 8 in U-Boot and 4 in EDK2(CapsuleApp.efi). Here is the wrong EFI_GUID dump. wrong dump : ImageTypeId - 00000000-7D83-058B-D550-474CA19560D8 expected : ImageTypeId - 058B7D83-50D5-4C47-A195-60D86AD341C4 EFI_FIRMWARE_IMAGE_DESCRIPTOR structure is defined in UEFI specification: typedef struct { UINT8 ImageIndex; EFI_GUID ImageTypeId; UINT64 ImageId <snip> } EFI_FIRMWARE_IMAGE_DESCRIPTOR; There was the relevant patch for linux kernel to use 32-bit alignment for efi_guid_t [1]. U-Boot should get aligned to EDK2 reference implementation and linux kernel. Due to this alignment change, efi_hii_ref structure in include/efi_api.h is affected, but it is not used in the current U-Boot code. [1] https://lore.kernel.org/all/20190202094119.13230-5-ard.biesheuvel@linaro.org/ Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- Changes in v2: - add source code comment to describe why the efi_guid_t structure is 4-byte aligned include/efi.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)