[PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Chen Yu]

The problem is Linux registers pm_power_off = efi_power_off
only if we are in hardware reduced mode. Actually, what we also
want is to do this when ACPI S5 is simply not supported on
non-legacy platforms. That should handle both the HW reduced mode,
and the HW-full mode where the DSDT fails to supply an _S5 object.

This patch fixes this issue by introducing a new flag acpi_no_s5 which
indicates the non-existence of _S5. The initial state of acpi_no_s5 is
false and probed in acpi_sleep_init, then we'll later see the updated
value in efi_poweroff_required, according to which we can set pm_power_off
to efi_power_off in efi_shutdown_init, if no other pm_power_off available.

Suggested-by: Len Brown <len.brown@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v3:
 - Only assign pm_power_off to efi_power_off when there are no
   other pm_power_off registered at that time, in case other
   commponents would like to customize their own implementation.
---
v2:
 - Convert the acpi_no_s5 to a global bool variable in sleep.c and
   add a declaration to include/linux/acpi.h.
---
 arch/x86/platform/efi/quirks.c | 2 +-
 drivers/acpi/sleep.c           | 8 ++++++++
 include/linux/acpi.h           | 1 +
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 2d66db8..0d4186b 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -295,5 +295,5 @@ bool efi_reboot_required(void)
 
 bool efi_poweroff_required(void)
 {
-	return !!acpi_gbl_reduced_hardware;
+	return acpi_gbl_reduced_hardware || (acpi_no_s5 && !pm_power_off);
 }
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 9cb9752..94099a4 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -25,6 +25,12 @@
 #include "internal.h"
 #include "sleep.h"
 
+/*
+ * Some HW-full platforms do not have _S5, they may have
+ * to leverage efi rather than acpi for a shutdown, if no
+ * other pm_power_off available.
+ */
+bool acpi_no_s5;
 static u8 sleep_states[ACPI_S_STATE_COUNT];
 
 static void acpi_sleep_tts_switch(u32 acpi_state)
@@ -846,6 +852,8 @@ int __init acpi_sleep_init(void)
 		sleep_states[ACPI_STATE_S5] = 1;
 		pm_power_off_prepare = acpi_power_off_prepare;
 		pm_power_off = acpi_power_off;
+	} else {
+		acpi_no_s5 = true;
 	}
 
 	supported[0] = 0;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 06ed7e5..4d2e67f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -278,6 +278,7 @@ void acpi_irq_stats_init(void);
 extern u32 acpi_irq_handled;
 extern u32 acpi_irq_not_handled;
 extern unsigned int acpi_sci_irq;
+extern bool acpi_no_s5;
 #define INVALID_ACPI_IRQ	((unsigned)-1)
 static inline bool acpi_sci_irq_valid(void)
 {
-- 
1.8.4.2

Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Rafael J. Wysocki]

On Mon, Mar 7, 2016 at 3:50 AM, Chen Yu <yu.c.chen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> The problem is Linux registers pm_power_off = efi_power_off
> only if we are in hardware reduced mode. Actually, what we also
> want is to do this when ACPI S5 is simply not supported on
> non-legacy platforms. That should handle both the HW reduced mode,
> and the HW-full mode where the DSDT fails to supply an _S5 object.
>
> This patch fixes this issue by introducing a new flag acpi_no_s5 which
> indicates the non-existence of _S5. The initial state of acpi_no_s5 is
> false and probed in acpi_sleep_init, then we'll later see the updated
> value in efi_poweroff_required, according to which we can set pm_power_off
> to efi_power_off in efi_shutdown_init, if no other pm_power_off available.
>
> Suggested-by: Len Brown <len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Chen Yu <yu.c.chen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> v3:
>  - Only assign pm_power_off to efi_power_off when there are no
>    other pm_power_off registered at that time, in case other
>    commponents would like to customize their own implementation.
> ---
> v2:
>  - Convert the acpi_no_s5 to a global bool variable in sleep.c and
>    add a declaration to include/linux/acpi.h.
> ---
>  arch/x86/platform/efi/quirks.c | 2 +-
>  drivers/acpi/sleep.c           | 8 ++++++++
>  include/linux/acpi.h           | 1 +
>  3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 2d66db8..0d4186b 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -295,5 +295,5 @@ bool efi_reboot_required(void)
>
>  bool efi_poweroff_required(void)
>  {
> -       return !!acpi_gbl_reduced_hardware;
> +       return acpi_gbl_reduced_hardware || (acpi_no_s5 && !pm_power_off);

What if CONFIG_ACPI is not set here?

>  }

