All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: jesse.brandeburg@intel.com, anthony.l.nguyen@intel.com
Cc: acelan.kao@canonical.com,
	Kai-Heng Feng <kai.heng.feng@canonical.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	intel-wired-lan@lists.osuosl.org (moderated list:INTEL ETHERNET
	DRIVERS), netdev@vger.kernel.org (open list:NETWORKING DRIVERS),
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH 3/3] e1000e: Serialize TGP e1000e PM ops
Date: Mon, 12 Jul 2021 21:34:59 +0800	[thread overview]
Message-ID: <20210712133500.1126371-3-kai.heng.feng@canonical.com> (raw)
In-Reply-To: <20210712133500.1126371-1-kai.heng.feng@canonical.com>

On TGL systems, PCI_COMMAND may randomly flip to 0 on system resume.
This is devastating to drivers that use pci_set_master(), like NVMe and
xHCI, to enable DMA in their resume routine, as pci_set_master() can
inadvertently disable PCI_COMMAND_IO and PCI_COMMAND_MEMORY, making
resources inaccessible.

The issue is reproducible on all kernel releases, but the situation is
exacerbated by commit 6cecf02e77ab ("Revert "e1000e: disable s0ix entry
and exit flows for ME systems"").

Seems like ME can do many things to other PCI devices until it's finally out of
ULP polling. So ensure e1000e PM ops are serialized by enforcing suspend/resume
order to workaround the issue.

Of course this will make system suspend and resume a bit slower, but we
probably need to settle on this workaround until ME is fully supported.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=212039
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e63445a8ce12..0244d3dd90a3 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7319,7 +7319,8 @@ static const struct net_device_ops e1000e_netdev_ops = {
 
 static void e1000e_create_device_links(struct pci_dev *pdev)
 {
-	struct pci_dev *tgp_mei_me;
+	struct pci_bus *bus = pdev->bus;
+	struct pci_dev *tgp_mei_me, *p;
 
 	/* Find TGP mei_me devices and make e1000e power depend on mei_me */
 	tgp_mei_me = pci_get_device(PCI_VENDOR_ID_INTEL, 0xa0e0, NULL);
@@ -7335,6 +7336,17 @@ static void e1000e_create_device_links(struct pci_dev *pdev)
 		pci_info(pdev, "System and runtime PM depends on %s\n",
 			 pci_name(tgp_mei_me));
 
+	/* Find other devices in the SoC and make them depend on e1000e */
+	list_for_each_entry(p, &bus->devices, bus_list) {
+		if (&p->dev == &pdev->dev || &p->dev == &tgp_mei_me->dev)
+			continue;
+
+		if (device_link_add(&p->dev, &pdev->dev,
+				    DL_FLAG_AUTOREMOVE_SUPPLIER))
+			pci_info(p, "System PM depends on %s\n",
+				 pci_name(pdev));
+	}
+
 	pci_dev_put(tgp_mei_me);
 }
 
-- 
2.31.1


WARNING: multiple messages have this Message-ID (diff)
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH 3/3] e1000e: Serialize TGP e1000e PM ops
Date: Mon, 12 Jul 2021 21:34:59 +0800	[thread overview]
Message-ID: <20210712133500.1126371-3-kai.heng.feng@canonical.com> (raw)
In-Reply-To: <20210712133500.1126371-1-kai.heng.feng@canonical.com>

On TGL systems, PCI_COMMAND may randomly flip to 0 on system resume.
This is devastating to drivers that use pci_set_master(), like NVMe and
xHCI, to enable DMA in their resume routine, as pci_set_master() can
inadvertently disable PCI_COMMAND_IO and PCI_COMMAND_MEMORY, making
resources inaccessible.

The issue is reproducible on all kernel releases, but the situation is
exacerbated by commit 6cecf02e77ab ("Revert "e1000e: disable s0ix entry
and exit flows for ME systems"").

