[U-Boot] [PATCH 1/1] efi_loader: re-enable GRUB workaround on 32bit ARM

[Heinrich Schuchardt]

On 7/27/19 11:58 AM, Alexander Graf wrote:
>
> On 27.07.19 10:02, Heinrich Schuchardt wrote:
>> GRUB on ARM 32bit prior to version 2.04 lacks proper handling of caches.
>> In U-Boot v2019.04 a workaround for this was inadvertently removed.
>>
>> The workaround is currently also needed for booting on systems with
>> caches
>> that cannot be managed via CP15 (e.g. with an i.MX6 CPU).
>>
>> Re-enable the workaround and make it customizable.
>>
>> Fixes: f69d63fae281 ("efi_loader: use efi_start_image() for bootefi")
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>   lib/efi_loader/Kconfig        |  8 ++++++++
>>   lib/efi_loader/efi_boottime.c | 28 +++++++++++++---------------
>>   2 files changed, 21 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index a7f2c68fa9..c7027a9676 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -107,4 +107,12 @@ config EFI_HAVE_RUNTIME_RESET
>>       default y
>>       depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET ||
>> SYSRESET_X86
>>
>> +config EFI_GRUB_ARM32_WORKAROUND
>> +    bool "Workaround for GRUB on 32bit ARM"
>> +    default y
>> +    depends on ARM && !ARM64
>> +    help
>> +      GRUB prior to version 2.04 requires U-Boot to disable caches. This
>> +      workaround currently is also needed on systems with caches that
>> +      cannot be managed via CP15.
>>   endif
>> diff --git a/lib/efi_loader/efi_boottime.c
>> b/lib/efi_loader/efi_boottime.c
>> index 4f6e8d1679..f75ca1a67c 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -39,14 +39,6 @@ LIST_HEAD(efi_register_notify_events);
>>   /* Handle of the currently executing image */
>>   static efi_handle_t current_image;
>>
>> -/*
>> - * If we're running on nasty systems (32bit ARM booting into non-EFI
>> Linux)
>> - * we need to do trickery with caches. Since we don't want to break
>> the EFI
>> - * aware boot path, only apply hacks when loading exiting directly
>> (breaking
>> - * direct Linux EFI booting along the way - oh well).
>> - */
>> -static bool efi_is_direct_boot = true;
>
>
> By removing the toggling, you now couple a U-Boot configuration variant
> to a specific grub version. New grub versions will run Linux using the
> EFI stub. So enabling CONFIG_EFI_GRUB_ARM32_WORKAROUND means only old
> versions work, but new grub versions break.

No, the Wandboard boots fine with GRUB 2.04 and Linux 4.19.55. Do you
have any negative test results?

>
> We really should treat this like any other erratum and make it as
> runtime detected as possible, so that we don't block people from making
> forward progress towards a the "sane" boot path.

It is not possible to determine at runtime which assumptions a loaded
EFI binary makes about the cache. Your original logic counting the
number of StartImage() invocations cannot detect this. This is why I
added a customizing option.

>
> Btw, has there been any solution for the non-CP15 caches? They would
> still be broken with this applied, as payloads can rely on CP15 caches
> enabled (for unaligned accesses), but non-CP15 caches disabled, right?

No, this will need future work. With CONFIG_EFI_GRUB_ARM32_WORKAROUND=y
all caches are disabled.

On i.MX6 CONFIG_SYS_L2CACHE_OFF=y selectively disables the non-CP15
cache. So here CONFIG_EFI_GRUB_ARM32_WORKAROUND=n and
CONFIG_SYS_L2CACHE_OFF=y result in compliance with the UEFI spec.

Best regards

Heinrich

>
>
> Thanks,
>
> Alex
>
>
>> -
>>   #ifdef CONFIG_ARM
>>   /*
>>    * The "gd" pointer lives in a register on ARM and AArch64 that we
>> declare
>> @@ -1911,13 +1903,21 @@ error:
>>    */
>>   static void efi_exit_caches(void)
>>   {
>> -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>> +#if defined(CONFIG_EFI_GRUB_ARM32_WORKAROUND)
>>       /*
>> -     * Grub on 32bit ARM needs to have caches disabled before jumping
>> into
>> -     * a zImage, but does not know of all cache layers. Give it a hand.
>> +     * Boooting Linux via GRUB prior to version 2.04 fails on 32bit
>> ARM if
>> +     * caches are enabled.
>> +     *
>> +     * TODO:
>> +     * According to the UEFI spec caches that can be managed via CP15
>> +     * operations should be enabled. Caches requiring platform
>> information
>> +     * to manage should be disabled. This should not happen in
>> +     * ExitBootServices() but before invoking any UEFI binary is
>> invoked.
>> +     *
>> +     * We want to keep the current workaround while GRUB prior to
>> version
>> +     * 2.04 is still in use.
>>        */
>> -    if (efi_is_direct_boot)
>> -        cleanup_before_linux();
>> +    cleanup_before_linux();
>>   #endif
>>   }
>>
>> @@ -2893,8 +2893,6 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t
>> image_handle,
>>       if (ret != EFI_SUCCESS)
>>           return EFI_EXIT(EFI_INVALID_PARAMETER);
>>
>> -    efi_is_direct_boot = false;
>> -
>>       image_obj->exit_data_size = exit_data_size;
>>       image_obj->exit_data = exit_data;
>>
>> --
>> 2.20.1
>>
>