| Message ID | 1426137682-12287-1-git-send-email-famz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 12/03/2015 06:21, Fam Zheng wrote: > If the device doesn't support shutdown, disabling interrupts may cause > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and > after we disable MSI-X, futher notifications from device will be > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and > may prevent us from making progress, by keep triggering interrupts. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > drivers/pci/pci-driver.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index 3cb2210..fb29c96 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) > > pm_runtime_resume(dev); > > - if (drv && drv->shutdown) > + if (drv && drv->shutdown) { > drv->shutdown(pci_dev); > - pci_msi_shutdown(pci_dev); > - pci_msix_shutdown(pci_dev); > + pci_msi_shutdown(pci_dev); > + pci_msix_shutdown(pci_dev); > + } > > #ifdef CONFIG_KEXEC > /* > The patch may be okay, but I think the bug here is also that virtio-pci is not defining a .shutdown callback. It should define one and call free_irq (for INTX) and pci_disable_msix. How is this related to the virtio-scsi patch that you posted? Do you need both to fix the problem you reported? Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 03/12 17:21, Paolo Bonzini wrote: > On 12/03/2015 06:21, Fam Zheng wrote: > > If the device doesn't support shutdown, disabling interrupts may cause > > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and > > after we disable MSI-X, futher notifications from device will be > > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and > > may prevent us from making progress, by keep triggering interrupts. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > drivers/pci/pci-driver.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > > index 3cb2210..fb29c96 100644 > > --- a/drivers/pci/pci-driver.c > > +++ b/drivers/pci/pci-driver.c > > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) > > > > pm_runtime_resume(dev); > > > > - if (drv && drv->shutdown) > > + if (drv && drv->shutdown) { > > drv->shutdown(pci_dev); > > - pci_msi_shutdown(pci_dev); > > - pci_msix_shutdown(pci_dev); > > + pci_msi_shutdown(pci_dev); > > + pci_msix_shutdown(pci_dev); > > + } > > > > #ifdef CONFIG_KEXEC > > /* > > > > The patch may be okay, but I think the bug here is also that virtio-pci > is not defining a .shutdown callback. It should define one and call > free_irq (for INTX) and pci_disable_msix. It's not enough. The device has to know we disabled msix, otherwise it will send us IRQ, which is wrong. > > How is this related to the virtio-scsi patch that you posted? Do you > need both to fix the problem you reported? > This one should be enough. I was wrong in saying virtio-scsi hanging the shutdown is because requests not being completed, it's more because of the unexpected IRQ as explained in [1]. [1]: https://bugzilla.redhat.com/attachment.cgi?id=998391 Fam -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Fam, Fam Zheng <famz@redhat.com> writes: > If the device doesn't support shutdown, disabling interrupts may cause > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and > after we disable MSI-X, futher notifications from device will be > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and > may prevent us from making progress, by keep triggering interrupts. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > drivers/pci/pci-driver.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index 3cb2210..fb29c96 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) > > pm_runtime_resume(dev); > > - if (drv && drv->shutdown) > + if (drv && drv->shutdown) { > drv->shutdown(pci_dev); > - pci_msi_shutdown(pci_dev); > - pci_msix_shutdown(pci_dev); > + pci_msi_shutdown(pci_dev); > + pci_msix_shutdown(pci_dev); > + } If the driver doesn't provide a shutdown method and doesn't itself disable MSI/MSI-X, then pci_msi(x)_shutdown won't be called anymore, no ? This is probably ok (but unclean) for a system shutdown, but could cause problems for something like kexec ? Bandan > #ifdef CONFIG_KEXEC > /* -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 03/12 19:56, Bandan Das wrote: > Hi Fam, > > Fam Zheng <famz@redhat.com> writes: > > > If the device doesn't support shutdown, disabling interrupts may cause > > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and > > after we disable MSI-X, futher notifications from device will be > > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and > > may prevent us from making progress, by keep triggering interrupts. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > drivers/pci/pci-driver.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > > index 3cb2210..fb29c96 100644 > > --- a/drivers/pci/pci-driver.c > > +++ b/drivers/pci/pci-driver.c > > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) > > > > pm_runtime_resume(dev); > > > > - if (drv && drv->shutdown) > > + if (drv && drv->shutdown) { > > drv->shutdown(pci_dev); > > - pci_msi_shutdown(pci_dev); > > - pci_msix_shutdown(pci_dev); > > + pci_msi_shutdown(pci_dev); > > + pci_msix_shutdown(pci_dev); > > + } > > If the driver doesn't provide a shutdown method and doesn't itself > disable MSI/MSI-X, then pci_msi(x)_shutdown won't be called anymore, > no ? Right. > > This is probably ok (but unclean) for a system shutdown, but could > cause problems for something like kexec ? Doesn't the reset in kexec clean this up during initialization? Without shutdown, anything in the driver is unclean anyway, for example free_irq is not called. Fam -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Ccing Alex, he can probably confirm if my understanding is indeed correct. Fam Zheng <famz@redhat.com> writes: > On Thu, 03/12 19:56, Bandan Das wrote: >> Hi Fam, >> >> Fam Zheng <famz@redhat.com> writes: >> >> > If the device doesn't support shutdown, disabling interrupts may cause >> > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and >> > after we disable MSI-X, futher notifications from device will be >> > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and >> > may prevent us from making progress, by keep triggering interrupts. >> > >> > Signed-off-by: Fam Zheng <famz@redhat.com> >> > --- >> > drivers/pci/pci-driver.c | 7 ++++--- >> > 1 file changed, 4 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c >> > index 3cb2210..fb29c96 100644 >> > --- a/drivers/pci/pci-driver.c >> > +++ b/drivers/pci/pci-driver.c >> > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) >> > >> > pm_runtime_resume(dev); >> > >> > - if (drv && drv->shutdown) >> > + if (drv && drv->shutdown) { >> > drv->shutdown(pci_dev); >> > - pci_msi_shutdown(pci_dev); >> > - pci_msix_shutdown(pci_dev); >> > + pci_msi_shutdown(pci_dev); >> > + pci_msix_shutdown(pci_dev); >> > + } >> >> If the driver doesn't provide a shutdown method and doesn't itself >> disable MSI/MSI-X, then pci_msi(x)_shutdown won't be called anymore, >> no ? > > Right. > >> >> This is probably ok (but unclean) for a system shutdown, but could >> cause problems for something like kexec ? > > Doesn't the reset in kexec clean this up during initialization? I guess it would take the same path as a reboot. > Without shutdown, anything in the driver is unclean anyway, for example > free_irq is not called. True, And that is why the MSI/-X shutdown functions are called here because pci can't always rely on the individual device drivers to do the right thing, but atleast can make a best effort at cleaning up. > Fam -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 03/12 23:09, Bandan Das wrote: > Ccing Alex, he can probably confirm if my understanding is indeed correct. > > Fam Zheng <famz@redhat.com> writes: > > > On Thu, 03/12 19:56, Bandan Das wrote: > >> Hi Fam, > >> > >> Fam Zheng <famz@redhat.com> writes: > >> > >> > If the device doesn't support shutdown, disabling interrupts may cause > >> > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and > >> > after we disable MSI-X, futher notifications from device will be > >> > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and > >> > may prevent us from making progress, by keep triggering interrupts. > >> > > >> > Signed-off-by: Fam Zheng <famz@redhat.com> > >> > --- > >> > drivers/pci/pci-driver.c | 7 ++++--- > >> > 1 file changed, 4 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > >> > index 3cb2210..fb29c96 100644 > >> > --- a/drivers/pci/pci-driver.c > >> > +++ b/drivers/pci/pci-driver.c > >> > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) > >> > > >> > pm_runtime_resume(dev); > >> > > >> > - if (drv && drv->shutdown) > >> > + if (drv && drv->shutdown) { > >> > drv->shutdown(pci_dev); > >> > - pci_msi_shutdown(pci_dev); > >> > - pci_msix_shutdown(pci_dev); > >> > + pci_msi_shutdown(pci_dev); > >> > + pci_msix_shutdown(pci_dev); > >> > + } > >> > >> If the driver doesn't provide a shutdown method and doesn't itself > >> disable MSI/MSI-X, then pci_msi(x)_shutdown won't be called anymore, > >> no ? > > > > Right. > > > >> > >> This is probably ok (but unclean) for a system shutdown, but could > >> cause problems for something like kexec ? > > > > Doesn't the reset in kexec clean this up during initialization? > > I guess it would take the same path as a reboot. I don't understand, do you mean that no reset will be done before more operations on the device? > > > Without shutdown, anything in the driver is unclean anyway, for example > > free_irq is not called. > > True, And that is why the MSI/-X shutdown functions are called here > because pci can't always rely on the individual device drivers to do > the right thing, but atleast can make a best effort at cleaning up. This virtio-scsi case shows us that intermediate state is bad, so I still think we should call pci_msix_shutdown conditionally. Fam -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fam Zheng <famz@redhat.com> writes: > On Thu, 03/12 23:09, Bandan Das wrote: >> Ccing Alex, he can probably confirm if my understanding is indeed correct. >> >> Fam Zheng <famz@redhat.com> writes: >> >> > On Thu, 03/12 19:56, Bandan Das wrote: >> >> Hi Fam, >> >> >> >> Fam Zheng <famz@redhat.com> writes: >> >> >> >> > If the device doesn't support shutdown, disabling interrupts may cause >> >> > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and >> >> > after we disable MSI-X, futher notifications from device will be >> >> > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and >> >> > may prevent us from making progress, by keep triggering interrupts. >> >> > >> >> > Signed-off-by: Fam Zheng <famz@redhat.com> >> >> > --- >> >> > drivers/pci/pci-driver.c | 7 ++++--- >> >> > 1 file changed, 4 insertions(+), 3 deletions(-) >> >> > >> >> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c >> >> > index 3cb2210..fb29c96 100644 >> >> > --- a/drivers/pci/pci-driver.c >> >> > +++ b/drivers/pci/pci-driver.c >> >> > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) >> >> > >> >> > pm_runtime_resume(dev); >> >> > >> >> > - if (drv && drv->shutdown) >> >> > + if (drv && drv->shutdown) { >> >> > drv->shutdown(pci_dev); >> >> > - pci_msi_shutdown(pci_dev); >> >> > - pci_msix_shutdown(pci_dev); >> >> > + pci_msi_shutdown(pci_dev); >> >> > + pci_msix_shutdown(pci_dev); >> >> > + } >> >> >> >> If the driver doesn't provide a shutdown method and doesn't itself >> >> disable MSI/MSI-X, then pci_msi(x)_shutdown won't be called anymore, >> >> no ? >> > >> > Right. >> > >> >> >> >> This is probably ok (but unclean) for a system shutdown, but could >> >> cause problems for something like kexec ? >> > >> > Doesn't the reset in kexec clean this up during initialization? >> >> I guess it would take the same path as a reboot. > > I don't understand, do you mean that no reset will be done before more > operations on the device? I meant that it's upto the individual driver, pci will take the same path as a regular reboot. >> >> > Without shutdown, anything in the driver is unclean anyway, for example >> > free_irq is not called. >> >> True, And that is why the MSI/-X shutdown functions are called here >> because pci can't always rely on the individual device drivers to do >> the right thing, but atleast can make a best effort at cleaning up. > > This virtio-scsi case shows us that intermediate state is bad, so I still think > we should call pci_msix_shutdown conditionally. > > Fam -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2015-03-12 at 23:09 -0400, Bandan Das wrote: > Ccing Alex, he can probably confirm if my understanding is indeed correct. > > Fam Zheng <famz@redhat.com> writes: > > > On Thu, 03/12 19:56, Bandan Das wrote: > >> Hi Fam, > >> > >> Fam Zheng <famz@redhat.com> writes: > >> > >> > If the device doesn't support shutdown, disabling interrupts may cause > >> > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and > >> > after we disable MSI-X, futher notifications from device will be > >> > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and > >> > may prevent us from making progress, by keep triggering interrupts. Won't it be disabled as a spurious interrupt if it's retriggering that quickly? Is there something unique about virtio that makes this worse than bare metal? > >> > Signed-off-by: Fam Zheng <famz@redhat.com> > >> > --- > >> > drivers/pci/pci-driver.c | 7 ++++--- > >> > 1 file changed, 4 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > >> > index 3cb2210..fb29c96 100644 > >> > --- a/drivers/pci/pci-driver.c > >> > +++ b/drivers/pci/pci-driver.c > >> > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) > >> > > >> > pm_runtime_resume(dev); > >> > > >> > - if (drv && drv->shutdown) > >> > + if (drv && drv->shutdown) { > >> > drv->shutdown(pci_dev); > >> > - pci_msi_shutdown(pci_dev); > >> > - pci_msix_shutdown(pci_dev); > >> > + pci_msi_shutdown(pci_dev); > >> > + pci_msix_shutdown(pci_dev); > >> > + } > >> > >> If the driver doesn't provide a shutdown method and doesn't itself > >> disable MSI/MSI-X, then pci_msi(x)_shutdown won't be called anymore, > >> no ? > > > > Right. > > > >> > >> This is probably ok (but unclean) for a system shutdown, but could > >> cause problems for something like kexec ? > > > > Doesn't the reset in kexec clean this up during initialization? > > I guess it would take the same path as a reboot. If we were to kexec, the only thing I see touching MSI/X enable bits is pci_msi_init_pci_dev(), which unconditionally disables MSI/X as we walk PCI and discover devices. Why is disabling there any different? A new instance of the virtio driver hasn't yet been bound to the device to quiesce it. > > Without shutdown, anything in the driver is unclean anyway, for example > > free_irq is not called. > > True, And that is why the MSI/-X shutdown functions are called here > because pci can't always rely on the individual device drivers to do > the right thing, but atleast can make a best effort at cleaning up. A concern with this patch would be that some drivers potentially don't implement a shutdown routine because they rely on pci-core to do this minimal cleanup. I'm tempted to suggest adding a call to pci_intx() to disable INTx as part of the PCI-core shutdown, but that sounds like asking for trouble with broken legacy devices. It sure seems a lot safer to make a virtio-scsi-pci shutdown function. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 03/12 22:13, Alex Williamson wrote: > On Thu, 2015-03-12 at 23:09 -0400, Bandan Das wrote: > > Ccing Alex, he can probably confirm if my understanding is indeed correct. > > > > Fam Zheng <famz@redhat.com> writes: > > > > > On Thu, 03/12 19:56, Bandan Das wrote: > > >> Hi Fam, > > >> > > >> Fam Zheng <famz@redhat.com> writes: > > >> > > >> > If the device doesn't support shutdown, disabling interrupts may cause > > >> > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and > > >> > after we disable MSI-X, futher notifications from device will be > > >> > delivered to IRQ, which is unexpected. This IRQ will not be cleared, and > > >> > may prevent us from making progress, by keep triggering interrupts. > > Won't it be disabled as a spurious interrupt if it's retriggering that > quickly? You're right, it will. Thanks. Fam > > > >> > Signed-off-by: Fam Zheng <famz@redhat.com> > > >> > --- > > >> > drivers/pci/pci-driver.c | 7 ++++--- > > >> > 1 file changed, 4 insertions(+), 3 deletions(-) > > >> > > > >> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > > >> > index 3cb2210..fb29c96 100644 > > >> > --- a/drivers/pci/pci-driver.c > > >> > +++ b/drivers/pci/pci-driver.c > > >> > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) > > >> > > > >> > pm_runtime_resume(dev); > > >> > > > >> > - if (drv && drv->shutdown) > > >> > + if (drv && drv->shutdown) { > > >> > drv->shutdown(pci_dev); > > >> > - pci_msi_shutdown(pci_dev); > > >> > - pci_msix_shutdown(pci_dev); > > >> > + pci_msi_shutdown(pci_dev); > > >> > + pci_msix_shutdown(pci_dev); > > >> > + } > > >> > > >> If the driver doesn't provide a shutdown method and doesn't itself > > >> disable MSI/MSI-X, then pci_msi(x)_shutdown won't be called anymore, > > >> no ? > > > > > > Right. > > > > > >> > > >> This is probably ok (but unclean) for a system shutdown, but could > > >> cause problems for something like kexec ? > > > > > > Doesn't the reset in kexec clean this up during initialization? > > > > I guess it would take the same path as a reboot. > > If we were to kexec, the only thing I see touching MSI/X enable bits is > pci_msi_init_pci_dev(), which unconditionally disables MSI/X as we walk > PCI and discover devices. Why is disabling there any different? A new > instance of the virtio driver hasn't yet been bound to the device to > quiesce it. > > > > Without shutdown, anything in the driver is unclean anyway, for example > > > free_irq is not called. > > > > True, And that is why the MSI/-X shutdown functions are called here > > because pci can't always rely on the individual device drivers to do > > the right thing, but atleast can make a best effort at cleaning up. > > A concern with this patch would be that some drivers potentially don't > implement a shutdown routine because they rely on pci-core to do this > minimal cleanup. I'm tempted to suggest adding a call to pci_intx() to > disable INTx as part of the PCI-core shutdown, but that sounds like > asking for trouble with broken legacy devices. It sure seems a lot > safer to make a virtio-scsi-pci shutdown function. Thanks, > > Alex > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13/03/2015 00:21, Fam Zheng wrote: >> I think the bug here is also that virtio-pci >> is not defining a .shutdown callback. It should define one and call >> free_irq (for INTX) and pci_disable_msix. > > It's not enough. The device has to know we disabled msix, otherwise it will > send us IRQ, which is wrong. You can use pci_intx to disable INTX too. So I think this is a virtio-pci bug. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 12, 2015 at 01:21:22PM +0800, Fam Zheng wrote: > If the device doesn't support shutdown, disabling interrupts may cause > trouble. For example, virtio-scsi-pci doesn't implement shutdown, and > after we disable MSI-X, futher notifications from device will be > delivered to IRQ, which is unexpected. > This IRQ will not be cleared, and > may prevent us from making progress, by keep triggering interrupts. I would say: Since there's no handler, the interrupt line will never be cleared, causing a deadlock. > > Signed-off-by: Fam Zheng <famz@redhat.com> However, see below: > --- > drivers/pci/pci-driver.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index 3cb2210..fb29c96 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) > > pm_runtime_resume(dev); > > - if (drv && drv->shutdown) > + if (drv && drv->shutdown) { > drv->shutdown(pci_dev); > - pci_msi_shutdown(pci_dev); > - pci_msix_shutdown(pci_dev); > + pci_msi_shutdown(pci_dev); > + pci_msix_shutdown(pci_dev); > + } > So the following bus reset will disable msi anyway, IMHO there's no need to do it in software. kexec is more interesting. This is an attempt to leave device in a consistent state: commit d52877c7b1afb8c37ebe17e2005040b79cb618b0 Author: Yinghai Lu <yhlu.kernel.send@gmail.com> Date: Wed Apr 23 14:58:09 2008 -0700 pci/irq: let pci_device_shutdown to call pci_msi_shutdown v2 but arguably, it's better to disable msi/msix when kexec starts - for example, kexec might run after a crash (kdump) and shutdown callbacks are not invoked in that case. The reason this does not trigger for MPT is because it has an intx handler so that gets invoked. So here's my proposal: - for 4.0, let's just do a virtio specific work-around, this seems safer. - for 4.1, let's disable msi/msix first thing on kexec startup, before driver probe. I'll post both patches shortly. > #ifdef CONFIG_KEXEC > /* > -- > 1.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 3cb2210..fb29c96 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -448,10 +448,11 @@ static void pci_device_shutdown(struct device *dev) pm_runtime_resume(dev); - if (drv && drv->shutdown) + if (drv && drv->shutdown) { drv->shutdown(pci_dev); - pci_msi_shutdown(pci_dev); - pci_msix_shutdown(pci_dev); + pci_msi_shutdown(pci_dev); + pci_msix_shutdown(pci_dev); + } #ifdef CONFIG_KEXEC /*
If the device doesn't support shutdown, disabling interrupts may cause trouble. For example, virtio-scsi-pci doesn't implement shutdown, and after we disable MSI-X, futher notifications from device will be delivered to IRQ, which is unexpected. This IRQ will not be cleared, and may prevent us from making progress, by keep triggering interrupts. Signed-off-by: Fam Zheng <famz@redhat.com> --- drivers/pci/pci-driver.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)