Seems like ME can do many things to other PCI devices until it's finally out of
ULP polling. So ensure e1000e PM ops are serialized by enforcing suspend/resume
order to workaround the issue.

Of course this will make system suspend and resume a bit slower, but we
probably need to settle on this workaround until ME is fully supported.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=212039
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e63445a8ce12..0244d3dd90a3 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7319,7 +7319,8 @@ static const struct net_device_ops e1000e_netdev_ops = {
 
 static void e1000e_create_device_links(struct pci_dev *pdev)
 {
-	struct pci_dev *tgp_mei_me;
+	struct pci_bus *bus = pdev->bus;
+	struct pci_dev *tgp_mei_me, *p;
 
 	/* Find TGP mei_me devices and make e1000e power depend on mei_me */
 	tgp_mei_me = pci_get_device(PCI_VENDOR_ID_INTEL, 0xa0e0, NULL);
@@ -7335,6 +7336,17 @@ static void e1000e_create_device_links(struct pci_dev *pdev)
 		pci_info(pdev, "System and runtime PM depends on %s\n",
 			 pci_name(tgp_mei_me));
 
+	/* Find other devices in the SoC and make them depend on e1000e */
+	list_for_each_entry(p, &bus->devices, bus_list) {
+		if (&p->dev == &pdev->dev || &p->dev == &tgp_mei_me->dev)
+			continue;
+
+		if (device_link_add(&p->dev, &pdev->dev,
+				    DL_FLAG_AUTOREMOVE_SUPPLIER))
+			pci_info(p, "System PM depends on %s\n",
+				 pci_name(pdev));
+	}
+
 	pci_dev_put(tgp_mei_me);
 }
 
-- 
2.31.1


  parent reply	other threads:[~2021-07-12 13:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-12 13:34 [PATCH 1/3] e1000e: Separate TGP from SPT Kai-Heng Feng
2021-07-12 13:34 ` [Intel-wired-lan] " Kai-Heng Feng
2021-07-12 13:34 ` [PATCH 2/3] e1000e: Make mei_me active when e1000e is in use Kai-Heng Feng
2021-07-12 13:34   ` [Intel-wired-lan] " Kai-Heng Feng
2021-07-14  5:39   ` Sasha Neftin
2021-07-14  5:39     ` Sasha Neftin
2021-07-14  6:28     ` Kai-Heng Feng
2021-07-14  6:28       ` Kai-Heng Feng
2021-07-14  9:05       ` Ruinskiy, Dima
2021-07-14  9:05         ` Ruinskiy, Dima
2021-07-14  9:52         ` Kai-Heng Feng
2021-07-14  9:52           ` Kai-Heng Feng
2021-07-18  8:37           ` Sasha Neftin
2021-07-18  8:37             ` Sasha Neftin
2021-07-12 13:34 ` Kai-Heng Feng [this message]
2021-07-12 13:34   ` [Intel-wired-lan] [PATCH 3/3] e1000e: Serialize TGP e1000e PM ops Kai-Heng Feng
2021-07-27  6:53   ` Kai-Heng Feng
2021-07-27  6:53     ` [Intel-wired-lan] " Kai-Heng Feng
2021-08-01  4:15     ` Sasha Neftin
2021-08-01  4:15       ` [Intel-wired-lan] " Sasha Neftin
2021-07-13 17:58 ` [Intel-wired-lan] [PATCH 1/3] e1000e: Separate TGP from SPT Sasha Neftin
2021-07-13 17:58   ` Sasha Neftin
2021-07-14  4:19   ` Kai-Heng Feng
2021-07-14  4:19     ` Kai-Heng Feng
2021-07-14  8:43     ` Sasha Neftin
2021-07-14  8:43       ` Sasha Neftin

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=20210712133500.1126371-3-kai.heng.feng@canonical.com \
    --to=kai.heng.feng@canonical.com \
    --cc=acelan.kao@canonical.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.