Thanks,
Rafael

RE: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Chen, Yu C]

Hi Rafael,


> -----Original Message-----
> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of
> Rafael J. Wysocki
> Sent: Monday, March 07, 2016 9:19 PM
> To: Chen, Yu C
> Cc: ACPI Devel Maling List; x86@kernel.org; linux-efi@vger.kernel.org; Linux
> Kernel Mailing List; linux-pm@vger.kernel.org; Rafael J. Wysocki; Len Brown;
> Matt Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin; Zhang, Rui
> Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full
> platforms without _S5
> 
> On Mon, Mar 7, 2016 at 3:50 AM, Chen Yu <yu.c.chen@intel.com> wrote:
> > The problem is Linux registers pm_power_off = efi_power_off only if we
> > are in hardware reduced mode. Actually, what we also want is to do
> > this when ACPI S5 is simply not supported on non-legacy platforms.
> > That should handle both the HW reduced mode, and the HW-full mode
> > where the DSDT fails to supply an _S5 object.
> >
> > This patch fixes this issue by introducing a new flag acpi_no_s5 which
> > indicates the non-existence of _S5. The initial state of acpi_no_s5 is
> > false and probed in acpi_sleep_init, then we'll later see the updated
> > value in efi_poweroff_required, according to which we can set
> > pm_power_off to efi_power_off in efi_shutdown_init, if no other
> pm_power_off available.
> >
> > Suggested-by: Len Brown <len.brown@intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> > v3:
> >  - Only assign pm_power_off to efi_power_off when there are no
> >    other pm_power_off registered at that time, in case other
> >    commponents would like to customize their own implementation.
> > ---
> > v2:
> >  - Convert the acpi_no_s5 to a global bool variable in sleep.c and
> >    add a declaration to include/linux/acpi.h.
> > ---
> >  arch/x86/platform/efi/quirks.c | 2 +-
> >  drivers/acpi/sleep.c           | 8 ++++++++
> >  include/linux/acpi.h           | 1 +
> >  3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/platform/efi/quirks.c
> > b/arch/x86/platform/efi/quirks.c index 2d66db8..0d4186b 100644
> > --- a/arch/x86/platform/efi/quirks.c
> > +++ b/arch/x86/platform/efi/quirks.c
> > @@ -295,5 +295,5 @@ bool efi_reboot_required(void)
> >
> >  bool efi_poweroff_required(void)
> >  {
> > -       return !!acpi_gbl_reduced_hardware;
> > +       return acpi_gbl_reduced_hardware || (acpi_no_s5 &&
> > + !pm_power_off);
> 
> What if CONFIG_ACPI is not set here?
> 
If CONFIG_ACPI is not set, this file would not be compiled,
because CONFIG_EFI depends on CONFIG_ACPI.

RE: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Chen, Yu C]

Hi Rafael,
(resend for broken content)

> -----Original Message-----
> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of
> Rafael J. Wysocki
> Sent: Monday, March 07, 2016 9:19 PM
> To: Chen, Yu C
> Cc: ACPI Devel Maling List; x86@kernel.org; linux-efi@vger.kernel.org; Linux
> Kernel Mailing List; linux-pm@vger.kernel.org; Rafael J. Wysocki; Len Brown;
> Matt Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin; Zhang, Rui
> Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full
> platforms without _S5
> 
[cut]
> >  bool efi_poweroff_required(void)
> >  {
> > -       return !!acpi_gbl_reduced_hardware;
> > +       return acpi_gbl_reduced_hardware || (acpi_no_s5 &&
> > + !pm_power_off);
> 
> What if CONFIG_ACPI is not set here?
If CONFIG_ACPI is not set, this file would not 
be compiled, because CONFIG_EFI depends on CONFIG_ACPI.

thanks,
yu

Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Rafael J. Wysocki]

