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

["Rafael J. Wysocki" <rafael@kernel.org>]

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?