Re: EFI_STUB fails to boot non-EFI on arm64

[Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>]

On Wed, May 28, 2014 at 07:05:26PM +0100, Leif Lindholm wrote:
> On Wed, May 28, 2014 at 04:59:31PM +0100, Will Deacon wrote:
> > > > Can we add another of detecting whether it's an EFI application and
> > > > avoid calling efi_init()? I can see x86 sets some efi_loader_signature
> > > > string in exit_boot() and checks against it later when calling
> > > > efi_init().
> > > 
> > > Well, I agree that we shouldn't be spewing error messages for expected
> > > operation, but efi_init() is the function we call to determine
> > > whether we _are_ booting via UEFI - and it sets flags accordingly for
> > > the efi_enabled() macro.
> > > 
> > > My view is that this should be fixed in fdt_find_uefi_params(). A
> > > single info message that we can't find evidence of UEFI should be
> > > printed in the non-error case.
> > > 
> > > Like below?
> > 
> > Why not move the efi_get_fdt_params call out of efi_init and into
> > setup_arch via a wrapper? Then efi_get_fdt_params and efi_init can have
> > useful return values, which allow us to distinguish between "My DT doesn't
> > have the necessary UEFI properties" and "UEFI failed to initialise" without
> > having to make some printks pr_info and others pr_err within efi_init
> > itself..
> 
> Well, but (for the output part) my patch already did that?
> If the "Getting parameters from FDT:\n" was too verbose, we could
> just drop it, and have the same effect on output.

It's the pr_err which is annoying, not the "Getting parameters from FDT:\n"
message. Why should I have an error logged to my console when I was never
intending to boot using EFI anyway?

> Thing is - there is not really any error case available anywhere
> during the execution of efi_init() and its branches other than:
> - Information required for UEFI boot cannot be found.
> - Information exists, but is invalid.
> - Failed to early_memremap some UEFI regions into the kernel.
> which all amounts to "UEFI not available or something went wrong",
> rather than "UEFI failed to initialise".

Fine, but in this case the DT had the relevant properties which is a good
indication that the user was at least *trying* to boot using EFI, no?

> If efi_init returns successfully, EFI_BOOT is set, and testable using
> the efi_enabled() macro.
> 
> The proper "UEFI failed to initialise" bit does not come until the
> early_initcall arm64_enter_virtual_mode(), and is indicated not by
> a return value, but by setting the flag indicating that
> EFI_RUNTIME_SERVICES are available, which is checked later in core
> code using the efi_enabled() macro.

Sorry, I naively assumed that with a name like efi_init it might, you know,
initialise EFI? ;)

> So moving the call to efi_get_fdt_params() would have little effect
> other than adding a third call site for UEFI bits in setup_arch().

I don't mind having the extra call site if it allows us to distinguish
errors from information.

Will