LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: bhelgaas@google.com, helgaas@kernel.org, mr.nuke.me@gmail.com,
	linux-pci@vger.kernel.org
Cc: austin_bolen@dell.com, alex_gagniuc@dellteam.com,
	keith.busch@intel.com, Shyam_Iyer@Dell.com, lukas@wunner.de,
	okaya@kernel.org, torvalds@linux-foundation.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] PCI: Add link_change error handler and vfio-pci user
Date: Tue, 23 Apr 2019 16:42:28 -0600
Message-ID: <155605909349.3575.13433421148215616375.stgit@gimli.home> (raw)

The PCIe bandwidth notification service generates logging any time a
link changes speed or width to a state that is considered downgraded.
Unfortunately, it cannot differentiate signal integrity related link
changes from those intentionally initiated by an endpoint driver,
including drivers that may live in userspace or VMs when making use
of vfio-pci.  Therefore, allow the driver to have a say in whether
the link is indeed downgraded and worth noting in the log, or if the
change is perhaps intentional.

For vfio-pci, we don't know the intentions of the user/guest driver
either, but we do know that GPU drivers in guests actively manage
the link state and therefore trigger the bandwidth notification for
what appear to be entirely intentional link changes.

Fixes: e8303bb7a75c PCI/LINK: Report degraded links via link bandwidth notification
Link: https://lore.kernel.org/linux-pci/155597243666.19387.1205950870601742062.stgit@gimli.home/T/#u
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

Changing to pci_dbg() logging is not super usable, so let's try the
previous idea of letting the driver handle link change events as they
see fit.  Ideally this might be two patches, but for easier handling,
folding the pci and vfio-pci bits together.  Comments?  Thanks,

Alex

 drivers/pci/probe.c         |   13 +++++++++++++
 drivers/vfio/pci/vfio_pci.c |   10 ++++++++++
 include/linux/pci.h         |    3 +++
 3 files changed, 26 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 7e12d0163863..233cd4b5b6e8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2403,6 +2403,19 @@ void pcie_report_downtraining(struct pci_dev *dev)
 	if (PCI_FUNC(dev->devfn) != 0 || dev->is_virtfn)
 		return;
 
+	/*
+	 * If driver handles link_change event, defer to driver.  PCIe drivers
+	 * can call pcie_print_link_status() to print current link info.
+	 */
+	device_lock(&dev->dev);
+	if (dev->driver && dev->driver->err_handler &&
+	    dev->driver->err_handler->link_change) {
+		dev->driver->err_handler->link_change(dev);
+		device_unlock(&dev->dev);
+		return;
+	}
+	device_unlock(&dev->dev);
+
 	/* Print link status only if the device is constrained by the fabric */
 	__pcie_print_link_status(dev, false);
 }
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index cab71da46f4a..c9ffc0ccabb3 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1418,8 +1418,18 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
 	return PCI_ERS_RESULT_CAN_RECOVER;
 }
 
+/*
+ * Ignore link change notification, we can't differentiate signal related
+ * link changes from user driver power management type operations, so do
+ * nothing.  Potentially this could be routed out to the user.
+ */
+static void vfio_pci_link_change(struct pci_dev *pdev)
+{
+}
+
 static const struct pci_error_handlers vfio_err_handlers = {
 	.error_detected = vfio_pci_aer_err_detected,
+	.link_change = vfio_pci_link_change,
 };
 
 static struct pci_driver vfio_pci_driver = {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 27854731afc4..e9194bc03f9e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -763,6 +763,9 @@ struct pci_error_handlers {
 
 	/* Device driver may resume normal operations */
 	void (*resume)(struct pci_dev *dev);
+
+	/* PCIe link change notification */
+	void (*link_change)(struct pci_dev *dev);
 };
 
 


             reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-23 22:42 Alex Williamson [this message]
2019-04-24 16:45 ` Alex_Gagniuc
2019-04-24 17:19   ` Alex Williamson
2019-04-24 17:35     ` Alex G
2019-04-24 17:57 ` Bjorn Helgaas
2019-04-29 14:51   ` Alex Williamson
2019-04-29 16:45     ` Sinan Kaya
2019-04-29 16:59       ` Alex Williamson
2019-04-30 17:59         ` Keith Busch
2019-04-29 17:43     ` Bjorn Helgaas

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=155605909349.3575.13433421148215616375.stgit@gimli.home \
    --to=alex.williamson@redhat.com \
    --cc=Shyam_Iyer@Dell.com \
    --cc=alex_gagniuc@dellteam.com \
    --cc=austin_bolen@dell.com \
    --cc=bhelgaas@google.com \
    --cc=helgaas@kernel.org \
    --cc=keith.busch@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mr.nuke.me@gmail.com \
    --cc=okaya@kernel.org \
    --cc=torvalds@linux-foundation.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

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

Example config snippet for mirrors

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


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