From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Furquan Shaikh <furquan@google.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
"Rafael J . Wysocki" <rjw@rjwysocki.net>,
Len Brown <lenb@kernel.org>,
ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux PM <linux-pm@vger.kernel.org>,
Aaron Durbin <adurbin@google.com>,
Duncan Laurie <dlaurie@google.com>
Subject: Re: [RFC] ACPI PM during kernel poweroff/reboot
Date: Wed, 25 Nov 2020 18:51:06 +0100 [thread overview]
Message-ID: <CAJZ5v0j_XWiJyd4zyyuUf41WDEcu5TEo5tT7cYXi8FFqXpBzfA@mail.gmail.com> (raw)
In-Reply-To: <CAEGmHFFxxOxNBjut68azQ5eMh71J+ysJeX9SOak6WwNetuJnwA@mail.gmail.com>
On Wed, Nov 25, 2020 at 6:43 PM Furquan Shaikh <furquan@google.com> wrote:
>
> On Wed, Nov 25, 2020 at 8:39 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Nov 12, 2020 at 8:19 PM Furquan Shaikh <furquan@google.com> wrote:
> > >
> > > On x86 Chromebooks, we have observed this issue for a long time now -
> > > when the system is powered off or rebooted, ACPI PM is not invoked and
> > > this results in PowerResource _OFF methods not being invoked for any
> > > of the devices. The _OFF methods are invoked correctly in case of
> > > suspend-to-idle (S0ix) and suspend-to-memory(S3). However, they do not
> > > get invoked when `poweroff` or `reboot` are triggered.
> > >
> > > One of the differences between suspend, hibernate and shutdown paths
> > > in Linux kernel is that the shutdown path does not use the typical
> > > device PM phases (prepare, freeze/suspend, poweroff) as used by
> > > suspend/hibernate. Instead the shutdown path makes use of
> > > .shutdown_pre() and .shutdown() callbacks.
> > >
> > > If I understand correctly, .shutdown() has been around for a long time
> > > and existed even before the PM callbacks were added. Thus,
> > > pm->poweroff() and .shutdown() are supposed to be analogous and
> > > consistent in the behavior.
> >
> > Well, not quite.
> >
> > ->shutdown() is expected to be a lightweight operation also suitable
> > for kexec() and similar situations where ->poweroff() may not work.
> >
> > > This is why runtime PM is disallowed by
> > > device_shutdown() before it calls .shutdown() (i.e. to keep behavior
> > > consistent for both paths). However, in practice, there are
> > > differences in behavior for the pm->poweroff() and .shutdown() paths
> > > since the shutdown path does not execute any PM domain operations.
> >
> > That's correct.
> >
> > > Because of this difference in behavior, shutdown path never invokes
> > > ACPI PM and thus the ACPI PowerResources are not turned off when the
> > > system is rebooted or powered off (sleep S5). On Chromebooks, it is
> > > critical to run the _OFF methods for poweroff/reboot in order to
> > > ensure that the device power off sequencing requirements are met.
> > > Currently, these requirements are violated which impact the
> > > reliability of devices over the lifetime of the platform.
> > >
> > > There are a few ways in which this can be addressed:
> > >
> > > 1. Similar to the case of hibernation, a new
> > > PMSG_POWEROFF/PM_EVENT_POWEROFF can be introduced to invoke device
> > > power management phases using `dpm_suspend_start(PMSG_POWEROFF)` and
> > > `dpm_suspend_end(PMSG_POWEROFF)`. However, as the shutdown path uses
> > > the class/bus/driver .shutdown() callbacks, adding dpm phases for
> > > poweroff complicates the order of operations. If the dpm phases are
> > > run before .shutdown() callbacks, then it will result in the callbacks
> > > accessing devices after they are powered off. If the .shutdown()
> > > callbacks are run before dpm phases, then the pm->poweroff() calls are
> > > made after the device shutdown is done. Since .shutdown() and
> > > pm->poweroff() are supposed to be analogous, having both calls in the
> > > shutdown path is not only redundant but also results in incorrect
> > > behavior.
> > >
> > > 2. Another option is to update device_shutdown() to make
> > > pm_domain.poweroff calls after the class/bus/driver .shutdown() is
> > > done. However, this suffers from the same problem as #1 above i.e. it
> > > is redundant and creates conflicting order of operations.
> > >
> > > 3. Third possible solution is to detach the device from the PM domain
> > > after it is shutdown. Currently, device drivers perform a detach
> > > operation only when the device is removed. However, in case of
> > > poweroff/reboot as the device is already shutdown, detaching PM domain
> > > will give it the opportunity to ensure that any power resources are
> > > correctly turned off before the system shuts down.
> >
> > 4. Make Chromebooks call something like hibernation_platform_enter()
> > on S5 entries (including reboot).
>
> Actually, Chromebooks do not support S4 and hence CONFIG_HIBERNATION.
This doesn't matter. The ->poweroff callbacks can still be used by
them (of course, that part of the current hibernation support code
needs to be put under a more general Kconfig option for that, but this
is a technical detail).
> This is done for a number of reasons including security. Hence, I
> don't think using hibernation_platform_enter() would be an option.
Yes, it is an option.
Having "hibernation" in the name need not mean that the given piece of
code is really hibernation-specific ...
> >
> > > Out of these, I think #3 makes the most sense as it does not introduce
> > > any conflicting operations. I verified that the following diff results
> > > in _OFF methods getting invoked in both poweroff and reboot cases:
> >
> > This won't work for PCI devices though, only for devices in the ACPI
> > PM domain, so it is not sufficient in general.
>
> That is true. The proposed solution only handles detaching of PM
> domains. I understand your point about this not working for any
> devices not part of the PM domain. The issues that we have observed in
> shutdown/reboot paths have been specific to ACPI power resources
> controlling the sequencing to external devices.
PCI devices PM can use power resources too. For instance, this has
been quite common for discrete GPUs in laptops IIRC.
next prev parent reply other threads:[~2020-11-25 17:51 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-12 19:19 [RFC] ACPI PM during kernel poweroff/reboot Furquan Shaikh
2020-11-25 15:11 ` Furquan Shaikh
2020-11-25 16:39 ` Rafael J. Wysocki
2020-11-25 17:43 ` Furquan Shaikh
2020-11-25 17:51 ` Rafael J. Wysocki [this message]
2020-11-25 18:29 ` Furquan Shaikh
2020-12-01 21:38 ` Furquan Shaikh
2020-12-02 11:45 ` Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJZ5v0j_XWiJyd4zyyuUf41WDEcu5TEo5tT7cYXi8FFqXpBzfA@mail.gmail.com \
--to=rafael@kernel.org \
--cc=adurbin@google.com \
--cc=dlaurie@google.com \
--cc=furquan@google.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).