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 <robdcl...@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)); > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot