Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>, Keith Busch <keith.busch@intel.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Alexandru Gagniuc <mr.nuke.me@gmail.com>,
	linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH 2/3] PCI: Do not poll for PME if the device is in D3cold
Date: Sun, 9 Jun 2019 20:38:28 +0200
Message-ID: <20190609183828.eyjxg32bpwswkg2u@wunner.de> (raw)
In-Reply-To: <20190605145820.37169-3-mika.westerberg@linux.intel.com>

On Wed, Jun 05, 2019 at 05:58:19PM +0300, Mika Westerberg wrote:
> PME polling does not take into account that a device that is directly
> connected to the host bridge may go into D3cold as well. This leads to a
> situation where the PME poll thread reads from a config space of a
> device that is in D3cold and gets incorrect information because the
> config space is not accessible.
> 
> Here is an example from Intel Ice Lake system where two PCIe root ports
> are in D3cold (I've instrumented the kernel to log the PMCSR register
> contents):
> 
>   [   62.971442] pcieport 0000:00:07.1: Check PME status, PMCSR=0xffff
>   [   62.971504] pcieport 0000:00:07.0: Check PME status, PMCSR=0xffff
> 
> Since 0xffff is interpreted so that PME is pending, the root ports will
> be runtime resumed. This repeats over and over again essentially
> blocking all runtime power management.
> 
> Prevent this from happening by checking whether the device is in D3cold
> before its PME status is read.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Lukas Wunner <lukas@wunner.de>
Fixes: 71a83bd727cc ("PCI/PM: add runtime PM support to PCIe port")
Cc: stable@vger.kernel.org # v3.6+

Although the patch I've posted today (which checks for an "all ones" read
from config space) covers the issue fixed herein, your patch still makes
sense to avoid unnecessarily accessing config space in the first place.

Thanks,

Lukas

> ---
>  drivers/pci/pci.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 87a1f902fa8e..720da09d4d73 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2060,6 +2060,13 @@ static void pci_pme_list_scan(struct work_struct *work)
>  			 */
>  			if (bridge && bridge->current_state != PCI_D0)
>  				continue;
> +			/*
> +			 * If the device is in D3cold it should not be
> +			 * polled either.
> +			 */
> +			if (pme_dev->dev->current_state == PCI_D3cold)
> +				continue;
> +
>  			pci_pme_wakeup(pme_dev->dev, NULL);
>  		} else {
>  			list_del(&pme_dev->list);
> -- 
> 2.20.1
> 

  parent reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-05 14:58 [PATCH 0/3] PCI: Power management improvements Mika Westerberg
2019-06-05 14:58 ` [PATCH 1/3] PCI: Add missing link delays required by the PCIe spec Mika Westerberg
2019-06-06  8:40   ` Rafael J. Wysocki
2019-06-06 11:24     ` Mika Westerberg
2019-06-05 14:58 ` [PATCH 2/3] PCI: Do not poll for PME if the device is in D3cold Mika Westerberg
2019-06-05 19:05   ` Lukas Wunner
2019-06-06 11:21     ` Mika Westerberg
2019-06-09 18:38   ` Lukas Wunner [this message]
2019-06-10 11:35   ` Rafael J. Wysocki
2019-06-05 14:58 ` [PATCH 3/3] PCI / ACPI: Handle sibling devices sharing power resources Mika Westerberg
2019-06-06  8:54   ` Rafael J. Wysocki
2019-06-06 11:26     ` Mika Westerberg
2019-06-06 13:44       ` Mika Westerberg
2019-06-06 14:08         ` Rafael J. Wysocki
2019-06-06 14:17           ` Mika Westerberg
2019-06-06 14:27             ` Rafael J. Wysocki
2019-06-06 14:36               ` Mika Westerberg
2019-06-12 22:38                 ` Rafael J. Wysocki
2019-06-13 12:52                   ` Mika Westerberg
2019-06-13 13:51                     ` Rafael J. Wysocki
2019-06-13 14:27                       ` Mika Westerberg
2019-06-09 18:58               ` Lukas Wunner
2019-06-10 10:57                 ` Rafael J. Wysocki

Reply instructions:

You may reply publically 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=20190609183828.eyjxg32bpwswkg2u@wunner.de \
    --to=lukas@wunner.de \
    --cc=alex.williamson@redhat.com \
    --cc=bhelgaas@google.com \
    --cc=keith.busch@intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=mr.nuke.me@gmail.com \
    --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

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org linux-acpi@archiver.kernel.org
	public-inbox-index linux-acpi


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/ public-inbox