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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
