* [PATCH] x86/reboot: Avoid EFI reboot when not running on EFI
@ 2019-08-29 10:11 Rob Bradford
2019-08-29 12:18 ` Thomas Gleixner
0 siblings, 1 reply; 5+ messages in thread
From: Rob Bradford @ 2019-08-29 10:11 UTC (permalink / raw)
To: x86
Cc: Rob Bradford, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, Rik van Riel, Juergen Gross, Jian-Hong Pan,
linux-kernel
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@intel.com>
---
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))
reboot_type = BOOT_EFI;
return 0;
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/reboot: Avoid EFI reboot when not running on EFI
2019-08-29 10:11 [PATCH] x86/reboot: Avoid EFI reboot when not running on EFI Rob Bradford
@ 2019-08-29 12:18 ` Thomas Gleixner
2019-08-29 14:34 ` Bradford, Robert
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2019-08-29 12:18 UTC (permalink / raw)
To: Rob Bradford
Cc: x86, Ingo Molnar, Borislav Petkov, H. Peter Anvin, Rik van Riel,
Juergen Gross, Jian-Hong Pan, LKML, Ard Biesheuvel
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@intel.com>
> ---
> 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?
Thanks,
tglx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/reboot: Avoid EFI reboot when not running on EFI
2019-08-29 12:18 ` Thomas Gleixner
@ 2019-08-29 14:34 ` Bradford, Robert
2019-09-12 12:40 ` Bradford, Robert
2019-09-13 18:54 ` Ard Biesheuvel
0 siblings, 2 replies; 5+ messages in thread
From: Bradford, Robert @ 2019-08-29 14:34 UTC (permalink / raw)
To: tglx
Cc: jgross, riel, x86, bp, hpa, mingo, linux-kernel, jian-hong,
ard.biesheuvel
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@intel.com>
> > ---
> > 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@intel.com>
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@intel.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
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.
Rob
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/reboot: Avoid EFI reboot when not running on EFI
2019-08-29 14:34 ` Bradford, Robert
@ 2019-09-12 12:40 ` Bradford, Robert
2019-09-13 18:54 ` Ard Biesheuvel
1 sibling, 0 replies; 5+ messages in thread
From: Bradford, Robert @ 2019-09-12 12:40 UTC (permalink / raw)
To: tglx
Cc: riel, jgross, x86, bp, hpa, mingo, linux-kernel, jian-hong,
ard.biesheuvel
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@intel.com>
> > > ---
> > > 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@intel.com>
> 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@intel.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/reboot: Avoid EFI reboot when not running on EFI
2019-08-29 14:34 ` Bradford, Robert
2019-09-12 12:40 ` Bradford, Robert
@ 2019-09-13 18:54 ` Ard Biesheuvel
1 sibling, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2019-09-13 18:54 UTC (permalink / raw)
To: Bradford, Robert
Cc: tglx, jgross, riel, x86, bp, hpa, mingo, linux-kernel, jian-hong
On Thu, 29 Aug 2019 at 07:34, Bradford, Robert
<robert.bradford@intel.com> 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@intel.com>
> > > ---
> > > 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))
This change by itself seems correct to me - efi_runtime_disabled()
only gives you whether EFI runtime services were disabled explicitly
command line rather than whether they are available at all.
> >
> > 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:
>
I'm not that familiar with the situation on x86, but I did discuss
this with the MS Windows engineers back in April, and their policy is
to strongly prefer ACPI reboot, and so that is most likely to work on
arbitrary x86 hardware which was built to run Windows.
So I do think the patch below is overly broad, given that the ACPI
spec is unambiguous about the fact the FADT reset is also covered by
H/W reduced ACPI.
However, I do share Rob's reservation given how quirky x86 hardware
can be in this respect.
TL;DR:
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> commit 44be28e9dd9880dca3e2cbf7a844f2114e67f2cb
> Author: Matt Fleming <matt.fleming@intel.com>
> 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@intel.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
>
> 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.
>
> Rob
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-13 18:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-29 10:11 [PATCH] x86/reboot: Avoid EFI reboot when not running on EFI Rob Bradford
2019-08-29 12:18 ` Thomas Gleixner
2019-08-29 14:34 ` Bradford, Robert
2019-09-12 12:40 ` Bradford, Robert
2019-09-13 18:54 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).