All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown
@ 2015-03-24 15:42 Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 01/10] pci: export functions for msi/msix ctrl Michael S. Tsirkin
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu

Fam Zheng noticed that pci shutdown disables msi and msix of a device while
device is still active. This was intended to fix kexec with fusion devices but
had the unintended effect of breaking even regular shutdown when using virtio.

As a result, people reported VMs being hung on shutdown,
with traces that look like this:

...

  [<ffffffff815f367b>] ? _raw_spin_unlock_irqrestore+0x1b/0x40
  [<ffffffff812e068f>] pci_bus_read_config_word+0x9f/0xb0
  [<ffffffff812e205c>] pci_read_config_word+0x1c/0x20
  [<ffffffff812e5ea3>] pci_intx+0x33/0xb0
  [<ffffffff81304ddd>] pci_msix_shutdown+0x8d/0x90
  [<ffffffff812ead26>] pci_device_shutdown+0x46/0x60
  [<ffffffff813b4258>] device_shutdown+0xc8/0x180
  [<ffffffff810761c5>] kernel_power_off+0x35/0x80
  [<ffffffff81077ceb>] SYSC_reboot+0x18b/0x260
  [<ffffffff811c5bb5>] ? d_free+0x55/0x60
  [<ffffffff811cf414>] ? mntput+0x24/0x40
  [<ffffffff811b1613>] ? __fput+0x183/0x270
  [<ffffffff81077dee>] SyS_reboot+0xe/0x10
  [<ffffffff815fc819>] system_call_fastpath+0x16/0x1b

...

as well as the messages that look like this:
BUG: soft lockup - CPU#2 stuck for 22s!  [poweroff:1964]

The same problem would affect any driver which doesn't register
a level interrupt handler when using msix.

I think the fix is to avoid touching device on shutdown:
we clear bus master anyway, so we won't get any more
msi interrupts, and bus reset will clear the msi/msix
state eventually.

The patch that added the code in question is from
	d52877c7b1afb8c37ebe17e2005040b79cb618b0:
        "pci/irq: let pci_device_shutdown to call pci_msi_shutdown v2"
from 2008, but it turns out that in 2011 the following commit
    commit d5dea7d95c48d7bc951cee4910a7fd9c0cd26fb0
        "PCI: msi: Disable msi interrupts when we initialize a pci device"
fixed the issue in a more robust way.  The only thing left to do is to
fix configurations with CONFIG_PCI_MSI disabled, and then
d52877c7b1afb8c37ebe17e2005040b79cb618b0 can be reverted.

Status:

Patches 1-6 work well for me.
Given they affect all pci devices, and the bug has been there since 2.6 times,
I think there's no rush: we can merge them for 4.1.

At the same time, once merged, patches 1-4 will likely make a good stable
candidate.

Patches 7-10 compiled only, will need maintainer ack.

Please review, and consider at least 1-6 for 4.1.

Changes from v3:
	move code from probe to device enumeration
	add patches to unexport pci_msi_off

Michael S. Tsirkin (10):
  pci: export functions for msi/msix ctrl
  pci: move pci_msi_init_pci_dev to pci.c
  pci: drop some duplicate code
  pci: don't disable msi/msix at shutdown
  pci: make msi/msix shutdown functions static
  virtio_pci: drop msi_off on probe
  ntb: drop pci_msi_off call on probe
  mic: drop pci_msi_off call on probe
  pci: drop pci_msi_off calls from quirks
  pci: unexport pci_msi_off

 drivers/pci/pci.h                  | 25 +++++++++++++--
 include/linux/pci.h                |  5 ---
 drivers/misc/mic/host/mic_intr.c   |  2 --
 drivers/ntb/ntb_hw.c               |  2 --
 drivers/pci/msi.c                  | 62 ++++++++------------------------------
 drivers/pci/pci-driver.c           |  2 --
 drivers/pci/pci.c                  | 26 ++++------------
 drivers/pci/probe.c                | 16 ++++++++++
 drivers/pci/quirks.c               |  2 --
 drivers/virtio/virtio_pci_common.c |  3 --
 10 files changed, 57 insertions(+), 88 deletions(-)

