From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: EFI_STUB fails to boot non-EFI on arm64 Date: Wed, 28 May 2014 19:40:43 +0100 Message-ID: <20140528184043.GB20523@arm.com> References: <20140523094513.GC9252@arm.com> <20140523131656.GR4179@bivouac.eciton.net> <20140523134720.GA9256@arm.com> <20140523150331.GS4179@bivouac.eciton.net> <20140528155931.GA20523@arm.com> <20140528180525.GU4179@bivouac.eciton.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20140528180525.GU4179-t77nlHhSwNqAroYi2ySoxKxOck334EZe@public.gmane.org> Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Leif Lindholm Cc: Catalin Marinas , Roy Franz , "linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" List-Id: linux-efi@vger.kernel.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