Re: EFI_STUB fails to boot non-EFI on arm64

[Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

On Wed, May 28, 2014 at 07:40:43PM +0100, Will Deacon wrote:
> > 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?

The pr_err would now only be triggered if some UEFI properties were
present and not others. I.e. if the UEFI stub loader failed miserably
without detecting it, or something corrupted the DT later on. We would
then be the proud owners of a system in an undefined state.

> > 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?

Having a partial/invalid DT is going to cause undefined behaviour
regardless of your method of booting.

And if the data provided proved inaccessible or broken, the symptom
would be a complete lack of memblocks. So it may in fact not be
possible for that pr_err to ever make it to the console :)

> > 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? ;)

Name cargo-culted from ia64/x86 by me, and from me by Mark :)

> > 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.

And I still don't see how an extra call site in setup_arch would make
this more possible than it already is (with my suggested patch to the
current version of the code).

/
    Leif