| Message ID | 20170807121934.25735-1-robdclark@gmail.com |
|---|---|
| State | Deferred |
| Delegated to: | Alexander Graf |
| Headers | show |
> Am 07.08.2017 um 13:19 schrieb Rob Clark <robdclark@gmail.com>: > > Device-path structures in UEFI are only byte aligned, which can result > in unaligned access faults (either in u-boot or the efi payload which is > loaded). From the UEFI spec (sect 10.3.1 in UEFI spec v2.7): > > A Device Path is a series of generic Device Path nodes. The first > Device Path node starts at byte offset zero of the Device Path. > The next Device Path node starts at the end of the previous Device > Path node. Therefore all nodes are byte-packed data structures that > may appear on any byte boundary. All code references to device path > notes must assume all fields are unaligned. Since every Device Path > node contains a length field in a known place, it is possible to > traverse Device Path nodes that are of an unknown type. There is > no limit to the number, type, or sequence of nodes in a Device Path. > > This isn't a matter of "just fix u-boot", it is baked into the spec. > Not enabling alignment faults is consistent with what TianoCore edk2 > does. > > For armv6 and earlier, we probably still need hacks to pad the device- > path nodes, which isn't quite in line with the spec, and sanitize > device-paths passed in from the efi payload. But we can at least dtrt > on armv7 (and aarch64 which already doesn't enable alignment faults). > > Probably we can skip clearing the bit when EFI_LOADER is enabled, since > '0' is the reset value. But I guess safest to clear it just in case an > early stage in the boot chain set it. > > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > Only tested in qemu, and it is unclear if alignment faults are even > trapped in qemu. If someone wants to test, then try (on top of the > "enough UEFI for standard distro boot" patchset[1]) either fallback.efi > (which uses gnu-efi lib to parse device-paths to string) or any efi > payload that uses the device-path-to-text protocol. Either of those > should trigger unaligned accesses. Grub's lsefi command should also > trigger unaligned faults. It's unfortunately much harder. To have working hardware alignment fixups, you need to enable the MMU as well. That's why I rewrote the MMU management for AArch64 back then. The reason it worked out so far for me at least is that grub on 32bit as well as u-boot are compiled with unaligned checks. Alex > > arch/arm/cpu/armv7/start.S | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S > index f06fd28940..c1cec30af6 100644 > --- a/arch/arm/cpu/armv7/start.S > +++ b/arch/arm/cpu/armv7/start.S > @@ -149,7 +149,11 @@ ENTRY(cpu_init_cp15) > mrc p15, 0, r0, c1, c0, 0 > bic r0, r0, #0x00002000 @ clear bits 13 (--V-) > bic r0, r0, #0x00000007 @ clear bits 2:0 (-CAM) > +#ifdef EFI_LOADER > + bic r0, r0, #0x00000002 @ clear bit 1 (--A-) Align > +#else > orr r0, r0, #0x00000002 @ set bit 1 (--A-) Align > +#endif > orr r0, r0, #0x00000800 @ set bit 11 (Z---) BTB > #ifdef CONFIG_SYS_ICACHE_OFF > bic r0, r0, #0x00001000 @ clear bit 12 (I) I-cache > -- > 2.13.0 >
On Mon, Aug 7, 2017 at 9:16 AM, Alexander Graf <agraf@suse.de> wrote: > > >> Am 07.08.2017 um 13:19 schrieb Rob Clark <robdclark@gmail.com>: >> >> Device-path structures in UEFI are only byte aligned, which can result >> in unaligned access faults (either in u-boot or the efi payload which is >> loaded). From the UEFI spec (sect 10.3.1 in UEFI spec v2.7): >> >> A Device Path is a series of generic Device Path nodes. The first >> Device Path node starts at byte offset zero of the Device Path. >> The next Device Path node starts at the end of the previous Device >> Path node. Therefore all nodes are byte-packed data structures that >> may appear on any byte boundary. All code references to device path >> notes must assume all fields are unaligned. Since every Device Path >> node contains a length field in a known place, it is possible to >> traverse Device Path nodes that are of an unknown type. There is >> no limit to the number, type, or sequence of nodes in a Device Path. >> >> This isn't a matter of "just fix u-boot", it is baked into the spec. >> Not enabling alignment faults is consistent with what TianoCore edk2 >> does. >> >> For armv6 and earlier, we probably still need hacks to pad the device- >> path nodes, which isn't quite in line with the spec, and sanitize >> device-paths passed in from the efi payload. But we can at least dtrt >> on armv7 (and aarch64 which already doesn't enable alignment faults). >> >> Probably we can skip clearing the bit when EFI_LOADER is enabled, since >> '0' is the reset value. But I guess safest to clear it just in case an >> early stage in the boot chain set it. >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> >> --- >> Only tested in qemu, and it is unclear if alignment faults are even >> trapped in qemu. If someone wants to test, then try (on top of the >> "enough UEFI for standard distro boot" patchset[1]) either fallback.efi >> (which uses gnu-efi lib to parse device-paths to string) or any efi >> payload that uses the device-path-to-text protocol. Either of those >> should trigger unaligned accesses. Grub's lsefi command should also >> trigger unaligned faults. > > It's unfortunately much harder. To have working hardware alignment fixups, you need to enable the MMU as well. That's why I rewrote the MMU management for AArch64 back then. > > The reason it worked out so far for me at least is that grub on 32bit as well as u-boot are compiled with unaligned checks. > hmm, sadness.. well then, back to -DBROKEN_UNALIGNED for armv7 then until someone enables mmu.. BR, -R > > Alex > >> >> arch/arm/cpu/armv7/start.S | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S >> index f06fd28940..c1cec30af6 100644 >> --- a/arch/arm/cpu/armv7/start.S >> +++ b/arch/arm/cpu/armv7/start.S >> @@ -149,7 +149,11 @@ ENTRY(cpu_init_cp15) >> mrc p15, 0, r0, c1, c0, 0 >> bic r0, r0, #0x00002000 @ clear bits 13 (--V-) >> bic r0, r0, #0x00000007 @ clear bits 2:0 (-CAM) >> +#ifdef EFI_LOADER >> + bic r0, r0, #0x00000002 @ clear bit 1 (--A-) Align >> +#else >> orr r0, r0, #0x00000002 @ set bit 1 (--A-) Align >> +#endif >> orr r0, r0, #0x00000800 @ set bit 11 (Z---) BTB >> #ifdef CONFIG_SYS_ICACHE_OFF >> bic r0, r0, #0x00001000 @ clear bit 12 (I) I-cache >> -- >> 2.13.0 >> >
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index f06fd28940..c1cec30af6 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -149,7 +149,11 @@ ENTRY(cpu_init_cp15) mrc p15, 0, r0, c1, c0, 0 bic r0, r0, #0x00002000 @ clear bits 13 (--V-) bic r0, r0, #0x00000007 @ clear bits 2:0 (-CAM) +#ifdef EFI_LOADER + bic r0, r0, #0x00000002 @ clear bit 1 (--A-) Align +#else orr r0, r0, #0x00000002 @ set bit 1 (--A-) Align +#endif orr r0, r0, #0x00000800 @ set bit 11 (Z---) BTB #ifdef CONFIG_SYS_ICACHE_OFF bic r0, r0, #0x00001000 @ clear bit 12 (I) I-cache
Device-path structures in UEFI are only byte aligned, which can result in unaligned access faults (either in u-boot or the efi payload which is loaded). From the UEFI spec (sect 10.3.1 in UEFI spec v2.7): A Device Path is a series of generic Device Path nodes. The first Device Path node starts at byte offset zero of the Device Path. The next Device Path node starts at the end of the previous Device Path node. Therefore all nodes are byte-packed data structures that may appear on any byte boundary. All code references to device path notes must assume all fields are unaligned. Since every Device Path node contains a length field in a known place, it is possible to traverse Device Path nodes that are of an unknown type. There is no limit to the number, type, or sequence of nodes in a Device Path. This isn't a matter of "just fix u-boot", it is baked into the spec. Not enabling alignment faults is consistent with what TianoCore edk2 does. For armv6 and earlier, we probably still need hacks to pad the device- path nodes, which isn't quite in line with the spec, and sanitize device-paths passed in from the efi payload. But we can at least dtrt on armv7 (and aarch64 which already doesn't enable alignment faults). Probably we can skip clearing the bit when EFI_LOADER is enabled, since '0' is the reset value. But I guess safest to clear it just in case an early stage in the boot chain set it. Signed-off-by: Rob Clark <robdclark@gmail.com> --- Only tested in qemu, and it is unclear if alignment faults are even trapped in qemu. If someone wants to test, then try (on top of the "enough UEFI for standard distro boot" patchset[1]) either fallback.efi (which uses gnu-efi lib to parse device-paths to string) or any efi payload that uses the device-path-to-text protocol. Either of those should trigger unaligned accesses. Grub's lsefi command should also trigger unaligned faults. arch/arm/cpu/armv7/start.S | 4 ++++ 1 file changed, 4 insertions(+)