On Monday, March 07, 2016 03:53:13 PM Chen, Yu C wrote:
> Hi Rafael,
> (resend for broken content)
> 
> > -----Original Message-----
> > From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of
> > Rafael J. Wysocki
> > Sent: Monday, March 07, 2016 9:19 PM
> > To: Chen, Yu C
> > Cc: ACPI Devel Maling List; x86@kernel.org; linux-efi@vger.kernel.org; Linux
> > Kernel Mailing List; linux-pm@vger.kernel.org; Rafael J. Wysocki; Len Brown;
> > Matt Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin; Zhang, Rui
> > Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full
> > platforms without _S5
> > 
> [cut]
> > >  bool efi_poweroff_required(void)
> > >  {
> > > -       return !!acpi_gbl_reduced_hardware;
> > > +       return acpi_gbl_reduced_hardware || (acpi_no_s5 &&
> > > + !pm_power_off);
> > 
> > What if CONFIG_ACPI is not set here?
> If CONFIG_ACPI is not set, this file would not 
> be compiled, because CONFIG_EFI depends on CONFIG_ACPI.

OK

So the next question will be if efi_poweroff_required() is guaranteed to run
after all of the other code that may register alternative power off handling.

Thanks,
Rafael

RE: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Chen, Yu C]

> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> Sent: Tuesday, March 08, 2016 9:54 AM
> To: Chen, Yu C
> Cc: Rafael J. Wysocki; ACPI Devel Maling List; x86@kernel.org; linux-
> efi@vger.kernel.org; Linux Kernel Mailing List; linux-pm@vger.kernel.org;
> Len Brown; Matt Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin;
> Zhang, Rui
> Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full
> platforms without _S5
> 
> On Monday, March 07, 2016 03:53:13 PM Chen, Yu C wrote:
> > Hi Rafael,
> > (resend for broken content)
> >
> > > -----Original Message-----
> > > From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of
> > > Rafael J. Wysocki
> > > Sent: Monday, March 07, 2016 9:19 PM
> > > To: Chen, Yu C
> > > Cc: ACPI Devel Maling List; x86@kernel.org;
> > > linux-efi@vger.kernel.org; Linux Kernel Mailing List;
> > > linux-pm@vger.kernel.org; Rafael J. Wysocki; Len Brown; Matt
> > > Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin; Zhang, Rui
> > > Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on
> > > HW-full platforms without _S5
> > >
> > [cut]
> > > >  bool efi_poweroff_required(void)
> > > >  {
> > > > -       return !!acpi_gbl_reduced_hardware;
> > > > +       return acpi_gbl_reduced_hardware || (acpi_no_s5 &&
> > > > + !pm_power_off);
> > >
> > > What if CONFIG_ACPI is not set here?
> > If CONFIG_ACPI is not set, this file would not be compiled, because
> > CONFIG_EFI depends on CONFIG_ACPI.
> 
> OK
> 
> So the next question will be if efi_poweroff_required() is guaranteed to run
> after all of the other code that may register alternative power off handling.
Hum. unfortunately it is not guaranteed to run after all of the other code,
because other components who register pm_power_off may be built as modules, and
we can not predict/control the sequence registration.   So this patch may
break the EFI platforms who use non-efi poweroff due to unstable EFI service
,  not sure if there are any released-products of this kind.

Currently I'm thinking of 3 possible solutions,  could you please give some advices on them:

1. Introduce bootopt of 'poweroff=efi'
     Set the pm_power_off to efi_power_off no matter whether there is _S5 or not

2. Introduce /sys/power/poweroff
    Allow the user to choose which  pm_power_off, for example:
 
# cat /sys/power/poweroff
*acpi		acpi_power_off
efi		efi_power_off	
gpio		gpio_poweroff_do_poweroff
user can echo string to enable which one.

And two APIs:
register_power_off(char *name, power_off func)
unregister_power_off(char *name)  


3. replace all the codes of  pm_power_off() with reliable_pm_power_off()

void reliable_pm_power_off(void)
{
	if (!pm_power_off) {
		if (acpi_no_s5)
			pm_power_off = efi_power_off;
	/* Other conditions added in the future. */
	}
	pm_power_off();
}


thanks,
yu

Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Rafael J. Wysocki]