-- 
MST


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 01/10] pci: export functions for msi/msix ctrl
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
@ 2015-03-24 15:42 ` Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 02/10] pci: move pci_msi_init_pci_dev to pci.c Michael S. Tsirkin
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu

move pci_msi_set_enable and pci_msix_clear_and_set_ctrl out of msi.c, so
we can use them will be used which MSI isn't configured in kernel.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/pci/pci.h | 21 +++++++++++++++++++++
 drivers/pci/msi.c | 45 ++++++++++++---------------------------------
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4091f82..17f213d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -146,6 +146,27 @@ static inline void pci_no_msi(void) { }
 static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
 #endif
 
+static inline void pci_msi_set_enable(struct pci_dev *dev, int enable)
+{
+	u16 control;
+
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
+	control &= ~PCI_MSI_FLAGS_ENABLE;
+	if (enable)
+		control |= PCI_MSI_FLAGS_ENABLE;
+	pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
+}
+
+static inline void pci_msix_clear_and_set_ctrl(struct pci_dev *dev, u16 clear, u16 set)
+{
+	u16 ctrl;
+
+	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &ctrl);
+	ctrl &= ~clear;
+	ctrl |= set;
+	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, ctrl);
+}
+
 void pci_realloc_get_opt(char *);
 
 static inline int pci_no_d1d2(struct pci_dev *dev)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index c3e7dfc..9942f68 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -185,27 +185,6 @@ void __weak arch_restore_msi_irqs(struct pci_dev *dev)
 	return default_restore_msi_irqs(dev);
 }
 
-static void msi_set_enable(struct pci_dev *dev, int enable)
-{
-	u16 control;
-
-	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
-	control &= ~PCI_MSI_FLAGS_ENABLE;
-	if (enable)
-		control |= PCI_MSI_FLAGS_ENABLE;
-	pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
-}
-
-static void msix_clear_and_set_ctrl(struct pci_dev *dev, u16 clear, u16 set)
-{
-	u16 ctrl;
-
-	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &ctrl);
-	ctrl &= ~clear;
-	ctrl |= set;
-	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, ctrl);
-}
-
 static inline __attribute_const__ u32 msi_mask(unsigned x)
 {
 	/* Don't shift by >= width of type */
@@ -452,7 +431,7 @@ static void __pci_restore_msi_state(struct pci_dev *dev)
 	entry = irq_get_msi_desc(dev->irq);
 
 	pci_intx_for_msi(dev, 0);
-	msi_set_enable(dev, 0);
+	pci_msi_set_enable(dev, 0);
 	arch_restore_msi_irqs(dev);
 
 	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
@@ -473,14 +452,14 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
 
 	/* route the table */
 	pci_intx_for_msi(dev, 0);
-	msix_clear_and_set_ctrl(dev, 0,
+	pci_msix_clear_and_set_ctrl(dev, 0,
 				PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL);
 
 	arch_restore_msi_irqs(dev);
 	list_for_each_entry(entry, &dev->msi_list, list)
 		msix_mask_irq(entry, entry->masked);
 
-	msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
+	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
 }
 
 void pci_restore_msi_state(struct pci_dev *dev)
@@ -647,7 +626,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 	int ret;
 	unsigned mask;
 
-	msi_set_enable(dev, 0);	/* Disable MSI during set up */
+	pci_msi_set_enable(dev, 0);	/* Disable MSI during set up */
 
 	entry = msi_setup_entry(dev, nvec);
 	if (!entry)
@@ -683,7 +662,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 
 	/* Set MSI enabled bits	 */
 	pci_intx_for_msi(dev, 0);
-	msi_set_enable(dev, 1);
+	pci_msi_set_enable(dev, 1);
 	dev->msi_enabled = 1;
 
 	dev->irq = entry->irq;
@@ -775,7 +754,7 @@ static int msix_capability_init(struct pci_dev *dev,
 	void __iomem *base;
 
 	/* Ensure MSI-X is disabled while it is set up */
-	msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
+	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
 
 	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
 	/* Request & Map MSI-X table region */
@@ -801,7 +780,7 @@ static int msix_capability_init(struct pci_dev *dev,
 	 * MSI-X registers.  We need to mask all the vectors to prevent
 	 * interrupts coming in before they're fully set up.
 	 */
-	msix_clear_and_set_ctrl(dev, 0,
+	pci_msix_clear_and_set_ctrl(dev, 0,
 				PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE);
 
 	msix_program_entries(dev, entries);
@@ -814,7 +793,7 @@ static int msix_capability_init(struct pci_dev *dev,
 	pci_intx_for_msi(dev, 0);
 	dev->msix_enabled = 1;
 
-	msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
+	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
 
 	return 0;
 
@@ -919,7 +898,7 @@ void pci_msi_shutdown(struct pci_dev *dev)
 	BUG_ON(list_empty(&dev->msi_list));
 	desc = list_first_entry(&dev->msi_list, struct msi_desc, list);
 
-	msi_set_enable(dev, 0);
+	pci_msi_set_enable(dev, 0);
 	pci_intx_for_msi(dev, 1);
 	dev->msi_enabled = 0;
 
@@ -1027,7 +1006,7 @@ void pci_msix_shutdown(struct pci_dev *dev)
 		__pci_msix_desc_mask_irq(entry, 1);
 	}
 
-	msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
+	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
 	pci_intx_for_msi(dev, 1);
 	dev->msix_enabled = 0;
 }
@@ -1069,11 +1048,11 @@ void pci_msi_init_pci_dev(struct pci_dev *dev)
 	 */
 	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
 	if (dev->msi_cap)
-		msi_set_enable(dev, 0);
+		pci_msi_set_enable(dev, 0);
 
 	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
 	if (dev->msix_cap)
-		msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
+		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
 }
 
 /**
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 02/10] pci: move pci_msi_init_pci_dev to pci.c
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 01/10] pci: export functions for msi/msix ctrl Michael S. Tsirkin
@ 2015-03-24 15:42 ` Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 03/10] pci: drop some duplicate code Michael S. Tsirkin
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu, stable,
	Eric W. Biederman

commit d5dea7d95c48d7bc951cee4910a7fd9c0cd26fb0
    "PCI: msi: Disable msi interrupts when we initialize a pci device"
fixes kexec when the booting kernel does not enable msi interupts.

Unfortunately the relevant functionality is in msi.c so it isn't
compiled in when CONFIG_PCI_MSI is off, which means such configurations
would still get interrupt storms.

Fix by moving the function to pci.c, and compiling it unconditionally.

Cc: stable@kernel.org
Cc: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/pci/pci.h   |  2 --
 drivers/pci/msi.c   | 17 -----------------
 drivers/pci/probe.c | 19 +++++++++++++++++++
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 17f213d..a964569 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -140,10 +140,8 @@ extern unsigned int pci_pm_d3_delay;
 
 #ifdef CONFIG_PCI_MSI
 void pci_no_msi(void);
