* Re: [PATCH] ACPICA: Enable sleep button on ACPI legacy wake [not found] ` <CF6A88132359CE47947DB4C6E1709ED53C66675F@ORSMSX122.amr.corp.intel.com> @ 2019-11-01 22:26 ` Agarwal, Anchal 2019-11-04 17:58 ` Agarwal, Anchal 0 siblings, 1 reply; 8+ messages in thread From: Agarwal, Anchal @ 2019-11-01 22:26 UTC (permalink / raw) To: Schmauss, Erik; +Cc: Moore, Robert, Wysocki, Rafael J, linux-acpi + Rafael, Bob Also, is there a reason why this wasn't CC-ed to linux-acpi mailing list? It would be good to get feedback from there too.. I think I did to all the email id's generated from get_maintainer.pl. Ccing again + linux-acpi Thanks, Erik > -----Original Message----- > From: Anchal Agarwal <anchalag@amazon.com> > Sent: Monday, October 29, 2018 10:52 AM > Subject: [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 <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); > } > -- > 2.15.3.AMZN ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ACPICA: Enable sleep button on ACPI legacy wake 2019-11-01 22:26 ` [PATCH] ACPICA: Enable sleep button on ACPI legacy wake Agarwal, Anchal @ 2019-11-04 17:58 ` Agarwal, Anchal 2019-11-08 17:56 ` Anchal Agarwal 0 siblings, 1 reply; 8+ messages in thread From: Agarwal, Anchal @ 2019-11-04 17:58 UTC (permalink / raw) To: Schmauss, Erik Cc: Moore, Robert, Wysocki, Rafael J, linux-acpi, linux-kernel, lenb, devel Apologies for resending it. Looks like it does not show in cc list it went to all mailing list however the thread do appear on lkml. Thanks, Anchal + Rafael, Bob Also, is there a reason why this wasn't CC-ed to linux-acpi mailing list? It would be good to get feedback from there too.. I think I did to all the email id's generated from get_maintainer.pl. Ccing again + linux-acpi Thanks, Erik > -----Original Message----- > From: Anchal Agarwal <anchalag@amazon.com> > Sent: Monday, October 29, 2018 10:52 AM > Subject: [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 <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); > } > -- > 2.15.3.AMZN ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ACPICA: Enable sleep button on ACPI legacy wake 2019-11-04 17:58 ` Agarwal, Anchal @ 2019-11-08 17:56 ` Anchal Agarwal 0 siblings, 0 replies; 8+ messages in thread From: Anchal Agarwal @ 2019-11-08 17:56 UTC (permalink / raw) To: Schmauss, Erik, robert.moore, rafael.j.wysocki, linux-acpi, linux-kernel, lenb, devel Gentle ping on this one. -- Anchal On Mon, Nov 04, 2019 at 09:58:16AM -0800, Agarwal, Anchal wrote: > Apologies for resending it. Looks like it does not show in cc list it went to all mailing list however the thread do appear on lkml. > Thanks, > Anchal > > + Rafael, Bob > > Also, is there a reason why this wasn't CC-ed to linux-acpi mailing list? It would be good to get feedback from there too.. > I think I did to all the email id's generated from get_maintainer.pl. Ccing again > + linux-acpi > > Thanks, > Erik > > > -----Original Message----- > > From: Anchal Agarwal <anchalag@amazon.com> > > Sent: Monday, October 29, 2018 10:52 AM > > Subject: [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 <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); > > } > > -- > > 2.15.3.AMZN > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ACPICA: Enable sleep button on ACPI legacy wake [not found] <20191101212319.1FFC440EB1@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> [not found] ` <CF6A88132359CE47947DB4C6E1709ED53C66675F@ORSMSX122.amr.corp.intel.com> @ 2019-12-12 22:10 ` Rafael J. Wysocki 2020-01-02 22:39 ` Anchal Agarwal 1 sibling, 1 reply; 8+ messages in thread From: Rafael J. Wysocki @ 2019-12-12 22:10 UTC (permalink / raw) To: Anchal Agarwal Cc: Balbir Singh, Frank van der Linden, Linux ACPI, Erik Kaneda On Monday, October 29, 2018 6:52:16 PM CET Anchal Agarwal 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. I'm not against this change in principle, although it may change behavior in a somewhat unexpected way on some systems. > This causes subsequent invocation of sleep state to fail > in the guest. However, can you explain this in a bit more detail? > 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); > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ACPICA: Enable sleep button on ACPI legacy wake 2019-12-12 22:10 ` Rafael J. Wysocki @ 2020-01-02 22:39 ` Anchal Agarwal 2020-01-15 10:24 ` Rafael J. Wysocki 0 siblings, 1 reply; 8+ messages in thread From: Anchal Agarwal @ 2020-01-02 22:39 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: sblbir, fllinden, linux-acpi, erik.kaneda On Thu, Dec 12, 2019 at 11:10:32PM +0100, Rafael J. Wysocki wrote: > On Monday, October 29, 2018 6:52:16 PM CET Anchal Agarwal 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. > > I'm not against this change in principle, although it may change behavior > in a somewhat unexpected way on some systems. > Apologies for late response, I was out on vacation. Can you please elaborate more on that? > > This causes subsequent invocation of sleep state to fail > > in the guest. > > However, can you explain this in a bit more detail? > I can explain this for my particular case, however, any code path going through acpi_hw_legacy_wake path will not have sleep button enabled. Why is only power button event enabled and not sleep here? In my case, I am using sleep button event to generate S4 state which causes guest VM running on xen to hibernate. I have found in my debugging process that after first resume when kernel goes through this code path, sleep button event is not enabled and when hypervisor try to generate sci interrupt, its not received in the guest since this path does not enable sleep event. Thanks, Anchal > > 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); > > } > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ACPICA: Enable sleep button on ACPI legacy wake 2020-01-02 22:39 ` Anchal Agarwal @ 2020-01-15 10:24 ` Rafael J. Wysocki 2020-01-15 18:58 ` Anchal Agarwal 0 siblings, 1 reply; 8+ messages in thread From: Rafael J. Wysocki @ 2020-01-15 10:24 UTC (permalink / raw) To: Anchal Agarwal; +Cc: sblbir, fllinden, linux-acpi, erik.kaneda, Bob Moore On Thursday, January 2, 2020 11:39:19 PM CET Anchal Agarwal wrote: > On Thu, Dec 12, 2019 at 11:10:32PM +0100, Rafael J. Wysocki wrote: > > On Monday, October 29, 2018 6:52:16 PM CET Anchal Agarwal 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. > > > > I'm not against this change in principle, although it may change behavior > > in a somewhat unexpected way on some systems. > > > Apologies for late response, I was out on vacation. > Can you please elaborate more on that? See below. > > > This causes subsequent invocation of sleep state to fail > > > in the guest. > > > > However, can you explain this in a bit more detail? > > > I can explain this for my particular case, however, any code path going > through acpi_hw_legacy_wake path will not have sleep button enabled. Why > is only power button event enabled and not sleep here? Good question and related to the potentially unexpected behavior part. Presumably, on some systems the sleep button could not be enabled automatically as it triggered another suspend right away after that. That's speculation, though. > In my case, I am using sleep button event to generate S4 state > which causes guest VM running on xen to hibernate. Usually, however, the sleep button is used to trigger transitions into S3 (or suspend-to-idle on systems that support it). > I have found > in my debugging process that after first resume when kernel goes > through this code path, sleep button event is not enabled and > when hypervisor try to generate sci interrupt, its not received in > the guest since this path does not enable sleep event. OK So the patch can be applied AFAICS, but if it is reported to cause problems to happen, it will need to be reverted. Please resend the patch with a proper CC list (including Bob, Erik, the ACPICA devel list etc). Thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ACPICA: Enable sleep button on ACPI legacy wake 2020-01-15 10:24 ` Rafael J. Wysocki @ 2020-01-15 18:58 ` Anchal Agarwal 2020-01-15 22:57 ` Rafael J. Wysocki 0 siblings, 1 reply; 8+ messages in thread From: Anchal Agarwal @ 2020-01-15 18:58 UTC (permalink / raw) To: Rafael J. Wysocki Cc: sblbir, fllinden, linux-acpi, erik.kaneda, Bob Moore, anchalag On Wed, Jan 15, 2020 at 11:24:47AM +0100, Rafael J. Wysocki wrote: > On Thursday, January 2, 2020 11:39:19 PM CET Anchal Agarwal wrote: > > On Thu, Dec 12, 2019 at 11:10:32PM +0100, Rafael J. Wysocki wrote: > > > On Monday, October 29, 2018 6:52:16 PM CET Anchal Agarwal 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. > > > > > > I'm not against this change in principle, although it may change behavior > > > in a somewhat unexpected way on some systems. > > > > > Apologies for late response, I was out on vacation. > > Can you please elaborate more on that? > > See below. > > > > > This causes subsequent invocation of sleep state to fail > > > > in the guest. > > > > > > However, can you explain this in a bit more detail? > > > > > I can explain this for my particular case, however, any code path going > > through acpi_hw_legacy_wake path will not have sleep button enabled. Why > > is only power button event enabled and not sleep here? > > Good question and related to the potentially unexpected behavior part. > > Presumably, on some systems the sleep button could not be enabled > automatically as it triggered another suspend right away after that. > > That's speculation, though. > > > In my case, I am using sleep button event to generate S4 state > > which causes guest VM running on xen to hibernate. > > Usually, however, the sleep button is used to trigger transitions into S3 > (or suspend-to-idle on systems that support it). > > > I have found > > in my debugging process that after first resume when kernel goes > > through this code path, sleep button event is not enabled and > > when hypervisor try to generate sci interrupt, its not received in > > the guest since this path does not enable sleep event. > > OK > > So the patch can be applied AFAICS, but if it is reported to cause problems to > happen, it will need to be reverted. > > Please resend the patch with a proper CC list (including Bob, Erik, the ACPICA > devel list etc). > > Thanks! Sure can I add your Reviewed-by or Acked-by? Thanks, Anchal > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ACPICA: Enable sleep button on ACPI legacy wake 2020-01-15 18:58 ` Anchal Agarwal @ 2020-01-15 22:57 ` Rafael J. Wysocki 0 siblings, 0 replies; 8+ messages in thread From: Rafael J. Wysocki @ 2020-01-15 22:57 UTC (permalink / raw) To: Anchal Agarwal Cc: Rafael J. Wysocki, Singh, Balbir, fllinden, ACPI Devel Maling List, Erik Kaneda, Bob Moore On Wed, Jan 15, 2020 at 7:58 PM Anchal Agarwal <anchalag@amazon.com> wrote: > > On Wed, Jan 15, 2020 at 11:24:47AM +0100, Rafael J. Wysocki wrote: > > On Thursday, January 2, 2020 11:39:19 PM CET Anchal Agarwal wrote: > > > On Thu, Dec 12, 2019 at 11:10:32PM +0100, Rafael J. Wysocki wrote: > > > > On Monday, October 29, 2018 6:52:16 PM CET Anchal Agarwal 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. > > > > > > > > I'm not against this change in principle, although it may change behavior > > > > in a somewhat unexpected way on some systems. > > > > > > > Apologies for late response, I was out on vacation. > > > Can you please elaborate more on that? > > > > See below. > > > > > > > This causes subsequent invocation of sleep state to fail > > > > > in the guest. > > > > > > > > However, can you explain this in a bit more detail? > > > > > > > I can explain this for my particular case, however, any code path going > > > through acpi_hw_legacy_wake path will not have sleep button enabled. Why > > > is only power button event enabled and not sleep here? > > > > Good question and related to the potentially unexpected behavior part. > > > > Presumably, on some systems the sleep button could not be enabled > > automatically as it triggered another suspend right away after that. > > > > That's speculation, though. > > > > > In my case, I am using sleep button event to generate S4 state > > > which causes guest VM running on xen to hibernate. > > > > Usually, however, the sleep button is used to trigger transitions into S3 > > (or suspend-to-idle on systems that support it). > > > > > I have found > > > in my debugging process that after first resume when kernel goes > > > through this code path, sleep button event is not enabled and > > > when hypervisor try to generate sci interrupt, its not received in > > > the guest since this path does not enable sleep event. > > > > OK > > > > So the patch can be applied AFAICS, but if it is reported to cause problems to > > happen, it will need to be reverted. > > > > Please resend the patch with a proper CC list (including Bob, Erik, the ACPICA > > devel list etc). > > > > Thanks! > Sure can I add your Reviewed-by or Acked-by? I'll be applying this eventually anyway, so why? ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-15 22:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20191101212319.1FFC440EB1@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com> [not found] ` <CF6A88132359CE47947DB4C6E1709ED53C66675F@ORSMSX122.amr.corp.intel.com> 2019-11-01 22:26 ` [PATCH] ACPICA: Enable sleep button on ACPI legacy wake Agarwal, Anchal 2019-11-04 17:58 ` Agarwal, Anchal 2019-11-08 17:56 ` Anchal Agarwal 2019-12-12 22:10 ` Rafael J. Wysocki 2020-01-02 22:39 ` Anchal Agarwal 2020-01-15 10:24 ` Rafael J. Wysocki 2020-01-15 18:58 ` Anchal Agarwal 2020-01-15 22:57 ` Rafael J. Wysocki
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).