[U-Boot] [PATCH v0 13/20] efi_loader: use proper device-paths for partitions

[Rob Clark]

On Sat, Aug 5, 2017 at 11:10 AM, Heinrich Schuchardt <xypron.glpk at 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 at xs4all.nl> wrote:
>>>> Date: Sat, 5 Aug 2017 16:01:51 +0200 (CEST)
>>>> From: Mark Kettenis <mark.kettenis at 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 at redhat.com>
>>> Date:   Wed Jun 21 16:39:02 2017 -0400
>>>
>>>     efi: add some more device path structures
>>>
>>>     Signed-off-by: Peter Jones <pjones at redhat.com>
>>>     Signed-off-by: Rob Clark <robdclark at 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.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 :-/

BR,
-R


> Regards
>
> Heinrich
>
>>
>> addr2line the faulting address I guess should confirm that.  If this
>> is the issue, it's going to be a bit sad since we'll have to do a lot
>> of copying back/forth of efi_device_path ptrs to aligned addresses :-/
>>
>> BR,
>> -R
>>
>