[U-Boot,RFC,1/1] efi_loader: usage of cleanup_before_linux()

Always call cleanup_before_linux() on ARM 32bit during ExitBootServices().

This fixes a problem problem for booting BSD on ARM 32bit.

Reported-by: Jonathan Gray <jsg@jsg.id.au>
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_boottime.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

--
2.20.1

On 18.07.19 19:46, Heinrich Schuchardt wrote:
> Always call cleanup_before_linux() on ARM 32bit during ExitBootServices().
>
> This fixes a problem problem for booting BSD on ARM 32bit.
>
> Reported-by: Jonathan Gray <jsg@jsg.id.au>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


NAK. Instead this should never call cleanup_before_linux() and we 
declare ourselves incompatible to old grub versions. That way we don't 
lure others into believing you could boot with caches disabled in a UEFI 
world.

Alex


> ---
>   lib/efi_loader/efi_boottime.c | 13 +------------
>   1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 4f6e8d1679..171f485f6b 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
> @@ -1916,8 +1908,7 @@ static void efi_exit_caches(void)
>   	 * 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();
> +	cleanup_before_linux();
>   #endif
>   }
>
> @@ -2893,8 +2884,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
>

On Thu, Jul 18, 2019 at 08:53:01PM +0200, Alexander Graf wrote:
> 
> On 18.07.19 19:46, Heinrich Schuchardt wrote:
> > Always call cleanup_before_linux() on ARM 32bit during ExitBootServices().
> > 
> > This fixes a problem problem for booting BSD on ARM 32bit.
> > 
> > Reported-by: Jonathan Gray <jsg@jsg.id.au>
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> NAK. Instead this should never call cleanup_before_linux() and we declare
> ourselves incompatible to old grub versions. That way we don't lure others
> into believing you could boot with caches disabled in a UEFI world.

Agreed.

This is my fault by the way, for merging a loader in GRUB that was
designed to be used without the linux EFI stub (way, waaaay back).
Feel free to shout.

As of GRUB 2.04 release (and cherry-picked into debian Buster before
that), the 32-bit and 64-bit UEFI ports use the same loader.

/
    Leif

On Thu, Jul 18, 2019 at 9:47 PM Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Jul 18, 2019 at 08:53:01PM +0200, Alexander Graf wrote:
> >
> > On 18.07.19 19:46, Heinrich Schuchardt wrote:
> > > Always call cleanup_before_linux() on ARM 32bit during ExitBootServices().
> > >
> > > This fixes a problem problem for booting BSD on ARM 32bit.
> > >
> > > Reported-by: Jonathan Gray <jsg@jsg.id.au>
> > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > NAK. Instead this should never call cleanup_before_linux() and we declare
> > ourselves incompatible to old grub versions. That way we don't lure others
> > into believing you could boot with caches disabled in a UEFI world.
>
> Agreed.
>
> This is my fault by the way, for merging a loader in GRUB that was
> designed to be used without the linux EFI stub (way, waaaay back).
> Feel free to shout.
>
> As of GRUB 2.04 release (and cherry-picked into debian Buster before
> that), the 32-bit and 64-bit UEFI ports use the same loader.

Do you have a reference to this patch set handy?

Peter

On Fri, Jul 19, 2019 at 07:26:19AM +0100, Peter Robinson wrote:
> > As of GRUB 2.04 release (and cherry-picked into debian Buster before
> > that), the 32-bit and 64-bit UEFI ports use the same loader.
> 
> Do you have a reference to this patch set handy?

I figured it might be good to write a proper summary and collect this
info somewhere, so I just posted that to:
https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html

/
    Leif

On Fri, Jul 19, 2019 at 9:51 AM Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Jul 19, 2019 at 07:26:19AM +0100, Peter Robinson wrote:
> > > As of GRUB 2.04 release (and cherry-picked into debian Buster before
> > > that), the 32-bit and 64-bit UEFI ports use the same loader.
> >
> > Do you have a reference to this patch set handy?
>
> I figured it might be good to write a proper summary and collect this
> info somewhere, so I just posted that to:
> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html

Seen it, awesome, thanks