[SRU][Artful][Bionic][Cosmic][PATCH 0/1] UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Artful][Bionic][Cosmic][PATCH 0/1] UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()

Joseph Salisbury-3
BugLink: http://bugs.launchpad.net/bugs/1765646

== SRU Justification ==
A regression was introduced in 4.13-rc1 and newer kernels.  This
regression caused battery drain during system suspend, hibernation or
shutdown for some PCI devices that are not allowed by user space to wake
up the system from sleep (or power off).

This fix has been submitted upstream and cc'd to stable.  However, it
has not landed in linux-next or mainline yet, so it is being sent as
SAUCE.

== Fix ==
UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()

== Regression Potential ==
Medium.  Commit fixes a current regresssion, but affects PCI power management.  
It will also be submitted to upstream stable and have additional review.
The commit is a clean cherry pick, builds successfully and was confirmed
to resolve regression.

== Test Case ==
A test kernel was built with this patch and tested by the original bug reporter.
The bug reporter states the test kernel resolved the bug.

Rafael J. Wysocki (1):
  PCI / PM: Check device_may_wakeup() in pci_enable_wake()

 drivers/pci/pci.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Artful][Bionic][Cosmic][PATCH 1/1] UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()

Joseph Salisbury-3
From: "Rafael J. Wysocki" <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1765646

Commit 0847684cfc5f0 (PCI / PM: Simplify device wakeup settings code)
went too far and dropped the device_may_wakeup() check from
pci_enable_wake() which causes wakeup to be enabled during system
suspend, hibernation or shutdown for some PCI devices that are not
allowed by user space to wake up the system from sleep (or power off).

As a result of this excessive power is drawn by some of the affected
systems while in sleep states or off.

Restore the device_may_wakeup() check in pci_enable_wake(), but make
sure that the PCI bus type's runtime suspend callback will not call
device_may_wakeup() which is about system wakeup from sleep and not
about device wakeup from runtime suspend.

Fixes: 0847684cfc5f0 (PCI / PM: Simplify device wakeup settings code)
Reported-by: Joseph Salisbury <[hidden email]>
Signed-off-by: Rafael J. Wysocki <[hidden email]>
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 drivers/pci/pci.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8eac876..b1faa84 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1893,7 +1893,7 @@ void pci_pme_active(struct pci_dev *dev, bool enable)
 EXPORT_SYMBOL(pci_pme_active);
 
 /**
- * pci_enable_wake - enable PCI device as wakeup event source
+ * __pci_enable_wake - enable PCI device as wakeup event source
  * @dev: PCI device affected
  * @state: PCI state from which device will issue wakeup events
  * @enable: True to enable event generation; false to disable
@@ -1911,7 +1911,7 @@ EXPORT_SYMBOL(pci_pme_active);
  * Error code depending on the platform is returned if both the platform and
  * the native mechanism fail to enable the generation of wake-up events
  */
-int pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable)
+static int __pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable)
 {
  int ret = 0;
 
@@ -1952,6 +1952,23 @@ int pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable)
 
  return ret;
 }
