On Sat, Aug 5, 2017 at 11:24 AM, Rob Clark <[email protected]> wrote: > On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt <[email protected]> > wrote: >> On 08/05/2017 04:35 PM, Rob Clark wrote: >>> On Sat, Aug 5, 2017 at 10:28 AM, Mark Kettenis <[email protected]> >>> wrote: >>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST) >>>>> From: Mark Kettenis <[email protected]> >>>>> >>>>> 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 <[email protected]> >>>> Date: Wed Jun 21 16:39:02 2017 -0400 >>>> >>>> efi: add some more device path structures >>>> >>>> Signed-off-by: Peter Jones <[email protected]> >>>> Signed-off-by: Rob Clark <[email protected]> >>> >>> >>> 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.html >> >> <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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
