From: Jason Wang <jasowang@redhat.com>
To: mst@redhat.com, jasowang@redhat.com,
virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org
Cc: tglx@linutronix.de, peterz@infradead.org,
Marc Zyngier <maz@kernel.org>,
Stefano Garzarella <sgarzare@redhat.com>
Subject: [PATCH 2/2] Revert "virtio_pci: harden MSI-X interrupts"
Date: Wed, 23 Mar 2022 11:15:24 +0800 [thread overview]
Message-ID: <20220323031524.6555-2-jasowang@redhat.com> (raw)
In-Reply-To: <20220323031524.6555-1-jasowang@redhat.com>
This reverts commit 9e35276a5344f74d4a3600fc4100b3dd251d5c56. Issue
were reported for the drivers that are using affinity managed IRQ
where manually toggling IRQ status is not expected. And we forget to
enable the interrupts in the restore path as well.
In the future, we will rework on the interrupt hardening.
Fixes: 9e35276a5344 ("virtio_pci: harden MSI-X interrupts")
Reported-by: Marc Zyngier <maz@kernel.org>
Reported-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_common.c | 27 ++++++---------------------
drivers/virtio/virtio_pci_common.h | 6 ++----
drivers/virtio/virtio_pci_legacy.c | 5 ++---
drivers/virtio/virtio_pci_modern.c | 6 ++----
4 files changed, 12 insertions(+), 32 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 3f51fdb7be45..d724f676608b 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -24,8 +24,8 @@ MODULE_PARM_DESC(force_legacy,
"Force legacy mode for transitional virtio 1 devices");
#endif
-/* disable irq handlers */
-void vp_disable_cbs(struct virtio_device *vdev)
+/* wait for pending irq handlers */
+void vp_synchronize_vectors(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
int i;
@@ -34,20 +34,7 @@ void vp_disable_cbs(struct virtio_device *vdev)
synchronize_irq(vp_dev->pci_dev->irq);
for (i = 0; i < vp_dev->msix_vectors; ++i)
- disable_irq(pci_irq_vector(vp_dev->pci_dev, i));
-}
-
-/* enable irq handlers */
-void vp_enable_cbs(struct virtio_device *vdev)
-{
- struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- int i;
-
- if (vp_dev->intx_enabled)
- return;
-
- for (i = 0; i < vp_dev->msix_vectors; ++i)
- enable_irq(pci_irq_vector(vp_dev->pci_dev, i));
+ synchronize_irq(pci_irq_vector(vp_dev->pci_dev, i));
}
/* the notify function used when creating a virt queue */
@@ -154,8 +141,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
"%s-config", name);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, v),
- vp_config_changed, IRQF_NO_AUTOEN,
- vp_dev->msix_names[v],
+ vp_config_changed, 0, vp_dev->msix_names[v],
vp_dev);
if (err)
goto error;
@@ -174,8 +160,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
"%s-virtqueues", name);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, v),
- vp_vring_interrupt, IRQF_NO_AUTOEN,
- vp_dev->msix_names[v],
+ vp_vring_interrupt, 0, vp_dev->msix_names[v],
vp_dev);
if (err)
goto error;
@@ -352,7 +337,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
"%s-%s",
dev_name(&vp_dev->vdev.dev), names[i]);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
- vring_interrupt, IRQF_NO_AUTOEN,
+ vring_interrupt, 0,
vp_dev->msix_names[msix_vec],
vqs[i]);
if (err)
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index d3c6f72c7390..eb17a29fc7ef 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -101,10 +101,8 @@ static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
return container_of(vdev, struct virtio_pci_device, vdev);
}
-/* disable irq handlers */
-void vp_disable_cbs(struct virtio_device *vdev);
-/* enable irq handlers */
-void vp_enable_cbs(struct virtio_device *vdev);
+/* wait for pending irq handlers */
+void vp_synchronize_vectors(struct virtio_device *vdev);
/* the notify function used when creating a virt queue */
bool vp_notify(struct virtqueue *vq);
/* the config->del_vqs() implementation */
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 34141b9abe27..6f4e34ce96b8 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -98,8 +98,8 @@ static void vp_reset(struct virtio_device *vdev)
/* Flush out the status write, and flush in device writes,
* including MSi-X interrupts, if any. */
vp_legacy_get_status(&vp_dev->ldev);
- /* Disable VQ/configuration callbacks. */
- vp_disable_cbs(vdev);
+ /* Flush pending VQ/configuration callbacks. */
+ vp_synchronize_vectors(vdev);
}
static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
@@ -185,7 +185,6 @@ static void del_vq(struct virtio_pci_vq_info *info)
}
static const struct virtio_config_ops virtio_pci_config_ops = {
- .enable_cbs = vp_enable_cbs,
.get = vp_get,
.set = vp_set,
.get_status = vp_get_status,
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 5455bc041fb6..30654d3a0b41 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -172,8 +172,8 @@ static void vp_reset(struct virtio_device *vdev)
*/
while (vp_modern_get_status(mdev))
msleep(1);
- /* Disable VQ/configuration callbacks. */
- vp_disable_cbs(vdev);
+ /* Flush pending VQ/configuration callbacks. */
+ vp_synchronize_vectors(vdev);
}
static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
@@ -380,7 +380,6 @@ static bool vp_get_shm_region(struct virtio_device *vdev,
}
static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
- .enable_cbs = vp_enable_cbs,
.get = NULL,
.set = NULL,
.generation = vp_generation,
@@ -398,7 +397,6 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
};
static const struct virtio_config_ops virtio_pci_config_ops = {
- .enable_cbs = vp_enable_cbs,
.get = vp_get,
.set = vp_set,
.generation = vp_generation,
--
2.25.1
next prev parent reply other threads:[~2022-03-23 3:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-23 3:15 [PATCH 1/2] Revert "virtio-pci: harden INTX interrupts" Jason Wang
2022-03-23 3:15 ` Jason Wang [this message]
2022-03-23 9:03 ` [PATCH 2/2] Revert "virtio_pci: harden MSI-X interrupts" Marc Zyngier
2022-03-23 9:21 ` Jason Wang
2022-03-23 9:05 ` Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220323031524.6555-2-jasowang@redhat.com \
--to=jasowang@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=mst@redhat.com \
--cc=peterz@infradead.org \
--cc=sgarzare@redhat.com \
--cc=tglx@linutronix.de \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox