2020-01-16 00:27:23

by Anchal Agarwal

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

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 <[email protected]>
Reviewed-by: Balbir Singh <[email protected]>
Reviewed-by: Frank van der Linden <[email protected]>
---
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);
}


2020-01-16 10:04:00

by Rafael J. Wysocki

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

On Thu, Jan 16, 2020 at 12:26 AM Anchal Agarwal <[email protected]> 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 <[email protected]>
> Reviewed-by: Balbir Singh <[email protected]>
> Reviewed-by: Frank van der Linden <[email protected]>
> ---
> 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.

2020-01-22 19:13:54

by Kaneda, Erik

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



> -----Original Message-----
> From: Rafael J. Wysocki <[email protected]>
> Sent: Thursday, January 16, 2020 1:40 AM
> To: Anchal Agarwal <[email protected]>; Kaneda, Erik
> <[email protected]>; Moore, Robert <[email protected]>
> Cc: Wysocki, Rafael J <[email protected]>; ACPI Devel Maling List
> <[email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>; Len Brown <[email protected]>; open list:ACPI
> COMPONENT ARCHITECTURE (ACPICA) <[email protected]>; Singh, Balbir
> <[email protected]>; [email protected]
> Subject: Re: [RESEND PATCH] ACPICA: Enable sleep button on ACPI legacy
> wake
>
> On Thu, Jan 16, 2020 at 12:26 AM Anchal Agarwal <[email protected]>
> 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 <[email protected]>
> > Reviewed-by: Balbir Singh <[email protected]>
> > Reviewed-by: Frank van der Linden <[email protected]>
> > ---
> > 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