search for: pci_disable_msix

..._unmap(struct nvme_dev *dev) { > - if (dev->pci_dev->msi_enabled) > + if (pci_dev_msi_enabled(dev->pci_dev, MSI_TYPE)) > pci_disable_msi(dev->pci_dev); > - else if (dev->pci_dev->msix_enabled) > + else if (pci_dev_msi_enabled(dev->pci_dev, MSIX_TYPE)) > pci_disable_msix(dev->pci_dev); > > if (dev->bar) { > diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c index > 4e3549a..a11dac1 100644 > --- a/drivers/dma/ioat/dma.c > +++ b/drivers/dma/ioat/dma.c > @@ -1088,7 +1088,7 @@ static void ioat1_intr_quirk(struct ioatdma_device &g...

..._unmap(struct nvme_dev *dev) { > - if (dev->pci_dev->msi_enabled) > + if (pci_dev_msi_enabled(dev->pci_dev, MSI_TYPE)) > pci_disable_msi(dev->pci_dev); > - else if (dev->pci_dev->msix_enabled) > + else if (pci_dev_msi_enabled(dev->pci_dev, MSIX_TYPE)) > pci_disable_msix(dev->pci_dev); > > if (dev->bar) { > diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c index > 4e3549a..a11dac1 100644 > --- a/drivers/dma/ioat/dma.c > +++ b/drivers/dma/ioat/dma.c > @@ -1088,7 +1088,7 @@ static void ioat1_intr_quirk(struct ioatdma_device &g...

...ev); > + 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

...ev); > + 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

...*dev) static void nvme_dev_unmap(struct nvme_dev *dev) { - if (dev->pci_dev->msi_enabled) + if (pci_dev_msi_enabled(dev->pci_dev, MSI_TYPE)) pci_disable_msi(dev->pci_dev); - else if (dev->pci_dev->msix_enabled) + else if (pci_dev_msi_enabled(dev->pci_dev, MSIX_TYPE)) pci_disable_msix(dev->pci_dev); if (dev->bar) { diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c index 4e3549a..a11dac1 100644 --- a/drivers/dma/ioat/dma.c +++ b/drivers/dma/ioat/dma.c @@ -1088,7 +1088,7 @@ static void ioat1_intr_quirk(struct ioatdma_device *device) u32 dmactrl; pci_read...

...1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index a7ce730..1aba255 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -289,9 +289,9 @@ static void vp_free_vectors(struct virtio_device *vdev) pci_disable_msix(vp_dev->pci_dev); vp_dev->msix_enabled = 0; - vp_dev->msix_vectors = 0; } + vp_dev->msix_vectors = 0; vp_dev->msix_used_vectors = 0; kfree(vp_dev->msix_names); vp_dev->msix_names = NULL; @@ -309,6 +309,8 @@ static int vp_request_msix_vectors(struct virtio_device *...

...1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index a7ce730..1aba255 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -289,9 +289,9 @@ static void vp_free_vectors(struct virtio_device *vdev) pci_disable_msix(vp_dev->pci_dev); vp_dev->msix_enabled = 0; - vp_dev->msix_vectors = 0; } + vp_dev->msix_vectors = 0; vp_dev->msix_used_vectors = 0; kfree(vp_dev->msix_names); vp_dev->msix_names = NULL; @@ -309,6 +309,8 @@ static int vp_request_msix_vectors(struct virtio_device *...

Hi, Keir, Jan, This patch is a bugfix pointed by Jan. Fix mask_base(actually MSI-X table base, copy name from native) to be a virtual address rather than a physical address. And remove wrong printk in pci_disable_msix. Jan, the error message you saw is wrong output from kernel''s MSI code. Really sorry for my dirty code there. Could you please review the patch and give me feedback? Best Regards Haitao Shan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensour...

...ix_affinity_masks[i]); if (vp_dev->msix_enabled) { - /* Disable the vector used for configuration */ - iowrite16(VIRTIO_MSI_NO_VECTOR, - vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); - /* Flush the write out to device */ - ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); - pci_disable_msix(vp_dev->pci_dev); vp_dev->msix_enabled = 0; } @@ -339,24 +334,18 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, goto error; vp_dev->msix_enabled = 1; - /* Set the vector used for configuration */ - v = vp_dev->msix_used_vectors; - snprintf(vp...

...ix_affinity_masks[i]); if (vp_dev->msix_enabled) { - /* Disable the vector used for configuration */ - iowrite16(VIRTIO_MSI_NO_VECTOR, - vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); - /* Flush the write out to device */ - ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); - pci_disable_msix(vp_dev->pci_dev); vp_dev->msix_enabled = 0; } @@ -339,24 +334,18 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, goto error; vp_dev->msix_enabled = 1; - /* Set the vector used for configuration */ - v = vp_dev->msix_used_vectors; - snprintf(vp...

...> return 0; >> +error_msix_used: >> + v = --vp_dev->msix_used_vectors; >> + free_irq(vp_dev->msix_entries[v].vector, vp_dev); >> error: >> + vp_dev->msix_enabled = 0; > > As far as I can see, if you do this, guest will not call > pci_disable_msix thus leaving the device with MSIX enabled. I don't understand this comment. How is the work done in this path any different from what's done in vp_free_vectors()? Regards, Anthony Liguori > I'm not sure this won't break drivers if they then > try to use the device without...

...> return 0; >> +error_msix_used: >> + v = --vp_dev->msix_used_vectors; >> + free_irq(vp_dev->msix_entries[v].vector, vp_dev); >> error: >> + vp_dev->msix_enabled = 0; > > As far as I can see, if you do this, guest will not call > pci_disable_msix thus leaving the device with MSIX enabled. I don't understand this comment. How is the work done in this path any different from what's done in vp_free_vectors()? Regards, Anthony Liguori > I'm not sure this won't break drivers if they then > try to use the device without...

vp_dev->msix_vectors should be initialized before allocating msix_affinity_masks, otherwise vp_free_vectors will not free these objects. unreferenced object 0xffff88010f969d88 (size 512): comm "systemd-udevd", pid 158, jiffies 4294673645 (age 80.545s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00

vp_dev->msix_vectors should be initialized before allocating msix_affinity_masks, otherwise vp_free_vectors will not free these objects. unreferenced object 0xffff88010f969d88 (size 512): comm "systemd-udevd", pid 158, jiffies 4294673645 (age 80.545s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00

From: Anthony Liguori <aliguori at amazon.com> If MSI-X initialization fails after setting msix_enabled = 1, then the device is left in an inconsistent state. This would normally only happen if there was a bug in the device emulation but it still should be handled correctly. Cc: Matt Wilson <msw at amazon.com> Cc: Rusty Russell <rusty at rustcorp.com.au> Cc: Michael Tsirkin

From: Anthony Liguori <aliguori at amazon.com> If MSI-X initialization fails after setting msix_enabled = 1, then the device is left in an inconsistent state. This would normally only happen if there was a bug in the device emulation but it still should be handled correctly. Cc: Matt Wilson <msw at amazon.com> Cc: Rusty Russell <rusty at rustcorp.com.au> Cc: Michael Tsirkin

.../* Disable the vector used for configuration */ > > - iowrite16(VIRTIO_MSI_NO_VECTOR, > > - vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); > > - /* Flush the write out to device */ > > - ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); > > - > > pci_disable_msix(vp_dev->pci_dev); > > vp_dev->msix_enabled = 0; > > } > > @@ -339,24 +334,18 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, > > goto error; > > vp_dev->msix_enabled = 1; > > > > - /* Set the vector used...

.../* Disable the vector used for configuration */ > > - iowrite16(VIRTIO_MSI_NO_VECTOR, > > - vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); > > - /* Flush the write out to device */ > > - ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); > > - > > pci_disable_msix(vp_dev->pci_dev); > > vp_dev->msix_enabled = 0; > > } > > @@ -339,24 +334,18 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, > > goto error; > > vp_dev->msix_enabled = 1; > > > > - /* Set the vector used...

...y: Michael S. Tsirkin <mst at redhat.com> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 03564fe..4236d92 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -289,9 +289,9 @@ static void vp_free_vectors(struct virtio_device *vdev) pci_disable_msix(vp_dev->pci_dev); vp_dev->msix_enabled = 0; - vp_dev->msix_vectors = 0; } + vp_dev->msix_vectors = 0; vp_dev->msix_used_vectors = 0; kfree(vp_dev->msix_names); vp_dev->msix_names = NULL; > -- > 1.7.1

...etions(-) > > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c > index a7ce730..1aba255 100644 > --- a/drivers/virtio/virtio_pci.c > +++ b/drivers/virtio/virtio_pci.c > @@ -289,9 +289,9 @@ static void vp_free_vectors(struct virtio_device *vdev) > > pci_disable_msix(vp_dev->pci_dev); > vp_dev->msix_enabled = 0; > - vp_dev->msix_vectors = 0; > } > > + vp_dev->msix_vectors = 0; > vp_dev->msix_used_vectors = 0; > kfree(vp_dev->msix_names); > vp_dev->msix_names = NULL; > @@ -309,6 +309,8 @@ static int v...