Re: [PATCH] x86/reboot: Avoid EFI reboot when not running on EFI

[Bradford, Robert]

On Thu, 2019-08-29 at 15:34 +0100, Bradford, Robert wrote:
> On Thu, 2019-08-29 at 14:18 +0200, Thomas Gleixner wrote:
> > On Thu, 29 Aug 2019, Rob Bradford wrote:
> > 
> > CC+ Ard
> > 
> > > Replace the check using efi_runtime_disabled() which only checks
> > > if
> > > EFI
> > > runtime was disabled on the kernel command line with a call to
> > > efi_enabled(EFI_RUNTIME_SERVICES) to check if EFI runtime
> > > services
> > > are
> > > available.
> > > 
> > > In the situation where the kernel was booted without an EFI
> > > environment
> > > then only efi_enabled(EFI_RUNTIME_SERVICES) correctly represents
> > > that no
> > > EFI is available. Setting "noefi" or "efi=noruntime" on the
> > > commandline
> > > continue to have the same effect as
> > > efi_enabled(EFI_RUNTIME_SERVICES)
> > > will return false.
> > > 
> > > 
> > > Signed-off-by: Rob Bradford <robert.bradford@xxxxxxxxx>
> > > ---
> > >  arch/x86/kernel/reboot.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> > > index 09d6bded3c1e..0b0a7fccdb00 100644
> > > --- a/arch/x86/kernel/reboot.c
> > > +++ b/arch/x86/kernel/reboot.c
> > > @@ -500,7 +500,7 @@ static int __init reboot_init(void)
> > >  	 */
> > >  	rv = dmi_check_system(reboot_dmi_table);
> > >  
> > > -	if (!rv && efi_reboot_required() && !efi_runtime_disabled())
> > > +	if (!rv && efi_reboot_required() &&
> > > efi_enabled(EFI_RUNTIME_SERVICES))
> > 
> > Why is efi_reboot_required() set at all in that situation?
> > 
> Hi Thomas,
> 
> This platform uses HW Reduced ACPI (
> https://github.com/intel/cloud-hypervisor) but also supports direct
> kernel boot without EFI.
> 
> /*
>  * For most modern platforms the preferred method of powering off is
> via
>  * ACPI. However, there are some that are known to require the use of
>  * EFI runtime services and for which ACPI does not work at all.
>  *
>  * Using EFI is a last resort, to be used only if no other option
>  * exists.
>  */
> bool efi_reboot_required(void)
> {
> 	if (!acpi_gbl_reduced_hardware)
> 		return false;
> 
> 	efi_reboot_quirk_mode = EFI_RESET_WARM;
> 	return true;
> }
> 
> This is a hardware workaround that assumes that all ACPI HW reduced
> platforms do not support ACPI reset:
> 
> commit 44be28e9dd9880dca3e2cbf7a844f2114e67f2cb
> Author: Matt Fleming <matt.fleming@xxxxxxxxx>
> Date:   Fri Jun 13 12:39:55 2014 +0100
> 
>     x86/reboot: Add EFI reboot quirk for ACPI Hardware Reduced flag
>     
>     It appears that the BayTrail-T class of hardware requires EFI in
> order
>     to powerdown and reboot and no other reliable method exists.
>     
>     This quirk is generally applicable to all hardware that has the
> ACPI
>     Hardware Reduced bit set, since usually ACPI would be the
> preferred
>     method.
>     
>     Cc: Len Brown <len.brown@xxxxxxxxx>
>     Cc: Mark Salter <msalter@xxxxxxxxxx>
>     Cc: "Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>
>     Signed-off-by: Matt Fleming <matt.fleming@xxxxxxxxx>
> 
> I'm reluctant to change the behaviour of efi_reboot_required() as it
> might break older hardware whereas checking if we have an EFI runtime
> is safer.
> 

Hi Thomas,

Any update on your thoughts on this patch?

Rob