[U-Boot] [PATCH v0 21/20] efi_loader: hack for archs that cannot do unaligned accesses

[Leif Lindholm]

On Tue, Aug 08, 2017 at 08:01:10AM -0400, Rob Clark wrote:
> On Tue, Aug 8, 2017 at 7:32 AM, Leif Lindholm <leif.lindholm at linaro.org> wrote:
> > On Tue, Aug 08, 2017 at 09:11:14AM +0100, Ard Biesheuvel wrote:
> >> On 8 August 2017 at 07:52, Alexander Graf <agraf at suse.de> wrote:
> >> >> Am 07.08.2017 um 23:18 schrieb Rob Clark <robdclark at gmail.com>:
> >> >>
> >> >> This is problematic around file nodes in the device path.  Adding the
> >> >> padding bytes to the end of each device-path struct would "solve"
> >> >> that, and if pre-aarch64 we are aiming at "good enough to work", I
> >> >> kinda think that this the approach we should go for.  Other than
> >> >> file-path nodes, the rest of the issues in u-boot should be solved by
> >> >> addition of missing __packed on 'struct efi_device_path' (which I've
> >> >> added locally and will be in the next revision of the patchset).
> >> >
> >> > Let's ask Leif and Ard if that is actually correct. If I remember
> >> > correctly, edk2 some times leverages automatic padding from the
> >> > compiler on structs.
> >>
> >> I guess that that might work, if U-boot is the only agent
> >> instantiating device path structures. But what do you mean by
> >> 'correct' in this context?
> >
> > Well, if that is the case, are we risking the ability to in the future
> > support loading drivers or protocols at runtime. (This would for
> > example exclude Shim compatibility or running the UEFI Shell.)
> 
> My proposal (and this is only for <=armv6 and armv7 until someone gets
> around to enabling mmu and disabling alignment faults) is to add
> padding bytes at the end of the various device-path structs to at
> least keep the structs (and things like utf16 string in file-path
> struct) aligned, and rely on efi payload and u-boot to be compiled
> with -mno-unaligned-access if it needs to access fields within the
> device-path structs.
> 
> This is *essentially* what u-boot does implicitly at the moment (by
> missing __packed attribute on certain structs).  I want to fix that on
> aarch64, but without the padding bytes it causes a some unaligned
> accesses in u-boot on armv7 devices.
>
> I think the goal for armv7 is more to have enough uefi for grub and
> OpenBSD's bootloader.  If fancy things like loading drivers and
> protocols at runtime doesn't work, well these didn't work before so I
> won't loose much sleep.  But I would like that to work properly on
> aarch64.

I'm all for the just enough approach (I just keep hoping for feature
creep). If this means certain aspects will not be supportable, what I
want is for it to be not supportable in a predictable manner.

So I guess what I'd like is that if we do this, then we either turn
the efi_*install_* functions back into just returning
EFI_OUT_OF_RESOURCES on these platforms, or worst case make them
scream bloody murder (but progress and hope for the best - like [1]).

[1] https://lists.denx.de/pipermail/u-boot/2016-January/241454.html

/
    Leif