+
+/**
+ * pci_enable_wake - change wakeup settings for a PCI device
+ * @pci_dev: Target device
+ * @state: PCI state from which device will issue wakeup events
+ * @enable: Whether or not to enable event generation
+ *
+ * If @enable is set, check device_may_wakeup() for the device before calling
+ * __pci_enable_wake() for it.
+ */
+int pci_enable_wake(struct pci_dev *pci_dev, pci_power_t state, bool enable)
+{
+ if (enable && !device_may_wakeup(&pci_dev->dev))
+ return -EINVAL;
+
+ return __pci_enable_wake(pci_dev, state, enable);
+}
 EXPORT_SYMBOL(pci_enable_wake);
 
 /**
@@ -1964,9 +1981,9 @@ EXPORT_SYMBOL(pci_enable_wake);
  * should not be called twice in a row to enable wake-up due to PCI PM vs ACPI
  * ordering constraints.
  *
- * This function only returns error code if the device is not capable of
- * generating PME# from both D3_hot and D3_cold, and the platform is unable to
- * enable wake-up power for it.
+ * This function only returns error code if the device is not allowed to wake
+ * up the system from sleep or it is not capable of generating PME# from both
+ * D3_hot and D3_cold and the platform is unable to enable wake-up power for it.
  */
 int pci_wake_from_d3(struct pci_dev *dev, bool enable)
 {
@@ -2097,7 +2114,7 @@ int pci_finish_runtime_suspend(struct pci_dev *dev)
 
  dev->runtime_d3cold = target_state == PCI_D3cold;
 
- pci_enable_wake(dev, target_state, pci_dev_run_wake(dev));
+ __pci_enable_wake(dev, target_state, pci_dev_run_wake(dev));
 
  error = pci_set_power_state(dev, target_state);
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [SRU][Artful][Bionic][Cosmic][PATCH 1/1] UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()

Kleber Souza
On 05/11/18 20:41, Joseph Salisbury wrote:
> From: "Rafael J. Wysocki" <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1765646

The correct BugLink is http://bugs.launchpad.net/bugs/1745646

This can be fixed when applying the patch.

>
> Commit 0847684cfc5f0 (PCI / PM: Simplify device wakeup settings code)
> went too far and dropped the device_may_wakeup() check from
> pci_enable_wake() which causes wakeup to be enabled during system
> suspend, hibernation or shutdown for some PCI devices that are not
> allowed by user space to wake up the system from sleep (or power off).
>
> As a result of this excessive power is drawn by some of the affected
> systems while in sleep states or off.
>
> Restore the device_may_wakeup() check in pci_enable_wake(), but make
> sure that the PCI bus type's runtime suspend callback will not call
> device_may_wakeup() which is about system wakeup from sleep and not
> about device wakeup from runtime suspend.
>
> Fixes: 0847684cfc5f0 (PCI / PM: Simplify device wakeup settings code)
> Reported-by: Joseph Salisbury <[hidden email]>
> Signed-off-by: Rafael J. Wysocki <[hidden email]>
> Signed-off-by: Joseph Salisbury <[hidden email]>

The patch has landed upstream as cfcadfaad7251d8b640713724b388164d75465b2.

We can fix the subject to remove the "UBUNTU: SAUCE:" part and fix the
SOB block when applying the patch.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>


> ---
>  drivers/pci/pci.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8eac876..b1faa84 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1893,7 +1893,7 @@ void pci_pme_active(struct pci_dev *dev, bool enable)
>  EXPORT_SYMBOL(pci_pme_active);
>  
>  /**
> - * pci_enable_wake - enable PCI device as wakeup event source
> + * __pci_enable_wake - enable PCI device as wakeup event source
>   * @dev: PCI device affected
>   * @state: PCI state from which device will issue wakeup events
>   * @enable: True to enable event generation; false to disable
> @@ -1911,7 +1911,7 @@ EXPORT_SYMBOL(pci_pme_active);
>   * Error code depending on the platform is returned if both the platform and
>   * the native mechanism fail to enable the generation of wake-up events
>   */
> -int pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable)
> +static int __pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable)
>  {
>   int ret = 0;
>  
> @@ -1952,6 +1952,23 @@ int pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable)
>  
>   return ret;
>  }
> +
> +/**
> + * pci_enable_wake - change wakeup settings for a PCI device
> + * @pci_dev: Target device
> + * @state: PCI state from which device will issue wakeup events
> + * @enable: Whether or not to enable event generation
> + *
> + * If @enable is set, check device_may_wakeup() for the device before calling
> + * __pci_enable_wake() for it.
> + */
> +int pci_enable_wake(struct pci_dev *pci_dev, pci_power_t state, bool enable)
> +{
> + if (enable && !device_may_wakeup(&pci_dev->dev))
> + return -EINVAL;
> +
> + return __pci_enable_wake(pci_dev, state, enable);
> +}
>  EXPORT_SYMBOL(pci_enable_wake);
>  
>  /**
> @@ -1964,9 +1981,9 @@ EXPORT_SYMBOL(pci_enable_wake);
>   * should not be called twice in a row to enable wake-up due to PCI PM vs ACPI
>   * ordering constraints.
>   *
> - * This function only returns error code if the device is not capable of
> - * generating PME# from both D3_hot and D3_cold, and the platform is unable to
> - * enable wake-up power for it.
> + * This function only returns error code if the device is not allowed to wake
> + * up the system from sleep or it is not capable of generating PME# from both
> + * D3_hot and D3_cold and the platform is unable to enable wake-up power for it.
>   */
>  int pci_wake_from_d3(struct pci_dev *dev, bool enable)
>  {
> @@ -2097,7 +2114,7 @@ int pci_finish_runtime_suspend(struct pci_dev *dev)
>  
>   dev->runtime_d3cold = target_state == PCI_D3cold;
>  
> - pci_enable_wake(dev, target_state, pci_dev_run_wake(dev));
> + __pci_enable_wake(dev, target_state, pci_dev_run_wake(dev));
>  
>   error = pci_set_power_state(dev, target_state);
>  
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Artful][Bionic][Cosmic][PATCH 0/1] UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()

Khaled Elmously
In reply to this post by Joseph Salisbury-3
On 2018-05-11 14:41:52 , Joseph Salisbury wrote:

> BugLink: http://bugs.launchpad.net/bugs/1765646
>
> == SRU Justification ==
> A regression was introduced in 4.13-rc1 and newer kernels.  This
> regression caused battery drain during system suspend, hibernation or
> shutdown for some PCI devices that are not allowed by user space to wake
> up the system from sleep (or power off).
>
> This fix has been submitted upstream and cc'd to stable.  However, it
> has not landed in linux-next or mainline yet, so it is being sent as
> SAUCE.
>
> == Fix ==
> UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()
>
> == Regression Potential ==
> Medium.  Commit fixes a current regresssion, but affects PCI power management.  
> It will also be submitted to upstream stable and have additional review.
> The commit is a clean cherry pick, builds successfully and was confirmed
> to resolve regression.
>
> == Test Case ==
> A test kernel was built with this patch and tested by the original bug reporter.
> The bug reporter states the test kernel resolved the bug.
>
> Rafael J. Wysocki (1):
>   PCI / PM: Check device_may_wakeup() in pci_enable_wake()
>
>  drivers/pci/pci.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
Acked-by: Khalid Elmously <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU][Artful][Bionic][Cosmic][PATCH 0/1] UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()

Khaled Elmously
In reply to this post by Joseph Salisbury-3
Applied to A/B/C - Kleber's comments taken into account


On 2018-05-11 14:41:52 , Joseph Salisbury wrote:

> BugLink: http://bugs.launchpad.net/bugs/1765646
>
> == SRU Justification ==
> A regression was introduced in 4.13-rc1 and newer kernels.  This
> regression caused battery drain during system suspend, hibernation or
> shutdown for some PCI devices that are not allowed by user space to wake
> up the system from sleep (or power off).
>
> This fix has been submitted upstream and cc'd to stable.  However, it
> has not landed in linux-next or mainline yet, so it is being sent as
> SAUCE.
>
> == Fix ==
> UBUNTU: SAUCE: PCI / PM: Check device_may_wakeup() in pci_enable_wake()
>
> == Regression Potential ==
> Medium.  Commit fixes a current regresssion, but affects PCI power management.  
> It will also be submitted to upstream stable and have additional review.
> The commit is a clean cherry pick, builds successfully and was confirmed
> to resolve regression.
>
> == Test Case ==
> A test kernel was built with this patch and tested by the original bug reporter.
> The bug reporter states the test kernel resolved the bug.
>
> Rafael J. Wysocki (1):
>   PCI / PM: Check device_may_wakeup() in pci_enable_wake()
>
>  drivers/pci/pci.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team