* Re: [RFC PATCH] PCI: Disable MSI/MSI-X only if device is shutdown [not found] <1426137682-12287-1-git-send-email-famz@redhat.com> @ 2015-03-12 16:21 ` Paolo Bonzini 2015-03-12 23:21 ` Fam Zheng [not found] ` <20150312232104.GA32054@ad.nay.redhat.com> 0 siblings, 2 replies; 3+ messages in thread From: Paolo Bonzini @ 2015-03-12 16:21 UTC (permalink / raw) To: Fam Zheng, linux-kernel; +Cc: Bjorn Helgaas, linux-pci, Linux Virtualization 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] PCI: Disable MSI/MSI-X only if device is shutdown 2015-03-12 16:21 ` [RFC PATCH] PCI: Disable MSI/MSI-X only if device is shutdown Paolo Bonzini @ 2015-03-12 23:21 ` Fam Zheng [not found] ` <20150312232104.GA32054@ad.nay.redhat.com> 1 sibling, 0 replies; 3+ messages in thread From: Fam Zheng @ 2015-03-12 23:21 UTC (permalink / raw) To: Paolo Bonzini Cc: Bjorn Helgaas, linux-pci, mst, linux-kernel, Linux Virtualization 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <20150312232104.GA32054@ad.nay.redhat.com>]
* Re: [RFC PATCH] PCI: Disable MSI/MSI-X only if device is shutdown [not found] ` <20150312232104.GA32054@ad.nay.redhat.com> @ 2015-03-13 13:03 ` Paolo Bonzini 0 siblings, 0 replies; 3+ messages in thread From: Paolo Bonzini @ 2015-03-13 13:03 UTC (permalink / raw) To: Fam Zheng Cc: Bjorn Helgaas, linux-pci, mst, linux-kernel, Linux Virtualization 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-03-13 13:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1426137682-12287-1-git-send-email-famz@redhat.com>
2015-03-12 16:21 ` [RFC PATCH] PCI: Disable MSI/MSI-X only if device is shutdown Paolo Bonzini
2015-03-12 23:21 ` Fam Zheng
[not found] ` <20150312232104.GA32054@ad.nay.redhat.com>
2015-03-13 13:03 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox