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

[Alexander Graf]

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.

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.


Alex