On Tuesday, March 08, 2016 04:25:30 PM Chen, Yu C wrote:
> > -----Original Message-----
> > From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> > owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> > Sent: Tuesday, March 08, 2016 9:54 AM
> > To: Chen, Yu C
> > Cc: Rafael J. Wysocki; ACPI Devel Maling List; x86@kernel.org; linux-
> > efi@vger.kernel.org; Linux Kernel Mailing List; linux-pm@vger.kernel.org;
> > Len Brown; Matt Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin;
> > Zhang, Rui
> > Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full
> > platforms without _S5
> > 
> > On Monday, March 07, 2016 03:53:13 PM Chen, Yu C wrote:
> > > Hi Rafael,
> > > (resend for broken content)
> > >
> > > > -----Original Message-----
> > > > From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of
> > > > Rafael J. Wysocki
> > > > Sent: Monday, March 07, 2016 9:19 PM
> > > > To: Chen, Yu C
> > > > Cc: ACPI Devel Maling List; x86@kernel.org;
> > > > linux-efi@vger.kernel.org; Linux Kernel Mailing List;
> > > > linux-pm@vger.kernel.org; Rafael J. Wysocki; Len Brown; Matt
> > > > Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin; Zhang, Rui
> > > > Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on
> > > > HW-full platforms without _S5
> > > >
> > > [cut]
> > > > >  bool efi_poweroff_required(void)
> > > > >  {
> > > > > -       return !!acpi_gbl_reduced_hardware;
> > > > > +       return acpi_gbl_reduced_hardware || (acpi_no_s5 &&
> > > > > + !pm_power_off);
> > > >
> > > > What if CONFIG_ACPI is not set here?
> > > If CONFIG_ACPI is not set, this file would not be compiled, because
> > > CONFIG_EFI depends on CONFIG_ACPI.
> > 
> > OK
> > 
> > So the next question will be if efi_poweroff_required() is guaranteed to run
> > after all of the other code that may register alternative power off handling.
> Hum. unfortunately it is not guaranteed to run after all of the other code,
> because other components who register pm_power_off may be built as modules, and
> we can not predict/control the sequence registration.   So this patch may
> break the EFI platforms who use non-efi poweroff due to unstable EFI service
> ,  not sure if there are any released-products of this kind.
> 
> Currently I'm thinking of 3 possible solutions,  could you please give some advices on them:
> 
> 1. Introduce bootopt of 'poweroff=efi'
>      Set the pm_power_off to efi_power_off no matter whether there is _S5 or not
> 
> 2. Introduce /sys/power/poweroff
>     Allow the user to choose which  pm_power_off, for example:
>  
> # cat /sys/power/poweroff
> *acpi		acpi_power_off
> efi		efi_power_off	
> gpio		gpio_poweroff_do_poweroff
> user can echo string to enable which one.
> 
> And two APIs:
> register_power_off(char *name, power_off func)
> unregister_power_off(char *name)  
> 
> 
> 3. replace all the codes of  pm_power_off() with reliable_pm_power_off()
> 
> void reliable_pm_power_off(void)
> {
> 	if (!pm_power_off) {
> 		if (acpi_no_s5)
> 			pm_power_off = efi_power_off;
> 	/* Other conditions added in the future. */
> 	}
> 	pm_power_off();
> }

What about something like adding something like default_power_off that would
be used by pm_power_off if nothing else is available?

Thanks,
Rafael

RE: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Chen, Yu C]

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Wednesday, March 09, 2016 6:57 AM
> To: Chen, Yu C
> Cc: Rafael J. Wysocki; ACPI Devel Maling List; x86@kernel.org; linux-
> efi@vger.kernel.org; Linux Kernel Mailing List; linux-pm@vger.kernel.org;
> Len Brown; Matt Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin;
> Zhang, Rui
> Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full
> platforms without _S5
> 
> On Tuesday, March 08, 2016 04:25:30 PM Chen, Yu C wrote:
> > > -----Original Message-----
> > > From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> > > owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> > > Sent: Tuesday, March 08, 2016 9:54 AM
> > > To: Chen, Yu C
> > > Cc: Rafael J. Wysocki; ACPI Devel Maling List; x86@kernel.org;
> > > linux- efi@vger.kernel.org; Linux Kernel Mailing List;
> > > linux-pm@vger.kernel.org; Len Brown; Matt Fleming; Thomas Gleixner;
> > > Ingo Molnar; H. Peter Anvin; Zhang, Rui
> > > Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on
> > > HW-full platforms without _S5
> > >
> > > On Monday, March 07, 2016 03:53:13 PM Chen, Yu C wrote:
> > > > Hi Rafael,
> > > > (resend for broken content)
> > > >
> > > > > -----Original Message-----
> > > > > From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf
> > > > > Of Rafael J. Wysocki
> > > > > Sent: Monday, March 07, 2016 9:19 PM
> > > > > To: Chen, Yu C
> > > > > Cc: ACPI Devel Maling List; x86@kernel.org;
> > > > > linux-efi@vger.kernel.org; Linux Kernel Mailing List;
> > > > > linux-pm@vger.kernel.org; Rafael J. Wysocki; Len Brown; Matt
> > > > > Fleming; Thomas Gleixner; Ingo Molnar; H. Peter Anvin; Zhang,
> > > > > Rui
> > > > > Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on
> > > > > HW-full platforms without _S5
> > > > >
> > > > [cut]
> > > > > >  bool efi_poweroff_required(void)  {
> > > > > > -       return !!acpi_gbl_reduced_hardware;
> > > > > > +       return acpi_gbl_reduced_hardware || (acpi_no_s5 &&
> > > > > > + !pm_power_off);
> > > > >
> > > > > What if CONFIG_ACPI is not set here?
> > > > If CONFIG_ACPI is not set, this file would not be compiled,
> > > > because CONFIG_EFI depends on CONFIG_ACPI.
> > >
> > > OK
> > >
> > > So the next question will be if efi_poweroff_required() is
> > > guaranteed to run after all of the other code that may register alternative
> power off handling.
> > Hum. unfortunately it is not guaranteed to run after all of the other
> > code, because other components who register pm_power_off may be
> built as modules, and
> > we can not predict/control the sequence registration.   So this patch may
> > break the EFI platforms who use non-efi poweroff due to unstable EFI
> > service ,  not sure if there are any released-products of this kind.
> >
> > Currently I'm thinking of 3 possible solutions,  could you please give some
> advices on them:
> >
> > 1. Introduce bootopt of 'poweroff=efi'
> >      Set the pm_power_off to efi_power_off no matter whether there is
> > _S5 or not
> >
> > 2. Introduce /sys/power/poweroff
> >     Allow the user to choose which  pm_power_off, for example:
> >
> > # cat /sys/power/poweroff
> > *acpi		acpi_power_off
> > efi		efi_power_off
> > gpio		gpio_poweroff_do_poweroff
> > user can echo string to enable which one.
> >
> > And two APIs:
> > register_power_off(char *name, power_off func)
> > unregister_power_off(char *name)
> >
> >
> > 3. replace all the codes of  pm_power_off() with
> > reliable_pm_power_off()
> >
> > void reliable_pm_power_off(void)
> > {
> > 	if (!pm_power_off) {
> > 		if (acpi_no_s5)
> > 			pm_power_off = efi_power_off;
> > 	/* Other conditions added in the future. */
> > 	}
> > 	pm_power_off();
> > }
> 
> What about something like adding something like default_power_off that
> would be used by pm_power_off if nothing else is available?
OK,  I'll try another version, in which other components can set  its default_power_off,
then pm_power_off has a chance to be set to default_power_off if pm_power_off is NULL.

Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Matt Fleming]