-void pci_msi_init_pci_dev(struct pci_dev *dev);
 #else
 static inline void pci_no_msi(void) { }
-static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
 #endif
 
 static inline void pci_msi_set_enable(struct pci_dev *dev, int enable)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 9942f68..ae3f7cc 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1038,23 +1038,6 @@ int pci_msi_enabled(void)
 }
 EXPORT_SYMBOL(pci_msi_enabled);
 
-void pci_msi_init_pci_dev(struct pci_dev *dev)
-{
-	INIT_LIST_HEAD(&dev->msi_list);
-
-	/* Disable the msi hardware to avoid screaming interrupts
-	 * during boot.  This is the power on reset default so
-	 * usually this should be a noop.
-	 */
-	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
-	if (dev->msi_cap)
-		pci_msi_set_enable(dev, 0);
-
-	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
-	if (dev->msix_cap)
-		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
-}
-
 /**
  * pci_enable_msi_range - configure device's MSI capability structure
  * @dev: device to configure
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8d2f400..45d6d5c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1483,6 +1483,25 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	return dev;
 }
 
+static void pci_msi_init_pci_dev(struct pci_dev *dev)
+{
+#ifdef CONFIG_PCI_MSI
+	INIT_LIST_HEAD(&dev->msi_list);
+#endif
+
+	/* Disable the msi hardware to avoid screaming interrupts
+	 * during boot.  This is the power on reset default so
+	 * usually this should be a noop.
+	 */
+	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
+	if (dev->msi_cap)
+		pci_msi_set_enable(dev, 0);
+
+	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+	if (dev->msix_cap)
+		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
+}
+
 static void pci_init_capabilities(struct pci_dev *dev)
 {
 	/* MSI/MSI-X list */
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 03/10] pci: drop some duplicate code
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 01/10] pci: export functions for msi/msix ctrl Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 02/10] pci: move pci_msi_init_pci_dev to pci.c Michael S. Tsirkin
@ 2015-03-24 15:42 ` Michael S. Tsirkin
  2015-03-24 23:33   ` Bjorn Helgaas
  2015-03-24 15:42 ` [PATCH v3 04/10] pci: don't disable msi/msix at shutdown Michael S. Tsirkin
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu

pci_msi_init_pci_dev and pci_msi_off share a lot of code.
This used to be justified since pci_msi_init_pci_dev
wasn't compiled in when CONFIG_PCI_MSI is off.
Now that it is, let's reuse code.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/pci/pci.c   | 24 +++++-------------------
 drivers/pci/probe.c | 11 ++++-------
 2 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 81f06e8..d5f297a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3106,26 +3106,12 @@ EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
  */
 void pci_msi_off(struct pci_dev *dev)
 {
-	int pos;
-	u16 control;
+	if (dev->msi_cap)
+		pci_msi_set_enable(dev, 0);
 
-	/*
-	 * This looks like it could go in msi.c, but we need it even when
-	 * CONFIG_PCI_MSI=n.  For the same reason, we can't use
-	 * dev->msi_cap or dev->msix_cap here.
-	 */
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
-	if (pos) {
-		pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
-		control &= ~PCI_MSI_FLAGS_ENABLE;
-		pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
-	}
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
-	if (pos) {
-		pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
-		control &= ~PCI_MSIX_FLAGS_ENABLE;
-		pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
-	}
+	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+	if (dev->msix_cap)
+		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
 }
 EXPORT_SYMBOL_GPL(pci_msi_off);
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 45d6d5c..baf8ddd 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1489,17 +1489,14 @@ static void pci_msi_init_pci_dev(struct pci_dev *dev)
 	INIT_LIST_HEAD(&dev->msi_list);
 #endif
 
+	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
+	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+
 	/* Disable the msi hardware to avoid screaming interrupts
 	 * during boot.  This is the power on reset default so
 	 * usually this should be a noop.
 	 */
-	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
-	if (dev->msi_cap)
-		pci_msi_set_enable(dev, 0);
-
-	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
-	if (dev->msix_cap)
-		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
+	pci_msi_off(dev);
 }
 
 static void pci_init_capabilities(struct pci_dev *dev)
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 04/10] pci: don't disable msi/msix at shutdown
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
                   ` (2 preceding siblings ...)
  2015-03-24 15:42 ` [PATCH v3 03/10] pci: drop some duplicate code Michael S. Tsirkin
@ 2015-03-24 15:42 ` Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 05/10] pci: make msi/msix shutdown functions static Michael S. Tsirkin
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu,
	Ulrich Obergfell, Rusty Russell

This partially reverts commit d52877c7b1afb8c37ebe17e2005040b79cb618b0:
	"pci/irq: let pci_device_shutdown to call pci_msi_shutdown v2"

It's un-necessary now that we disable msi at start, and it actually
turns out to cause problems: some device drivers don't register a level
interrupt handler when they detect msi/msix capability, switching off
msi while device is going causes device to assert a level interrupt
which is never de-asserted, causing a kernel hang.

In particular, this was observed with virtio.

Cc: Yinghai Lu <yhlu.kernel.send@gmail.com>
Cc: Ulrich Obergfell <uobergfe@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Reported-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/pci/pci-driver.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 3cb2210..38a602c 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -450,8 +450,6 @@ 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);
 
 #ifdef CONFIG_KEXEC
 	/*
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 05/10] pci: make msi/msix shutdown functions static
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
                   ` (3 preceding siblings ...)
  2015-03-24 15:42 ` [PATCH v3 04/10] pci: don't disable msi/msix at shutdown Michael S. Tsirkin
@ 2015-03-24 15:42 ` Michael S. Tsirkin
  2015-03-24 15:42   ` Michael S. Tsirkin
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu

pci_msi_shutdown and pci_msix_shutdown are now internal to msi.c, drop
them from header and make them static.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/linux/pci.h | 4 ----
 drivers/pci/msi.c   | 4 ++--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 211e9da..a34df45 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1209,11 +1209,9 @@ struct msix_entry {
 
 #ifdef CONFIG_PCI_MSI
 int pci_msi_vec_count(struct pci_dev *dev);
-void pci_msi_shutdown(struct pci_dev *dev);
 void pci_disable_msi(struct pci_dev *dev);
 int pci_msix_vec_count(struct pci_dev *dev);
 int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec);
-void pci_msix_shutdown(struct pci_dev *dev);
 void pci_disable_msix(struct pci_dev *dev);
 void pci_restore_msi_state(struct pci_dev *dev);
 int pci_msi_enabled(void);
@@ -1237,13 +1235,11 @@ static inline int pci_enable_msix_exact(struct pci_dev *dev,
 }
 #else
 static inline int pci_msi_vec_count(struct pci_dev *dev) { return -ENOSYS; }
-static inline void pci_msi_shutdown(struct pci_dev *dev) { }
 static inline void pci_disable_msi(struct pci_dev *dev) { }
 static inline int pci_msix_vec_count(struct pci_dev *dev) { return -ENOSYS; }
 static inline int pci_enable_msix(struct pci_dev *dev,
 				  struct msix_entry *entries, int nvec)
 { return -ENOSYS; }
-static inline void pci_msix_shutdown(struct pci_dev *dev) { }
 static inline void pci_disable_msix(struct pci_dev *dev) { }
 static inline void pci_restore_msi_state(struct pci_dev *dev) { }
 static inline int pci_msi_enabled(void) { return 0; }
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index ae3f7cc..a353222 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -887,7 +887,7 @@ int pci_msi_vec_count(struct pci_dev *dev)
 }
 EXPORT_SYMBOL(pci_msi_vec_count);
 
-void pci_msi_shutdown(struct pci_dev *dev)
+static void pci_msi_shutdown(struct pci_dev *dev)
 {
 	struct msi_desc *desc;
 	u32 mask;
@@ -993,7 +993,7 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
 }
 EXPORT_SYMBOL(pci_enable_msix);
 
-void pci_msix_shutdown(struct pci_dev *dev)
+static void pci_msix_shutdown(struct pci_dev *dev)
 {
 	struct msi_desc *entry;
 
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 06/10] virtio_pci: drop msi_off on probe
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
@ 2015-03-24 15:42   ` Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 02/10] pci: move pci_msi_init_pci_dev to pci.c Michael S. Tsirkin
                     ` (9 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu,
	Rusty Russell, virtualization

pci core now disables msi on probe automatically,
drop this from device-specific code.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci_common.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index e894eb2..806bb2c 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -501,9 +501,6 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
 	INIT_LIST_HEAD(&vp_dev->virtqueues);
 	spin_lock_init(&vp_dev->lock);
 
-	/* Disable MSI/MSIX to bring device to a known good state. */
-	pci_msi_off(pci_dev);
-
 	/* enable the device */
 	rc = pci_enable_device(pci_dev);
 	if (rc)
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 06/10] virtio_pci: drop msi_off on probe
@ 2015-03-24 15:42   ` Michael S. Tsirkin
  0 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fam Zheng, linux-pci, stable, virtualization, Yinghai Lu, Bjorn Helgaas

pci core now disables msi on probe automatically,
drop this from device-specific code.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci_common.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index e894eb2..806bb2c 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -501,9 +501,6 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
 	INIT_LIST_HEAD(&vp_dev->virtqueues);
 	spin_lock_init(&vp_dev->lock);
 
-	/* Disable MSI/MSIX to bring device to a known good state. */
-	pci_msi_off(pci_dev);
-
 	/* enable the device */
 	rc = pci_enable_device(pci_dev);
 	if (rc)
-- 
MST

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 07/10] ntb: drop pci_msi_off call on probe
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
                   ` (5 preceding siblings ...)
  2015-03-24 15:42   ` Michael S. Tsirkin
@ 2015-03-24 15:42 ` Michael S. Tsirkin
  2015-03-24 16:12     ` Jiang, Dave
  2015-03-24 15:42 ` [PATCH v3 08/10] mic: " Michael S. Tsirkin
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu,
	Jon Mason, Dave Jiang

pci core now disables msi on probe automatically,
drop this from device-specific code.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/ntb/ntb_hw.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index cd29b10..8225cbc 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1313,8 +1313,6 @@ static int ntb_setup_intx(struct ntb_device *ndev)
 	struct pci_dev *pdev = ndev->pdev;
 	int rc;
 
-	pci_msi_off(pdev);
-
 	/* Verify intx is enabled */
 	pci_intx(pdev, 1);
 
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 08/10] mic: drop pci_msi_off call on probe
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
                   ` (6 preceding siblings ...)
  2015-03-24 15:42 ` [PATCH v3 07/10] ntb: drop pci_msi_off call " Michael S. Tsirkin
@ 2015-03-24 15:42 ` Michael S. Tsirkin
  2015-03-24 15:42 ` [PATCH v3 09/10] pci: drop pci_msi_off calls from quirks Michael S. Tsirkin
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu,
	Ashutosh Dixit, Nikhil Rao, Greg Kroah-Hartman, Sudeep Dutt,
	Siva Yerramreddy

pci core now disables msi on probe automatically,
drop this from device-specific code.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/misc/mic/host/mic_intr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/misc/mic/host/mic_intr.c b/drivers/misc/mic/host/mic_intr.c
index d686f28..b4ca6c8 100644
--- a/drivers/misc/mic/host/mic_intr.c
+++ b/drivers/misc/mic/host/mic_intr.c
@@ -363,8 +363,6 @@ static int mic_setup_intx(struct mic_device *mdev, struct pci_dev *pdev)
 {
 	int rc;
 
-	pci_msi_off(pdev);
-
 	/* Enable intx */
 	pci_intx(pdev, 1);
 	rc = mic_setup_callbacks(mdev);
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 09/10] pci: drop pci_msi_off calls from quirks
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
                   ` (7 preceding siblings ...)
  2015-03-24 15:42 ` [PATCH v3 08/10] mic: " Michael S. Tsirkin
@ 2015-03-24 15:42 ` Michael S. Tsirkin
  2015-03-24 15:43 ` [PATCH v3 10/10] pci: unexport pci_msi_off Michael S. Tsirkin
  2015-03-24 17:02 ` [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Greg KH
  10 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu

pci core now disables msi on probe automatically,
drop this from device-specific quirks.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/pci/quirks.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 85f247e..df3e718 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1600,7 +1600,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_EESSC,	quirk_a
 
 static void quirk_pcie_mch(struct pci_dev *pdev)
 {
-	pci_msi_off(pdev);
 	pdev->no_msi = 1;
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
@@ -1614,7 +1613,6 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quir
  */
 static void quirk_pcie_pxh(struct pci_dev *dev)
 {
-	pci_msi_off(dev);
 	dev->no_msi = 1;
 	dev_warn(&dev->dev, "PXH quirk detected; SHPC device MSI disabled\n");
 }
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 10/10] pci: unexport pci_msi_off
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
                   ` (8 preceding siblings ...)
  2015-03-24 15:42 ` [PATCH v3 09/10] pci: drop pci_msi_off calls from quirks Michael S. Tsirkin
@ 2015-03-24 15:43 ` Michael S. Tsirkin
  2015-03-24 17:02 ` [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Greg KH
  10 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-24 15:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu

Now one should be using pci_msi_off now, unexport it and move the
declaration to internal header to prevent misuse.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/pci/pci.h   | 2 ++
 include/linux/pci.h | 1 -
 drivers/pci/pci.c   | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a964569..dd17615 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -144,6 +144,8 @@ void pci_no_msi(void);
 static inline void pci_no_msi(void) { }
 #endif
 
+void pci_msi_off(struct pci_dev *dev);
+
 static inline void pci_msi_set_enable(struct pci_dev *dev, int enable)
 {
 	u16 control;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index a34df45..ef15f91 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -970,7 +970,6 @@ void pci_intx(struct pci_dev *dev, int enable);
 bool pci_intx_mask_supported(struct pci_dev *dev);
 bool pci_check_and_mask_intx(struct pci_dev *dev);
 bool pci_check_and_unmask_intx(struct pci_dev *dev);
-void pci_msi_off(struct pci_dev *dev);
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size);
 int pci_set_dma_seg_boundary(struct pci_dev *dev, unsigned long mask);
 int pci_wait_for_pending(struct pci_dev *dev, int pos, u16 mask);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d5f297a..04063cc 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3103,6 +3103,7 @@ EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
  * If you want to use MSI, see pci_enable_msi() and friends.
  * This is a lower-level primitive that allows us to disable
  * MSI operation at the device level.
+ * Not for use by drivers.
  */
 void pci_msi_off(struct pci_dev *dev)
 {
@@ -3113,7 +3114,6 @@ void pci_msi_off(struct pci_dev *dev)
 	if (dev->msix_cap)
 		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
 }
-EXPORT_SYMBOL_GPL(pci_msi_off);
 
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size)
 {
-- 
MST


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 07/10] ntb: drop pci_msi_off call on probe
  2015-03-24 15:42 ` [PATCH v3 07/10] ntb: drop pci_msi_off call " Michael S. Tsirkin
  2015-03-24 16:12     ` Jiang, Dave
@ 2015-03-24 16:12     ` Jiang, Dave
  0 siblings, 0 replies; 18+ messages in thread
From: Jiang, Dave @ 2015-03-24 16:12 UTC (permalink / raw)
  To: mst
  Cc: linux-kernel, linux-pci, yhlu.kernel.send, stable, bhelgaas,
	jdmason, famz

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 928 bytes --]

On Tue, 2015-03-24 at 16:42 +0100, Michael S. Tsirkin wrote:
> pci core now disables msi on probe automatically,
> drop this from device-specific code.
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/ntb_hw.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index cd29b10..8225cbc 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1313,8 +1313,6 @@ static int ntb_setup_intx(struct ntb_device *ndev)
>  	struct pci_dev *pdev = ndev->pdev;
>  	int rc;
>  
> -	pci_msi_off(pdev);
> -
>  	/* Verify intx is enabled */
>  	pci_intx(pdev, 1);
>  
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 07/10] ntb: drop pci_msi_off call on probe
@ 2015-03-24 16:12     ` Jiang, Dave
  0 siblings, 0 replies; 18+ messages in thread
From: Jiang, Dave @ 2015-03-24 16:12 UTC (permalink / raw)
  To: mst
  Cc: linux-kernel, linux-pci, yhlu.kernel.send, stable, bhelgaas,
	jdmason, famz

On Tue, 2015-03-24 at 16:42 +0100, Michael S. Tsirkin wrote:
> pci core now disables msi on probe automatically,
> drop this from device-specific code.
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/ntb_hw.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index cd29b10..8225cbc 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1313,8 +1313,6 @@ static int ntb_setup_intx(struct ntb_device *ndev)
>  	struct pci_dev *pdev = ndev->pdev;
>  	int rc;
>  
> -	pci_msi_off(pdev);
> -
>  	/* Verify intx is enabled */
>  	pci_intx(pdev, 1);
>  

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 07/10] ntb: drop pci_msi_off call on probe
@ 2015-03-24 16:12     ` Jiang, Dave
  0 siblings, 0 replies; 18+ messages in thread
From: Jiang, Dave @ 2015-03-24 16:12 UTC (permalink / raw)
  To: mst
  Cc: linux-kernel, linux-pci, yhlu.kernel.send, stable, bhelgaas,
	jdmason, famz

T24gVHVlLCAyMDE1LTAzLTI0IGF0IDE2OjQyICswMTAwLCBNaWNoYWVsIFMuIFRzaXJraW4gd3Jv
dGU6DQo+IHBjaSBjb3JlIG5vdyBkaXNhYmxlcyBtc2kgb24gcHJvYmUgYXV0b21hdGljYWxseSwN
Cj4gZHJvcCB0aGlzIGZyb20gZGV2aWNlLXNwZWNpZmljIGNvZGUuDQo+IA0KPiBDYzogQmpvcm4g
SGVsZ2FhcyA8YmhlbGdhYXNAZ29vZ2xlLmNvbT4NCj4gQ2M6IGxpbnV4LXBjaUB2Z2VyLmtlcm5l
bC5vcmcNCj4gU2lnbmVkLW9mZi1ieTogTWljaGFlbCBTLiBUc2lya2luIDxtc3RAcmVkaGF0LmNv
bT4NCg0KQWNrZWQtYnk6IERhdmUgSmlhbmcgPGRhdmUuamlhbmdAaW50ZWwuY29tPg0KDQo+IC0t
LQ0KPiAgZHJpdmVycy9udGIvbnRiX2h3LmMgfCAyIC0tDQo+ICAxIGZpbGUgY2hhbmdlZCwgMiBk
ZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL250Yi9udGJfaHcuYyBiL2Ry
aXZlcnMvbnRiL250Yl9ody5jDQo+IGluZGV4IGNkMjliMTAuLjgyMjVjYmMgMTAwNjQ0DQo+IC0t
LSBhL2RyaXZlcnMvbnRiL250Yl9ody5jDQo+ICsrKyBiL2RyaXZlcnMvbnRiL250Yl9ody5jDQo+
IEBAIC0xMzEzLDggKzEzMTMsNiBAQCBzdGF0aWMgaW50IG50Yl9zZXR1cF9pbnR4KHN0cnVjdCBu
dGJfZGV2aWNlICpuZGV2KQ0KPiAgCXN0cnVjdCBwY2lfZGV2ICpwZGV2ID0gbmRldi0+cGRldjsN
Cj4gIAlpbnQgcmM7DQo+ICANCj4gLQlwY2lfbXNpX29mZihwZGV2KTsNCj4gLQ0KPiAgCS8qIFZl
cmlmeSBpbnR4IGlzIGVuYWJsZWQgKi8NCj4gIAlwY2lfaW50eChwZGV2LCAxKTsNCj4gIA0K

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown
  2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
                   ` (9 preceding siblings ...)
  2015-03-24 15:43 ` [PATCH v3 10/10] pci: unexport pci_msi_off Michael S. Tsirkin
