Re: [RFC] ACPI PM during kernel poweroff/reboot
On Tue, Dec 1, 2020 at 10:38 PM Furquan Shaikh wrote: > > On Wed, Nov 25, 2020 at 10:29 AM Furquan Shaikh wrote: > > > > On Wed, Nov 25, 2020 at 9:51 AM Rafael J. Wysocki wrote: > > > > > > On Wed, Nov 25, 2020 at 6:43 PM Furquan Shaikh wrote: > > > > > > > > On Wed, Nov 25, 2020 at 8:39 AM Rafael J. Wysocki > > > > wrote: > > > > > > > > > > On Thu, Nov 12, 2020 at 8:19 PM Furquan Shaikh > > > > > 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
Re: [RFC] ACPI PM during kernel poweroff/reboot
On Wed, Nov 25, 2020 at 10:29 AM Furquan Shaikh wrote: > > On Wed, Nov 25, 2020 at 9:51 AM Rafael J. Wysocki wrote: > > > > On Wed, Nov 25, 2020 at 6:43 PM Furquan Shaikh wrote: > > > > > > On Wed, Nov 25, 2020 at 8:39 AM Rafael J. Wysocki > > > wrote: > > > > > > > > On Thu, Nov 12, 2020 at 8:19 PM Furquan Shaikh > > > > 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). > > Ah I see what you are saying. Just to be sure I understand this > correctly. Is this what you are thinking: > 1. Extract hibernation_platform_enter() and any other helpers required > to trigger the
Re: [RFC] ACPI PM during kernel poweroff/reboot
On Wed, Nov 25, 2020 at 9:51 AM Rafael J. Wysocki wrote: > > On Wed, Nov 25, 2020 at 6:43 PM Furquan Shaikh wrote: > > > > On Wed, Nov 25, 2020 at 8:39 AM Rafael J. Wysocki wrote: > > > > > > On Thu, Nov 12, 2020 at 8:19 PM Furquan Shaikh 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). Ah I see what you are saying. Just to be sure I understand this correctly. Is this what you are thinking: 1. Extract hibernation_platform_enter() and any other helpers required to trigger the PM phases for shutdown into a separate unit controlled by a more general Kconfig. 2. Add a new Kconfig that enables support for performing PM phases during the poweroff/reboot phases. 3. Based on this new Kconfig selection, LINUX_REBOOT_CMD_RESTART,
Re: [RFC] ACPI PM during kernel poweroff/reboot
On Wed, Nov 25, 2020 at 6:43 PM Furquan Shaikh wrote: > > On Wed, Nov 25, 2020 at 8:39 AM Rafael J. Wysocki wrote: > > > > On Thu, Nov 12, 2020 at 8:19 PM Furquan Shaikh 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
Re: [RFC] ACPI PM during kernel poweroff/reboot
On Wed, Nov 25, 2020 at 8:39 AM Rafael J. Wysocki wrote: > > On Thu, Nov 12, 2020 at 8:19 PM Furquan Shaikh 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 is done for a number of reasons including security. Hence, I don't think using hibernation_platform_enter() would be an option. > > > 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. > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > index 94df2ba1bbed..e55d65fbb4a9 100644 > > --- a/drivers/base/core.c > > +++ b/drivers/base/core.c > > @@ -23,6 +23,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -3230,6
Re: [RFC] ACPI PM during kernel poweroff/reboot
On Thu, Nov 12, 2020 at 8:19 PM Furquan Shaikh 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). > 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. > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 94df2ba1bbed..e55d65fbb4a9 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -23,6 +23,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -3230,6 +3231,8 @@ void device_shutdown(void) > dev->driver->shutdown(dev); > } > > + dev_pm_domain_detach(dev, true); > + It generally makes sense to do this, because ->shutdown() is sort of analogous to ->remove() from the driver model perspective, so if it is sufficient for you, please feel free to send a formal patch with that change. > device_unlock(dev); > if (parent) > device_unlock(parent); > > This was discussed on the mailing list some time back[1] in the > context of a different use case. However, the idea of detaching > devices (on any bus) from the PM domain during shutdown is important > to ensure correct power sequencing for the
Re: [RFC] ACPI PM during kernel poweroff/reboot
On Thu, Nov 12, 2020 at 11:19 AM Furquan Shaikh 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. 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. > > 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. > > 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: > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 94df2ba1bbed..e55d65fbb4a9 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -23,6 +23,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -3230,6 +3231,8 @@ void device_shutdown(void) > dev->driver->shutdown(dev); > } > > + dev_pm_domain_detach(dev, true); > + > device_unlock(dev); > if (parent) > device_unlock(parent); > > This was discussed on the mailing list some time back[1] in the > context of a different use case. However, the idea of detaching > devices (on any bus) from the PM domain during shutdown is important > to ensure correct power sequencing for the devices. > > One of the concerns that was raised on the above thread was slowing > down the shutdown process when not needed. I think this can be handled > by adding a sysfs attribute to allow platforms to decide if they need > the ability to power off PM domains on shutdown/reboot path. > > Questions that I am looking to get feedback/comments are: > > 1. Is my assessment of the problem and understanding of the > .shutdown() and pm.poweroff() correct? > 2. Does the solution #3 i.e. detaching PM domain after shutting down > device on shutdown path makes sense? > 3. Are there other possible approaches to solve