On Tue, 08 Mar, at 04:25:30PM, Chen, Yu C wrote:
> Hum. unfortunately it is not guaranteed to run after all of the other code,
> because other components who register pm_power_off may be built as modules, and
> we can not predict/control the sequence registration.   So this patch may
> break the EFI platforms who use non-efi poweroff due to unstable EFI service
> ,  not sure if there are any released-products of this kind.
 
Certainly the majority of x86 client machines do not use EFI power
off, because it hardly ever functions correctly.

> Currently I'm thinking of 3 possible solutions,  could you please give some advices on them:
> 
> 1. Introduce bootopt of 'poweroff=efi'
>      Set the pm_power_off to efi_power_off no matter whether there is _S5 or not
> 
> 2. Introduce /sys/power/poweroff
>     Allow the user to choose which  pm_power_off, for example:
>  
> # cat /sys/power/poweroff
> *acpi		acpi_power_off
> efi		efi_power_off	
> gpio		gpio_poweroff_do_poweroff
> user can echo string to enable which one.
> 
> And two APIs:
> register_power_off(char *name, power_off func)
> unregister_power_off(char *name)  
> 
> 
> 3. replace all the codes of  pm_power_off() with reliable_pm_power_off()
> 
> void reliable_pm_power_off(void)
> {
> 	if (!pm_power_off) {
> 		if (acpi_no_s5)
> 			pm_power_off = efi_power_off;
> 	/* Other conditions added in the future. */
> 	}
> 	pm_power_off();
> }

