public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Rob Clark <robdclark@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v0 21/20] efi_loader: hack for archs that cannot do unaligned accesses
Date: Sat,  5 Aug 2017 11:58:38 -0400	[thread overview]
Message-ID: <20170805155841.17889-1-robdclark@gmail.com> (raw)
In-Reply-To: <20170804193205.24669-1-robdclark@gmail.com>

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
 
 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));
-- 
2.13.0

  parent reply	other threads:[~2017-08-05 15:58 UTC|newest]

Thread overview: 116+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-04 19:31 [U-Boot] [PATCH v0 00/20] enough UEFI for standard distro boot Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 01/20] fs: add fs_readdir() Rob Clark
2017-08-07 18:19   ` Brüns, Stefan
2017-08-07 19:11     ` Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 02/20] fs/fat: implement readdir Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 03/20] short-wchar Rob Clark
2017-08-04 20:28   ` Heinrich Schuchardt
2017-08-04 20:36     ` Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 04/20] part: extract MBR signature from partitions Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 05/20] efi: add some more device path structures Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 06/20] common: add some utf16 handling helpers Rob Clark
2017-08-06  5:17   ` Simon Glass
2017-08-08 22:50   ` [U-Boot] [U-Boot, v0, " Heinrich Schuchardt
2017-08-08 23:21     ` Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 07/20] vsprintf.c: add wide string (%ls) support Rob Clark
2017-08-08 22:03   ` [U-Boot] [U-Boot, v0, " Heinrich Schuchardt
2017-08-08 22:44     ` Rob Clark
2017-08-08 23:08       ` Heinrich Schuchardt
2017-08-08 23:20         ` Alexander Graf
2017-08-08 23:39         ` Rob Clark
2017-08-08 23:55           ` Alexander Graf
2017-08-09  0:14             ` Rob Clark
2017-08-09  1:14               ` Rob Clark
2017-08-09 11:27               ` Tom Rini
2017-08-10  2:16                 ` rick at andestech.com
2017-08-09  8:50           ` Peter Robinson
2017-08-08 22:52     ` Rob Clark
2017-08-09 13:38     ` Rob Clark
2017-08-09 13:48       ` Alexander Graf
2017-08-09 14:35         ` Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 08/20] efi_loader: add back optional efi_handler::open() Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 09/20] efi_loader: add device-path utils Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 10/20] efi_loader: drop redundant efi_device_path_protocol Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 11/20] efi_loader: add guidstr helper Rob Clark
2017-08-05 19:33   ` Heinrich Schuchardt
2017-08-05 19:56     ` Rob Clark
2017-08-05 20:18       ` Heinrich Schuchardt
2017-08-04 19:31 ` [U-Boot] [PATCH v0 12/20] efi_loader: flesh out device-path to text Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 13/20] efi_loader: use proper device-paths for partitions Rob Clark
2017-08-04 20:41   ` Mark Kettenis
2017-08-04 20:57     ` Rob Clark
2017-08-05 14:01       ` Mark Kettenis
2017-08-05 14:19         ` Rob Clark
2017-08-05 14:28         ` Mark Kettenis
2017-08-05 14:35           ` Rob Clark
2017-08-05 15:08             ` Mark Kettenis
2017-08-05 15:22               ` Rob Clark
2017-08-05 16:02                 ` Heinrich Schuchardt
2017-08-05 16:13                   ` Rob Clark
2017-08-05 15:10             ` Heinrich Schuchardt
2017-08-05 15:24               ` Rob Clark
2017-08-05 15:36                 ` Rob Clark
2017-08-06 13:16                   ` Mark Kettenis
2017-08-06 14:17                     ` Rob Clark
2017-08-06 14:26                       ` Rob Clark
2017-08-06 14:28                     ` Mark Kettenis
2017-08-06 14:45                       ` Rob Clark
2017-08-06 15:34                         ` Rob Clark
2017-08-06 16:00                           ` Heinrich Schuchardt
2017-08-06 16:14                           ` Jonathan Gray
2017-08-06 17:28                           ` Mark Kettenis
2017-08-06 17:49                             ` Rob Clark
2017-08-06 18:13                               ` Peter Robinson
2017-08-06 18:21                               ` Mark Kettenis
2017-08-06 18:37                                 ` Mark Kettenis
2017-08-06 18:47                                   ` Rob Clark
2017-08-06 18:53                                     ` Rob Clark
2017-08-06 18:41                                 ` Rob Clark
2017-08-07 15:47                           ` Jonathan Gray
2017-08-07 16:16                             ` Rob Clark
2017-08-08  1:36                               ` Jonathan Gray
2017-08-05 14:28         ` Rob Clark
2017-08-06 12:53           ` Mark Kettenis
2017-08-07 17:32     ` Peter Jones
2017-08-04 19:31 ` [U-Boot] [PATCH v0 14/20] efi_loader: use proper device-paths for net Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 15/20] efi_loader: refactor boot device and loaded_image handling Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 16/20] efi_loader: add file/filesys support Rob Clark
2017-08-04 19:31 ` [U-Boot] [PATCH v0 17/20] efi_loader: support load_image() from a file-path Rob Clark
2017-08-04 19:32 ` [U-Boot] [PATCH v0 18/20] efi_loader: make pool allocations cacheline aligned Rob Clark
2017-08-04 19:32 ` [U-Boot] [PATCH v0 19/20] efi_loader: efi variable support Rob Clark
2017-08-06  5:17   ` Simon Glass
2017-08-04 19:32 ` [U-Boot] [PATCH v0 20/20] efi_loader: add bootmgr Rob Clark
2017-08-04 20:06   ` Heinrich Schuchardt
2017-08-04 20:28     ` Rob Clark
2017-08-04 20:29       ` Rob Clark
2017-08-04 22:46       ` Peter Jones
2017-08-05 15:58 ` Rob Clark [this message]
2017-08-05 16:12   ` [U-Boot] [PATCH v0 21/20] efi_loader: hack for archs that cannot do unaligned accesses Heinrich Schuchardt
2017-08-05 16:16     ` Rob Clark
2017-08-05 16:19       ` Rob Clark
2017-08-05 16:26         ` Heinrich Schuchardt
2017-08-05 16:46           ` Rob Clark
2017-08-05 16:52       ` Heinrich Schuchardt
2017-08-05 17:06         ` Rob Clark
2017-08-05 18:43           ` Rob Clark
2017-08-05 20:05             ` Heinrich Schuchardt
2017-08-05 20:31               ` Rob Clark
2017-08-07 20:19                 ` Alexander Graf
2017-08-07 21:14                   ` Mark Kettenis
2017-08-07 22:18                     ` Rob Clark
2017-08-08  6:52                       ` Alexander Graf
2017-08-08  8:11                         ` Ard Biesheuvel
2017-08-08 11:32                           ` Leif Lindholm
2017-08-08 12:01                             ` Rob Clark
2017-08-08 12:39                               ` Leif Lindholm
2017-08-08  9:27                         ` Rob Clark
2017-08-08 12:26                       ` Mark Kettenis
2017-08-08 12:54                         ` Rob Clark
2017-08-08 13:33                           ` Mark Kettenis
2017-08-08 14:03                             ` Rob Clark
2017-08-08 14:10                               ` Rob Clark
2017-08-08 18:20                               ` Rob Clark
2017-08-07 16:56           ` Rob Clark
2017-08-07 17:15         ` Peter Jones
2017-08-10  1:32 ` [U-Boot] [PATCH v0 00/20] enough UEFI for standard distro boot Tom Rini
2017-08-10 10:41   ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170805155841.17889-1-robdclark@gmail.com \
    --to=robdclark@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox