linux-kernel - [PATCH v7] pci: quirk to skip msi disable on shutdown

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 6 Sep 2015 18:32:35 +0300
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	linux-kernel@...r.kernel.org
Cc:	Fam Zheng <famz@...hat.com>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Yinghai Lu <yhlu.kernel.send@...il.com>,
	Ulrich Obergfell <uobergfe@...hat.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	linux-pci@...r.kernel.org,
	virtualization@...ts.linux-foundation.org
Subject: [PATCH v7] pci: quirk to skip msi disable on shutdown

On some hypervisors, virtio devices tend to generate spurious interrupts
when switching between MSI and non-MSI mode.  Normally, either MSI or
non-MSI is used and all is well, but during shutdown, linux disables MSI
which then causes an "irq %d: nobody cared" message, with irq being
subsequently disabled.

Since bus mastering is already disabled at this point, disabling MSI
isn't actually useful for spec compliant devices: MSI interrupts are
in-bus memory writes so disabling Bus Master (which is already done)
disables these as well: after some research, it appears to be there for
the benefit of devices that ignore the bus master bit.

As it's not clear how common this kind of bug is, this patch simply adds
a quirk, to be set by devices that wish to skip disabling msi on
shutdown, relying on bus master instead.

We set this quirk in virtio core.

Reported-by: Fam Zheng <famz@...hat.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>
Cc: Yinghai Lu <yhlu.kernel.send@...il.com>
Cc: Ulrich Obergfell <uobergfe@...hat.com>
Cc: Rusty Russell <rusty@...tcorp.com.au>
Cc: "Eric W. Biederman" <ebiederm@...ssion.com>
Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
---


changes from v6:
	limit changes to virtio only
changes from v5:
        rebased on top of pci/msi
        fixed commit log, including comments by Bjorn
        and adding explanation to address comments/questions by Eric
        dropped stable Cc, this patch does not seem to qualify for stable
changes from v4:
        Yijing Wang <wangyijing@...wei.com> noted that
        early fixups rely on pci_msi_off.
        Split out the functionality and move off the
        required part to run early during pci_device_setup.
Changes from v3:
        fix a copy-and-paste error in
          pci: drop some duplicate code
        other patches are unchanged
        drop Cc stable for now
Changes from v2:
        move code from probe to device enumeration
        add patches to unexport pci_msi_off


 include/linux/pci.h                | 2 ++
 drivers/pci/pci-driver.c           | 6 ++++--
 drivers/virtio/virtio_pci_common.c | 4 ++++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 860c751..80f3494 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -180,6 +180,8 @@ enum pci_dev_flags {
 	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 6),
 	/* Do not use PM reset even if device advertises NoSoftRst- */
 	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7),
+	/* Do not disable MSI on shutdown, disable bus master instead */
+	PCI_DEV_FLAGS_NO_MSI_SHUTDOWN = (__force pci_dev_flags_t) (1 << 8),
 };
 
 enum pci_irq_reroute_variant {
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 3cb2210..59d9e40 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -450,8 +450,10 @@ static void pci_device_shutdown(struct device *dev)
 
 	if (drv && drv->shutdown)
 		drv->shutdown(pci_dev);
-	pci_msi_shutdown(pci_dev);
-	pci_msix_shutdown(pci_dev);
+	if (!(pci_dev->dev_flags & PCI_DEV_FLAGS_NO_MSI_SHUTDOWN)) {
+		pci_msi_shutdown(pci_dev);
+		pci_msix_shutdown(pci_dev);
+	}
 
 #ifdef CONFIG_KEXEC
 	/*
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 78f804a..26f46c3 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -528,6 +528,8 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
 	if (rc)
 		goto err_register;
 
+	pci_dev->dev_flags |= PCI_DEV_FLAGS_NO_MSI_SHUTDOWN;
+
 	return 0;
 
 err_register:
@@ -546,6 +548,8 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
 {
 	struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
 
+	pci_dev->dev_flags &= ~PCI_DEV_FLAGS_NO_MSI_SHUTDOWN;
+
 	unregister_virtio_device(&vp_dev->vdev);
 
 	if (vp_dev->ioaddr)
-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/