[RESEND PATCH] ACPICA: Enable sleep button on ACPI legacy wake

[RESEND PATCH] ACPICA: Enable sleep button on ACPI legacy wake

[Anchal Agarwal]

Currently we do not see sleep_enable bit set after guest resumes
from hibernation. Hibernation is triggered in guest on receiving
a sleep trigger from the hypervisor(S4 state). We see that power
button is enabled on wake up from S4 state however sleep button
isn't. This causes subsequent invocation of sleep state to fail
in the guest. Any environment  going through acpi_hw_legacy_wake()
won't have sleep button enabled.

Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
Reviewed-by: Balbir Singh <sblbir@amazon.com>
Reviewed-by: Frank van der Linden <fllinden@amazon.com>
---
 drivers/acpi/acpica/hwsleep.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c
index b62db8ec446f..a176c7802760 100644
--- a/drivers/acpi/acpica/hwsleep.c
+++ b/drivers/acpi/acpica/hwsleep.c
@@ -300,6 +300,17 @@ acpi_status acpi_hw_legacy_wake(u8 sleep_state)
 				    [ACPI_EVENT_POWER_BUTTON].
 				    status_register_id, ACPI_CLEAR_STATUS);
 
+	/* Enable sleep button */
+	(void)
+	      acpi_write_bit_register(acpi_gbl_fixed_event_info
+				      [ACPI_EVENT_SLEEP_BUTTON].
+				      enable_register_id, ACPI_ENABLE_EVENT);
+
+	(void)
+	      acpi_write_bit_register(acpi_gbl_fixed_event_info
+				      [ACPI_EVENT_SLEEP_BUTTON].
+				      status_register_id, ACPI_CLEAR_STATUS);
+
 	acpi_hw_execute_sleep_method(METHOD_PATHNAME__SST, ACPI_SST_WORKING);
 	return_ACPI_STATUS(status);
 }

Re: [RESEND PATCH] ACPICA: Enable sleep button on ACPI legacy wake

[Rafael J. Wysocki]

On Thu, Jan 16, 2020 at 12:26 AM Anchal Agarwal <anchalag@amazon.com> wrote:
>
> Currently we do not see sleep_enable bit set after guest resumes
> from hibernation. Hibernation is triggered in guest on receiving
> a sleep trigger from the hypervisor(S4 state). We see that power
> button is enabled on wake up from S4 state however sleep button
> isn't. This causes subsequent invocation of sleep state to fail
> in the guest. Any environment  going through acpi_hw_legacy_wake()
> won't have sleep button enabled.
>
> Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
> Reviewed-by: Balbir Singh <sblbir@amazon.com>
> Reviewed-by: Frank van der Linden <fllinden@amazon.com>
> ---
>  drivers/acpi/acpica/hwsleep.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c
> index b62db8ec446f..a176c7802760 100644
> --- a/drivers/acpi/acpica/hwsleep.c
> +++ b/drivers/acpi/acpica/hwsleep.c
> @@ -300,6 +300,17 @@ acpi_status acpi_hw_legacy_wake(u8 sleep_state)
>                                     [ACPI_EVENT_POWER_BUTTON].
>                                     status_register_id, ACPI_CLEAR_STATUS);
>
> +       /* Enable sleep button */
> +       (void)
> +             acpi_write_bit_register(acpi_gbl_fixed_event_info
> +                                     [ACPI_EVENT_SLEEP_BUTTON].
> +                                     enable_register_id, ACPI_ENABLE_EVENT);
> +
> +       (void)
> +             acpi_write_bit_register(acpi_gbl_fixed_event_info
> +                                     [ACPI_EVENT_SLEEP_BUTTON].
> +                                     status_register_id, ACPI_CLEAR_STATUS);
> +
>         acpi_hw_execute_sleep_method(METHOD_PATHNAME__SST, ACPI_SST_WORKING);
>         return_ACPI_STATUS(status);
>  }

Erik, Bob, please pick this up if you don't have specific objections against it.

I'll wait for it to show up in an upstream release.

[Devel] Re: [RESEND PATCH] ACPICA: Enable sleep button on ACPI legacy wake

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]