Be wary of adding all these control knobs. People just want their
machines to reboot properly without having to mess with boot
parameters.

Let's go back to the start. What prompted this patch? Do Intel have
(or are planning) machines that do not have _S5 and are expected to
use EFI to reset the system? Or is this some new configuration
discussed in the ACPI spec that Linux needs to be support? 

Can we remove the ambiguity and options to force EFI reset if _S5 is
missing? Afterall, that's why the function is called
efi_poweroff_*required*.

RE: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full platforms without _S5

[Chen, Yu C]

Hi Matt,

> -----Original Message-----
> From: Matt Fleming [mailto:matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org]
> Sent: Wednesday, March 09, 2016 11:35 PM
> To: Chen, Yu C
> Cc: Rafael J. Wysocki; Rafael J. Wysocki; ACPI Devel Maling List;
> x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linux Kernel Mailing List; linux-
> pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Len Brown; Thomas Gleixner; Ingo Molnar; H. Peter
> Anvin; Zhang, Rui
> Subject: Re: [PATCH][RFC v3] ACPI / PM: Fix poweroff issue on HW-full
> platforms without _S5
> 
> On Tue, 08 Mar, at 04:25:30PM, Chen, Yu C wrote:
> > Hum. unfortunately it is not guaranteed to run after all of the other
> > code, because other components who register pm_power_off may be
> built as modules, and
> > we can not predict/control the sequence registration.   So this patch may
> > break the EFI platforms who use non-efi poweroff due to unstable EFI
> > service ,  not sure if there are any released-products of this kind.
> 
> Certainly the majority of x86 client machines do not use EFI power off,
> because it hardly ever functions correctly.
> 
> > Currently I'm thinking of 3 possible solutions,  could you please give some
> advices on them:
> >
> > 1. Introduce bootopt of 'poweroff=efi'
> >      Set the pm_power_off to efi_power_off no matter whether there is
> > _S5 or not
> >
> > 2. Introduce /sys/power/poweroff
> >     Allow the user to choose which  pm_power_off, for example:
> >
> > # cat /sys/power/poweroff
> > *acpi		acpi_power_off
> > efi		efi_power_off
> > gpio		gpio_poweroff_do_poweroff
> > user can echo string to enable which one.
> >
> > And two APIs:
> > register_power_off(char *name, power_off func)
> > unregister_power_off(char *name)
> >
> >
> > 3. replace all the codes of  pm_power_off() with
> > reliable_pm_power_off()
> >
> > void reliable_pm_power_off(void)
> > {
> > 	if (!pm_power_off) {
> > 		if (acpi_no_s5)
> > 			pm_power_off = efi_power_off;
> > 	/* Other conditions added in the future. */
> > 	}
> > 	pm_power_off();
> > }
> 
> Be wary of adding all these control knobs. People just want their machines to
> reboot properly without having to mess with boot parameters.
> 
> Let's go back to the start. What prompted this patch? Do Intel have (or are
> planning) machines that do not have _S5 and are expected to use EFI to reset
> the system? 
Yes, there might be a new platform without _S5, and might need EFI poweroff
for a backup.
> Or is this some new configuration discussed in the ACPI spec
> that Linux needs to be support?
> 
> Can we remove the ambiguity and options to force EFI reset if _S5 is missing?
> Afterall, that's why the function is called efi_poweroff_*required*.
OK, the boot option is obsoleted, I'll try another version of default_power_off per
Rafael's suggestion, thanks.

yu