[U-Boot] [PATCH v2 3/4] efi_loader: use efi_start_image() for bootefi

[Heinrich Schuchardt]

On 7/19/19 1:07 AM, Mark Kettenis wrote:
>> From: Alexander Graf <agraf at csgraf.de>
>> Date: Thu, 18 Jul 2019 20:50:39 +0200
>>
>> On 18.07.19 19:33, Heinrich Schuchardt wrote:
>>> On 7/18/19 11:16 AM, Jonathan Gray wrote:
>>>> On Thu, Jul 18, 2019 at 10:39:57AM +0200, Mark Kettenis wrote:
>>>>>> Date: Thu, 18 Jul 2019 16:00:16 +1000
>>>>>> From: Jonathan Gray <jsg at jsg.id.au>
>>>>>>
>>>>>> On Fri, Feb 08, 2019 at 07:46:49PM +0100, Heinrich Schuchardt wrote:
>>>>>>> Remove the duplicate code in efi_do_enter() and use
>>>>>>> efi_start_image() to
>>>>>>> start the image invoked by the bootefi command.
>>>>>>>
>>>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>>>> ---
>>>>>>> v2
>>>>>>>      use EFI_CALL
>>>>>>
>>>>>> This commit broke booting OpenBSD/armv7 kernels on mx6cuboxi with
>>>>>> U-Boot
>>>>>> releases after 2019.01.  2019.04 works if this commit is reverted.
>>>>>> With
>>>>>> 2019.07 there are conflicts trying to revert it and it is still broken
>>>>>> as released.
>>>>>>
>>>>>> f69d63fae281ba98c3d063097cf4e95d17f3754d is the first bad commit
>>>>>> commit f69d63fae281ba98c3d063097cf4e95d17f3754d
>>>>>> Author: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>>> Date:   Wed Dec 26 13:28:09 2018 +0100
>>>>>>
>>>>>>       efi_loader: use efi_start_image() for bootefi
>>>>>>
>>>>>>       Remove the duplicate code in efi_do_enter() and use
>>>>>> efi_start_image() to
>>>>>>       start the image invoked by the bootefi command.
>>>>>>
>>>>>>       Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>>>
>>>>>>    cmd/bootefi.c                 | 22 +---------------------
>>>>>>    include/efi_loader.h          |  4 ++++
>>>>>>    lib/efi_loader/efi_boottime.c |  6 +++---
>>>>>>    3 files changed, 8 insertions(+), 24 deletions(-)
>>>>>
>>>>> Hi Jonathan,
>>>>>
>>>>> With this commit the OpenBSD bootloader (an EFI application) still
>>>>> boots, but the loaded OpenBSD kernel doesn't isn't it?
>>>>
>>>> Yes, when it fails the last thing on serial is:
>>>>
>>>> ## Starting EFI application at 12000000 ...
>>>>>> OpenBSD/armv7 BOOTARM 1.3
>>>> boot>
>>>> booting sd0a:/bsd: 4572484+689312+238360+561608
>>>> [298268+120+314400+278666]=0x0
>>>>
>>>>>
>>>>> I bet the problem here is that efi_start_image() sets
>>>>> efi_is_direct_boot to false, which means that efi_exit_caches() which
>>>>> runs as a result of calling ExitBootServices() no longer
>>>>> flushes/disables the caches on 32-bit ARM.
>>>>
>>>> Indeed, removing 'efi_is_direct_boot = false;' from
>>>> efi_start_image() allows me to boot multiuser on cubox with 2019.07.
>>
>>
>> Heinrich asked me to clarify again in the mail thread.
>>
>> The UEFI spec says that caches on 32bit ARM are supposed to be *enabled*
>> always. That means throughout boot time, as well as after
>> ExitBootServices(). So the behavior with efi_is_direct_boot = false is
>> the expected behavior according to the spec.
>>
>> The reason we have the hack above in the tree is that grub had a nasty
>> deficiency on 32bit ARM where it called into the Linux kernel using the
>> legacy boot protocol which again expected caches to be disabled.
>>
>> The problem you're facing here sounds to me like a bug in the OpenBSD
>> loader that resembles the bug in grub. Instead of calling into a BSD
>> kernel code path that expects caches to already be enabled, it expects
>> to come up with caches disabled.
>
> I don't think we assume that the kernel is called with the
> architecturally defined caches disabled.  As far as I know, OpenBSD
> works fine with a recent U-Boot on Cortex-A7 systems that don't have
> an non-architectural L2 cache.
>
>> You *could* solve this generically by adding functionality akin to
>> clean_up_before_linux() inside the BSD loader. The downside of that is
>> that the code would be platform specific, as L2 caches used to be
>> enabled/disabled via MMIOs and not via architected registers in the old
>> days.
>
> Messing with the non-architectural L2 cache is someting we defenitely
> don't want to do in the OpenBSD bootloader and kernel bootstrap code.
> We have a single bootloader/kernel that works on all supported armv7
> systems!
>
> I quoted UEFI 2.5 upthread, but 2.8 retains the same text in section
> 2.3.5:
>
>    Implementations of boot services will enable architecturally
>    manageable caches and TLBs i.e., those that can be managed directly
>    using CP15 operations using mechanisms and procedures defined in the
>    ARM Architecture Reference Manual. They should not enable caches
>    requiring platform information to manage or invoke non-architectural
>    cache/TLB lockdown mechanisms
>
> So I'd say the problem here is that U-Boot passes control to the EFI
> payload with the L2 cache enabled.
>

Hello Mark,

your last observation concerned an i.MX6 board. Does this processor have
any cache that falls under the cited paragraph? A link to the processor
documentation would be helpful.

Best regards

Heinrich