On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark robdclark@gmail.com wrote:
On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 08/05/2017 04:35 PM, Rob Clark wrote:
On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis mark.kettenis@xs4all.nl wrote:
Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST) From: Mark Kettenis mark.kettenis@xs4all.nl
Unfortunately something in this patch series breaks things for me on a Banana Pi:
And according to git bisect:
4e3e748a50fc3f43e20c7ff407184596d7c9a589 is the first bad commit commit 4e3e748a50fc3f43e20c7ff407184596d7c9a589 Author: Peter Jones pjones@redhat.com Date: Wed Jun 21 16:39:02 2017 -0400
efi: add some more device path structures Signed-off-by: Peter Jones <pjones@redhat.com> Signed-off-by: Rob Clark <robdclark@gmail.com>hmm, odd.. it is only adding some #define's and structs that are not used until a later commit..
although it does also make 'struct efi_device_path_mac_addr' __packed, which it should have been before. Is this an armv7? I wonder if we have some troubles with unaligned accesses on armv7 that we don't have on aarch64 (or maybe compiler isn't turning access to device-path nodes into byte accesses if it can't do unaligned accesses. (The node in the device-path structure are byte-packed.)
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.htm...
<cite>On older processors, such as ARM9 family based processors, an unaligned load had to be synthesised in software. Typically by doing a series of small accesses, and combining the results. ... Unaligned access support must be enabled by setting the SCTLR.A bit in the system control coprocessor</cite>
Generally packing structures is not a good idea performance-wise. The sequence of fields should be carefully chosen to fill up to powers of two (2, 4 , 8).
Yeah, it was clearly a dumb idea for UEFI to not make device-path nodes word aligned. But when implementing a standard, we don't have a choice but to implement it properly, warts and all :-/
btw, just for reference (if anyone is curious), see sect 10.3.1 in UEFI spec v2.7. If you don't want to bother looking it up, here is the exact wording:
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.
So clearly what we were doing before was incorrect.. but cheating w/ extra padding bytes on arch's that cannot handle unaligned accesses will avoid having to go fix grub, bootaa64/shim/fallback (and anything else that uses gnu-efi), and apparently openbsd's bootaa64.efi. It is kinda weird to be using efi on these arch's in the first place, so I guess I don't feel as badly about the padding bytes hack on those arch's. (But we should not use the hack on aarch64.)
BR, -R