@ 2015-03-24 17:02 ` Greg KH
  10 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2015-03-24 17:02 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, stable, Bjorn Helgaas, linux-pci, Fam Zheng, Yinghai Lu

On Tue, Mar 24, 2015 at 04:42:25PM +0100, Michael S. Tsirkin wrote:
> Fam Zheng noticed that pci shutdown disables msi and msix of a device while
> device is still active. This was intended to fix kexec with fusion devices but
> had the unintended effect of breaking even regular shutdown when using virtio.
> 
> As a result, people reported VMs being hung on shutdown,
> with traces that look like this:
> 
> ...
> 
>   [<ffffffff815f367b>] ? _raw_spin_unlock_irqrestore+0x1b/0x40
>   [<ffffffff812e068f>] pci_bus_read_config_word+0x9f/0xb0
>   [<ffffffff812e205c>] pci_read_config_word+0x1c/0x20
>   [<ffffffff812e5ea3>] pci_intx+0x33/0xb0
>   [<ffffffff81304ddd>] pci_msix_shutdown+0x8d/0x90
>   [<ffffffff812ead26>] pci_device_shutdown+0x46/0x60
>   [<ffffffff813b4258>] device_shutdown+0xc8/0x180
>   [<ffffffff810761c5>] kernel_power_off+0x35/0x80
>   [<ffffffff81077ceb>] SYSC_reboot+0x18b/0x260
>   [<ffffffff811c5bb5>] ? d_free+0x55/0x60
>   [<ffffffff811cf414>] ? mntput+0x24/0x40
>   [<ffffffff811b1613>] ? __fput+0x183/0x270
>   [<ffffffff81077dee>] SyS_reboot+0xe/0x10
>   [<ffffffff815fc819>] system_call_fastpath+0x16/0x1b
> 
> ...
> 
> as well as the messages that look like this:
> BUG: soft lockup - CPU#2 stuck for 22s!  [poweroff:1964]
> 
> The same problem would affect any driver which doesn't register
> a level interrupt handler when using msix.
> 
> I think the fix is to avoid touching device on shutdown:
> we clear bus master anyway, so we won't get any more
> msi interrupts, and bus reset will clear the msi/msix
> state eventually.
> 
> The patch that added the code in question is from
> 	d52877c7b1afb8c37ebe17e2005040b79cb618b0:
>         "pci/irq: let pci_device_shutdown to call pci_msi_shutdown v2"
> from 2008, but it turns out that in 2011 the following commit
>     commit d5dea7d95c48d7bc951cee4910a7fd9c0cd26fb0
>         "PCI: msi: Disable msi interrupts when we initialize a pci device"
> fixed the issue in a more robust way.  The only thing left to do is to
> fix configurations with CONFIG_PCI_MSI disabled, and then
> d52877c7b1afb8c37ebe17e2005040b79cb618b0 can be reverted.
> 
> Status:
> 
> Patches 1-6 work well for me.
> Given they affect all pci devices, and the bug has been there since 2.6 times,
> I think there's no rush: we can merge them for 4.1.
> 
> At the same time, once merged, patches 1-4 will likely make a good stable
> candidate.
> 
> Patches 7-10 compiled only, will need maintainer ack.
> 
> Please review, and consider at least 1-6 for 4.1.
> 
> Changes from v3:
> 	move code from probe to device enumeration
> 	add patches to unexport pci_msi_off
> 
> Michael S. Tsirkin (10):
>   pci: export functions for msi/msix ctrl
>   pci: move pci_msi_init_pci_dev to pci.c
>   pci: drop some duplicate code
>   pci: don't disable msi/msix at shutdown
>   pci: make msi/msix shutdown functions static
>   virtio_pci: drop msi_off on probe
>   ntb: drop pci_msi_off call on probe
>   mic: drop pci_msi_off call on probe
>   pci: drop pci_msi_off calls from quirks
>   pci: unexport pci_msi_off
> 
>  drivers/pci/pci.h                  | 25 +++++++++++++--
>  include/linux/pci.h                |  5 ---
>  drivers/misc/mic/host/mic_intr.c   |  2 --
>  drivers/ntb/ntb_hw.c               |  2 --
>  drivers/pci/msi.c                  | 62 ++++++++------------------------------
>  drivers/pci/pci-driver.c           |  2 --
>  drivers/pci/pci.c                  | 26 ++++------------
>  drivers/pci/probe.c                | 16 ++++++++++
>  drivers/pci/quirks.c               |  2 --
>  drivers/virtio/virtio_pci_common.c |  3 --
>  10 files changed, 57 insertions(+), 88 deletions(-)


<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 03/10] pci: drop some duplicate code
  2015-03-24 15:42 ` [PATCH v3 03/10] pci: drop some duplicate code Michael S. Tsirkin
@ 2015-03-24 23:33   ` Bjorn Helgaas
  2015-03-25  5:40     ` Michael S. Tsirkin
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2015-03-24 23:33 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: linux-kernel, stable, linux-pci, Fam Zheng, Yinghai Lu

On Tue, Mar 24, 2015 at 04:42:35PM +0100, Michael S. Tsirkin wrote:
> pci_msi_init_pci_dev and pci_msi_off share a lot of code.
> This used to be justified since pci_msi_init_pci_dev
> wasn't compiled in when CONFIG_PCI_MSI is off.
> Now that it is, let's reuse code.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/pci/pci.c   | 24 +++++-------------------
>  drivers/pci/probe.c | 11 ++++-------
>  2 files changed, 9 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 81f06e8..d5f297a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3106,26 +3106,12 @@ EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
>   */
>  void pci_msi_off(struct pci_dev *dev)
>  {
> -	int pos;
> -	u16 control;
> +	if (dev->msi_cap)
> +		pci_msi_set_enable(dev, 0);
>  
> -	/*
> -	 * This looks like it could go in msi.c, but we need it even when
> -	 * CONFIG_PCI_MSI=n.  For the same reason, we can't use
> -	 * dev->msi_cap or dev->msix_cap here.
> -	 */
> -	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
> -	if (pos) {
> -		pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
> -		control &= ~PCI_MSI_FLAGS_ENABLE;
> -		pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
> -	}
> -	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> -	if (pos) {
> -		pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
> -		control &= ~PCI_MSIX_FLAGS_ENABLE;
> -		pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
> -	}
> +	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);

I think the above line is superfluous, isn't it?  You already do the same
in pci_msi_init_pci_dev() (below).