On Thu, Jan 16, 2020 at 12:26 AM Anchal Agarwal <anchalag(a)amazon.com> wrote:
>
> Currently we do not see sleep_enable bit set after guest resumes
> from hibernation. Hibernation is triggered in guest on receiving
> a sleep trigger from the hypervisor(S4 state). We see that power
> button is enabled on wake up from S4 state however sleep button
> isn't. This causes subsequent invocation of sleep state to fail
> in the guest. Any environment  going through acpi_hw_legacy_wake()
> won't have sleep button enabled.
>
> Signed-off-by: Anchal Agarwal <anchalag(a)amazon.com>
> Reviewed-by: Balbir Singh <sblbir(a)amazon.com>
> Reviewed-by: Frank van der Linden <fllinden(a)amazon.com>
> ---
>  drivers/acpi/acpica/hwsleep.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c
> index b62db8ec446f..a176c7802760 100644
> --- a/drivers/acpi/acpica/hwsleep.c
> +++ b/drivers/acpi/acpica/hwsleep.c
> @@ -300,6 +300,17 @@ acpi_status acpi_hw_legacy_wake(u8 sleep_state)
>                                     [ACPI_EVENT_POWER_BUTTON].
>                                     status_register_id, ACPI_CLEAR_STATUS);
>
> +       /* Enable sleep button */
> +       (void)
> +             acpi_write_bit_register(acpi_gbl_fixed_event_info
> +                                     [ACPI_EVENT_SLEEP_BUTTON].
> +                                     enable_register_id, ACPI_ENABLE_EVENT);
> +
> +       (void)
> +             acpi_write_bit_register(acpi_gbl_fixed_event_info
> +                                     [ACPI_EVENT_SLEEP_BUTTON].
> +                                     status_register_id, ACPI_CLEAR_STATUS);
> +
>         acpi_hw_execute_sleep_method(METHOD_PATHNAME__SST, ACPI_SST_WORKING);
>         return_ACPI_STATUS(status);
>  }

Erik, Bob, please pick this up if you don't have specific objections against it.

I'll wait for it to show up in an upstream release.

RE: [RESEND PATCH] ACPICA: Enable sleep button on ACPI legacy wake

[Kaneda, Erik]

> -----Original Message-----
> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Thursday, January 16, 2020 1:40 AM
> To: Anchal Agarwal <anchalag@amazon.com>; Kaneda, Erik
> <erik.kaneda@intel.com>; Moore, Robert <robert.moore@intel.com>
> Cc: Wysocki, Rafael J <rafael.j.wysocki@intel.com>; ACPI Devel Maling List
> <linux-acpi@vger.kernel.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Len Brown <lenb@kernel.org>; open list:ACPI
> COMPONENT ARCHITECTURE (ACPICA) <devel@acpica.org>; Singh, Balbir
> <sblbir@amazon.com>; fllinden@amazon.com
> Subject: Re: [RESEND PATCH] ACPICA: Enable sleep button on ACPI legacy
> wake
> 
> On Thu, Jan 16, 2020 at 12:26 AM Anchal Agarwal <anchalag@amazon.com>
> wrote:
> >
> > Currently we do not see sleep_enable bit set after guest resumes from
> > hibernation. Hibernation is triggered in guest on receiving a sleep
> > trigger from the hypervisor(S4 state). We see that power button is
> > enabled on wake up from S4 state however sleep button isn't. This
> > causes subsequent invocation of sleep state to fail in the guest. Any
> > environment  going through acpi_hw_legacy_wake() won't have sleep
> > button enabled.
> >
> > Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
> > Reviewed-by: Balbir Singh <sblbir@amazon.com>
> > Reviewed-by: Frank van der Linden <fllinden@amazon.com>
> > ---
> >  drivers/acpi/acpica/hwsleep.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/acpi/acpica/hwsleep.c
> > b/drivers/acpi/acpica/hwsleep.c index b62db8ec446f..a176c7802760
> > 100644
> > --- a/drivers/acpi/acpica/hwsleep.c
> > +++ b/drivers/acpi/acpica/hwsleep.c
> > @@ -300,6 +300,17 @@ acpi_status acpi_hw_legacy_wake(u8 sleep_state)
> >                                     [ACPI_EVENT_POWER_BUTTON].
> >                                     status_register_id,
> > ACPI_CLEAR_STATUS);
> >
> > +       /* Enable sleep button */
> > +       (void)
> > +             acpi_write_bit_register(acpi_gbl_fixed_event_info
> > +                                     [ACPI_EVENT_SLEEP_BUTTON].
> > +                                     enable_register_id,
> > + ACPI_ENABLE_EVENT);
> > +
> > +       (void)
> > +             acpi_write_bit_register(acpi_gbl_fixed_event_info
> > +                                     [ACPI_EVENT_SLEEP_BUTTON].
> > +                                     status_register_id,
> > + ACPI_CLEAR_STATUS);
> > +
> >         acpi_hw_execute_sleep_method(METHOD_PATHNAME__SST,
> ACPI_SST_WORKING);
> >         return_ACPI_STATUS(status);
> >  }
> 
> Erik, Bob, please pick this up if you don't have specific objections against it.
> 
> I'll wait for it to show up in an upstream release.

I've submitted a pull request to ACPICA upstream with changes to the commit message here:
https://github.com/acpica/acpica/pull/549

I'll send a Linux-ized version of this patch after our next ACPICA release.

Thanks,
Erik