On 08/05/2017 05:58 PM, Rob Clark wrote:
Some arch's have trouble with unaligned accesses. Technically EFI device-path structs should be byte aligned, and the next node in the path starts immediately after the previous. Meaning that a pointer to an 'struct efi_device_path' is not necessarily word aligned. See section 10.3.1 in v2.7 of UEFI spec.
This causes problems not just for u-boot, but also most/all EFI payloads loaded by u-boot on these archs. Fortunately the common practice for traversing a device path is to rely on the length field in the header, rather than the specified length of the particular device path type+subtype. So the EFI_DP_PAD() macro will add the specified number of bytes to the tail of device path structs to pad them to word alignment.
Technically this is non-compliant, BROKEN_UNALIGNED should *only* be defined on archs that cannot do unaligned accesses.
Signed-off-by: Rob Clark robdclark@gmail.com
I'm not sure if there are other arch's that need -DBROKEN_UNALIGNED
Mark, this is untested but I think it should solve your crash on the Banana Pi. Could you give it a try when you get a chance?
arch/arm/config.mk | 2 +- include/efi_api.h | 30 ++++++++++++++++++++++++++++++ lib/efi_loader/efi_device_path.c | 3 +++ 3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 1a77779db4..067dc93a9d 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -28,7 +28,7 @@ LLVMS_RELFLAGS := $(call cc-option,-mllvm,) \ $(call cc-option,-arm-use-movt=0,) PLATFORM_RELFLAGS += $(LLVM_RELFLAGS)
-PLATFORM_CPPFLAGS += -D__ARM__ +PLATFORM_CPPFLAGS += -D__ARM__ -DBROKEN_UNALIGNED
NAK
We have more then ARM. And other architectures also create exceptions for unaligned access.
I hate platform specific code. It should not be used outside /arch.
To play it save you should not use _packed at all! Use memcpy to transfer between aligned and unaligned memory.
Best regards
Heinrich
ifdef CONFIG_ARM64 PLATFORM_ELFFLAGS += -B aarch64 -O elf64-littleaarch64 diff --git a/include/efi_api.h b/include/efi_api.h index ef91e34c7b..ddd1e6100a 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -284,6 +284,31 @@ struct efi_loaded_image { #define DEVICE_PATH_TYPE_END 0x7f # define DEVICE_PATH_SUB_TYPE_END 0xff
+/*
- Some arch's have trouble with unaligned accesses. Technically
- EFI device-path structs should be byte aligned, and the next node
- in the path starts immediately after the previous. Meaning that
- a pointer to an 'struct efi_device_path' is not necessarily word
- aligned. See section 10.3.1 in v2.7 of UEFI spec.
- This causes problems not just for u-boot, but also most/all EFI
- payloads loaded by u-boot on these archs. Fortunately the common
- practice for traversing a device path is to rely on the length
- field in the header, rather than the specified length of the
- particular device path type+subtype. So the EFI_DP_PAD() macro
- will add the specified number of bytes to the tail of device path
- structs to pad them to word alignment.
- Technically this is non-compliant, BROKEN_UNALIGNED should *only*
- be defined on archs that cannot do unaligned accesses.
- */
+#ifdef BROKEN_UNALIGNED +# define EFI_DP_PAD(n) u8 __pad[n] +#else +# define EFI_DP_PAD(n) +#endif
struct efi_device_path { u8 type; u8 sub_type; @@ -318,12 +343,14 @@ struct efi_device_path_usb { struct efi_device_path dp; u8 parent_port_number; u8 usb_interface;
- EFI_DP_PAD(2);
} __packed;
struct efi_device_path_mac_addr { struct efi_device_path dp; struct efi_mac_addr mac; u8 if_type;
- EFI_DP_PAD(3);
} __packed;
struct efi_device_path_usb_class { @@ -333,11 +360,13 @@ struct efi_device_path_usb_class { u8 device_class; u8 device_subclass; u8 device_protocol;
- EFI_DP_PAD(1);
} __packed;
struct efi_device_path_sd_mmc_path { struct efi_device_path dp; u8 slot_number;
- EFI_DP_PAD(3);
} __packed;
#define DEVICE_PATH_TYPE_MEDIA_DEVICE 0x04 @@ -353,6 +382,7 @@ struct efi_device_path_hard_drive_path { u8 partition_signature[16]; u8 partmap_type; u8 signature_type;
- EFI_DP_PAD(1);
} __packed;
struct efi_device_path_cdrom_path { diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index b5acf73f98..515a1f4737 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -402,6 +402,9 @@ struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part,
// TODO efi_device_path_file_path should be variable length: fpsize = sizeof(struct efi_device_path) + 2 * (strlen(path) + 1); +#ifdef BROKEN_UNALIGNED
- fpsize = ALIGN(fpsize, 4);
+#endif dpsize += fpsize;
start = buf = calloc(1, dpsize + sizeof(END));