> +	if (dev->msix_cap)
> +		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
>  }
>  EXPORT_SYMBOL_GPL(pci_msi_off);
>  
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 45d6d5c..baf8ddd 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1489,17 +1489,14 @@ static void pci_msi_init_pci_dev(struct pci_dev *dev)
>  	INIT_LIST_HEAD(&dev->msi_list);
>  #endif
>  
> +	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
> +	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> +
>  	/* Disable the msi hardware to avoid screaming interrupts
>  	 * during boot.  This is the power on reset default so
>  	 * usually this should be a noop.
>  	 */
> -	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
> -	if (dev->msi_cap)
> -		pci_msi_set_enable(dev, 0);
> -
> -	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> -	if (dev->msix_cap)
> -		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
> +	pci_msi_off(dev);
>  }
>  
>  static void pci_init_capabilities(struct pci_dev *dev)
> -- 
> MST
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 03/10] pci: drop some duplicate code
  2015-03-24 23:33   ` Bjorn Helgaas
@ 2015-03-25  5:40     ` Michael S. Tsirkin
  0 siblings, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2015-03-25  5:40 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-kernel, stable, linux-pci, Fam Zheng, Yinghai Lu

On Tue, Mar 24, 2015 at 06:33:57PM -0500, Bjorn Helgaas wrote:
> On Tue, Mar 24, 2015 at 04:42:35PM +0100, Michael S. Tsirkin wrote:
> > pci_msi_init_pci_dev and pci_msi_off share a lot of code.
> > This used to be justified since pci_msi_init_pci_dev
> > wasn't compiled in when CONFIG_PCI_MSI is off.
> > Now that it is, let's reuse code.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  drivers/pci/pci.c   | 24 +++++-------------------
> >  drivers/pci/probe.c | 11 ++++-------
> >  2 files changed, 9 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 81f06e8..d5f297a 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3106,26 +3106,12 @@ EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
> >   */
> >  void pci_msi_off(struct pci_dev *dev)
> >  {
> > -	int pos;
> > -	u16 control;
> > +	if (dev->msi_cap)
> > +		pci_msi_set_enable(dev, 0);
> >  
> > -	/*
> > -	 * This looks like it could go in msi.c, but we need it even when
> > -	 * CONFIG_PCI_MSI=n.  For the same reason, we can't use
> > -	 * dev->msi_cap or dev->msix_cap here.
> > -	 */
> > -	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
> > -	if (pos) {
> > -		pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
> > -		control &= ~PCI_MSI_FLAGS_ENABLE;
> > -		pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
> > -	}
> > -	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> > -	if (pos) {
> > -		pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
> > -		control &= ~PCI_MSIX_FLAGS_ENABLE;
> > -		pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
> > -	}
> > +	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> 
> I think the above line is superfluous, isn't it?  You already do the same
> in pci_msi_init_pci_dev() (below).

Ugh. Yea. I'll fix that up, thanks!

> > +	if (dev->msix_cap)
> > +		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
> >  }
> >  EXPORT_SYMBOL_GPL(pci_msi_off);
> >  
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 45d6d5c..baf8ddd 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1489,17 +1489,14 @@ static void pci_msi_init_pci_dev(struct pci_dev *dev)
> >  	INIT_LIST_HEAD(&dev->msi_list);
> >  #endif
> >  
> > +	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
> > +	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> > +
> >  	/* Disable the msi hardware to avoid screaming interrupts
> >  	 * during boot.  This is the power on reset default so
> >  	 * usually this should be a noop.
> >  	 */
> > -	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
> > -	if (dev->msi_cap)
> > -		pci_msi_set_enable(dev, 0);
> > -
> > -	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> > -	if (dev->msix_cap)
> > -		pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
> > +	pci_msi_off(dev);
> >  }
> >  
> >  static void pci_init_capabilities(struct pci_dev *dev)
> > -- 
> > MST
> > 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-03-25  5:40 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-24 15:42 [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Michael S. Tsirkin
2015-03-24 15:42 ` [PATCH v3 01/10] pci: export functions for msi/msix ctrl Michael S. Tsirkin
2015-03-24 15:42 ` [PATCH v3 02/10] pci: move pci_msi_init_pci_dev to pci.c Michael S. Tsirkin
2015-03-24 15:42 ` [PATCH v3 03/10] pci: drop some duplicate code Michael S. Tsirkin
2015-03-24 23:33   ` Bjorn Helgaas
2015-03-25  5:40     ` Michael S. Tsirkin
2015-03-24 15:42 ` [PATCH v3 04/10] pci: don't disable msi/msix at shutdown Michael S. Tsirkin
2015-03-24 15:42 ` [PATCH v3 05/10] pci: make msi/msix shutdown functions static Michael S. Tsirkin
2015-03-24 15:42 ` [PATCH v3 06/10] virtio_pci: drop msi_off on probe Michael S. Tsirkin
2015-03-24 15:42   ` Michael S. Tsirkin
2015-03-24 15:42 ` [PATCH v3 07/10] ntb: drop pci_msi_off call " Michael S. Tsirkin
2015-03-24 16:12   ` Jiang, Dave
2015-03-24 16:12     ` Jiang, Dave
2015-03-24 16:12     ` Jiang, Dave
2015-03-24 15:42 ` [PATCH v3 08/10] mic: " Michael S. Tsirkin
2015-03-24 15:42 ` [PATCH v3 09/10] pci: drop pci_msi_off calls from quirks Michael S. Tsirkin
2015-03-24 15:43 ` [PATCH v3 10/10] pci: unexport pci_msi_off Michael S. Tsirkin
2015-03-24 17:02 ` [PATCH v3 00/10] pci: fix unhandled interrupt on shutdown Greg KH

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.