[U-Boot] [PATCH 1/1] efi_loader: remove efi_exit_caches()

[Heinrich Schuchardt]

On 7/20/19 1:59 AM, Jonathan Gray wrote:
> On Fri, Jul 19, 2019 at 08:25:45PM +0200, Heinrich Schuchardt wrote:
>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>> boards if U-Boot did not disable caches, cf.
>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>
>> In ExitBootServices() we were disabling the caches by calling
>> cleanup_before_linux(). This workaround is not needed anymore.
>
> It is required for at least OpenBSD, FreeBSD and NetBSD.
> Which are all now forced to carry patches to be able to use U-Boot.

As described below this code does not remove any functionality that was
active in v2019.04 or v2019.07.

The logic in pre-v2019.04 to change cache status in ExitBootImage() has
no relation to the UEFI spec. If we do anything it has to be at an
earlier stage.

I am able to reproduce that OpenBSD on ARM32 is broken with current
U-Boot. So some further work is needed.

My testing results on a Banana Pi (an Allwinner A20 system) were as follows:

Without any change the Banana Pi hangs on

boot>
booting sd0a:/bsd: 4519648+675728+235352+561868
[296447+107+311232+275275]=0x68f5b0
EHCI failed to shut down host controller.

CONFIG_SYS_L2CACHE_OFF=y has no effect on Allwinner CPUs (mach-sunxi) so
the Banana Pi keeps hanging.

When cleanup_before_linux() is called it boots into OpenBSD.

cleanup_before_linux() disables all caches and this contradicts the UEFI
specification.

Best regards

Heinrich

>
>>
>> The UEFI spec requires that caches are enabled but architecturally defined
>> caches should be disabled. But this requirement has to be fulfilled when
>> invoking StartImage() and not after calling ExitBootServices(). So there is
>> no reason for calling cleanup_before_linux() here.
>>
>> Since commit f69d63fae281 ("efi_loader: use efi_start_image() for
>> bootefi"), i.e. v2019.04, we have not been calling efi_exit_caches()
>> anymore which in turn would call cleanup_before_linux().
>>
>> Remove the obsolete function efi_exit_caches().
>>
>> Suggested-by: Alexander Graf <agraf at csgraf.de>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>  lib/efi_loader/efi_boottime.c | 28 ----------------------------
>>  1 file changed, 28 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 4f6e8d1679..f84ee24416 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;
>> -
>>  #ifdef CONFIG_ARM
>>  /*
>>   * The "gd" pointer lives in a register on ARM and AArch64 that we declare
>> @@ -1906,21 +1898,6 @@ error:
>>  	return EFI_EXIT(ret);
>>  }
>>
>> -/**
>> - * efi_exit_caches() - fix up caches for EFI payloads if necessary
>> - */
>> -static void efi_exit_caches(void)
>> -{
>> -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>> -	/*
>> -	 * 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.
>> -	 */
>> -	if (efi_is_direct_boot)
>> -		cleanup_before_linux();
>> -#endif
>> -}
>> -
>>  /**
>>   * efi_exit_boot_services() - stop all boot services
>>   * @image_handle: handle of the loaded image
>> @@ -1990,9 +1967,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>>  	/* Patch out unsupported runtime function */
>>  	efi_runtime_detach();
>>
>> -	/* Fix up caches for EFI payloads if necessary */
>> -	efi_exit_caches();
>> -
>>  	/* This stops all lingering devices */
>>  	bootm_disable_interrupts();
>>
>> @@ -2893,8 +2867,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
>>
>