From: Jason Wang <jasowang@redhat.com>
To: mst@redhat.com, jasowang@redhat.com
Cc: virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org, maz@kernel.org, tglx@linutronix.de,
peterz@infradead.org, sgarzare@redhat.com,
"Paul E. McKenney" <paulmck@kernel.org>
Subject: [PATCH V2 5/5] virtio: harden vring IRQ
Date: Wed, 6 Apr 2022 16:35:38 +0800 [thread overview]
Message-ID: <20220406083538.16274-6-jasowang@redhat.com> (raw)
In-Reply-To: <20220406083538.16274-1-jasowang@redhat.com>
This is a rework on the previous IRQ hardening that is done for
virtio-pci where several drawbacks were found and were reverted:
1) try to use IRQF_NO_AUTOEN which is not friendly to affinity managed IRQ
that is used by some device such as virtio-blk
2) done only for PCI transport
In this patch, we tries to borrow the idea from the INTX IRQ hardening
in the reverted commit 080cd7c3ac87 ("virtio-pci: harden INTX interrupts")
by introducing a global device_ready variable for each
virtio_device. Then we can to toggle it during
virtio_reset_device()/virtio_device_ready(). A
virtio_synchornize_vqs() is used in both virtio_device_ready() and
virtio_reset_device() to synchronize with the vring callbacks. With
this, vring_interrupt() can return check and early if driver_ready is
false.
Note that the hardening is only done for vring interrupt since the
config interrupt hardening is already done in commit 22b7050a024d7
("virtio: defer config changed notifications"). But the method that is
used by config interrupt can't be reused by the vring interrupt
handler because it uses spinlock to do the synchronization which is
expensive.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio.c | 11 +++++++++++
drivers/virtio/virtio_ring.c | 9 ++++++++-
include/linux/virtio.h | 2 ++
include/linux/virtio_config.h | 8 ++++++++
4 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 8dde44ea044a..2f3a6f8e3d9c 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -220,6 +220,17 @@ static int virtio_features_ok(struct virtio_device *dev)
* */
void virtio_reset_device(struct virtio_device *dev)
{
+ if (READ_ONCE(dev->driver_ready)) {
+ /*
+ * The below virtio_synchronize_vqs() guarantees that any
+ * interrupt for this line arriving after
+ * virtio_synchronize_vqs() has completed is guaranteed to see
+ * driver_ready == false.
+ */
+ WRITE_ONCE(dev->driver_ready, false);
+ virtio_synchronize_vqs(dev);
+ }
+
dev->config->reset(dev);
}
EXPORT_SYMBOL_GPL(virtio_reset_device);
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cfb028ca238e..a4592e55c9f8 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2127,10 +2127,17 @@ static inline bool more_used(const struct vring_virtqueue *vq)
return vq->packed_ring ? more_used_packed(vq) : more_used_split(vq);
}
-irqreturn_t vring_interrupt(int irq, void *_vq)
+irqreturn_t vring_interrupt(int irq, void *v)
{
+ struct virtqueue *_vq = v;
+ struct virtio_device *vdev = _vq->vdev;
struct vring_virtqueue *vq = to_vvq(_vq);
+ if (!READ_ONCE(vdev->driver_ready)) {
+ dev_warn_once(&vdev->dev, "virtio vring IRQ raised before DRIVER_OK");
+ return IRQ_NONE;
+ }
+
if (!more_used(vq)) {
pr_debug("virtqueue interrupt with no work for %p\n", vq);
return IRQ_NONE;
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 5464f398912a..dfa2638a293e 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -95,6 +95,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
* @failed: saved value for VIRTIO_CONFIG_S_FAILED bit (for restore)
* @config_enabled: configuration change reporting enabled
* @config_change_pending: configuration change reported while disabled
+ * @driver_ready: whehter the driver is ready (e.g for vring callbacks)
* @config_lock: protects configuration change reporting
* @dev: underlying device.
* @id: the device type identification (used to match it with a driver).
@@ -109,6 +110,7 @@ struct virtio_device {
bool failed;
bool config_enabled;
bool config_change_pending;
+ bool driver_ready;
spinlock_t config_lock;
spinlock_t vqs_list_lock; /* Protects VQs list access */
struct device dev;
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 08b73d9bbff2..c9e207bf2c9c 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -246,6 +246,14 @@ void virtio_device_ready(struct virtio_device *dev)
{
unsigned status = dev->config->get_status(dev);
+ virtio_synchronize_vqs(dev);
+ /*
+ * The above virtio_synchronize_vqs() make sure
+ * vring_interrupt() will see the driver specific setup if it
+ * see driver_ready as true.
+ */
+ WRITE_ONCE(dev->driver_ready, true);
+
BUG_ON(status & VIRTIO_CONFIG_S_DRIVER_OK);
dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
}
--
2.25.1
next prev parent reply other threads:[~2022-04-06 12:35 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-06 8:35 [PATCH V2 0/5] rework on the IRQ hardening of virtio Jason Wang
2022-04-06 8:35 ` [PATCH V2 1/5] virtio: use virtio_device_ready() in virtio_device_restore() Jason Wang
2022-04-06 11:44 ` Michael S. Tsirkin
2022-04-07 6:19 ` Jason Wang
2022-04-06 8:35 ` [PATCH V2 2/5] virtio: use virtio_reset_device() when possible Jason Wang
2022-04-06 11:53 ` Michael S. Tsirkin
2022-04-06 8:35 ` [PATCH V2 3/5] virtio: introduce config op to synchronize vring callbacks Jason Wang
2022-04-06 11:59 ` Michael S. Tsirkin
2022-04-07 6:25 ` Jason Wang
2022-04-06 8:35 ` [PATCH V2 4/5] virtio-pci: implement synchronize_vqs() Jason Wang
2022-04-06 12:00 ` Michael S. Tsirkin
2022-04-06 13:04 ` Cornelia Huck
2022-04-06 15:31 ` Michael S. Tsirkin
2022-04-07 6:38 ` Jason Wang
2022-04-07 7:52 ` Cornelia Huck
2022-04-07 8:04 ` Jason Wang
2022-04-08 13:03 ` Halil Pasic
2022-04-10 7:51 ` Michael S. Tsirkin
2022-04-11 8:22 ` Jason Wang
2022-04-11 8:56 ` Michael S. Tsirkin
2022-04-12 2:21 ` Jason Wang
2022-04-11 14:27 ` Cornelia Huck
2022-04-12 0:01 ` Halil Pasic
2022-04-12 2:24 ` Jason Wang
2022-04-12 7:55 ` Halil Pasic
2022-04-12 16:48 ` Cornelia Huck
2022-04-13 2:53 ` Jason Wang
2022-04-13 6:41 ` Cornelia Huck
2022-04-06 8:35 ` Jason Wang [this message]
2022-04-06 12:04 ` [PATCH V2 5/5] virtio: harden vring IRQ Michael S. Tsirkin
2022-04-07 6:39 ` Jason Wang
2022-04-06 11:36 ` [PATCH V2 0/5] rework on the IRQ hardening of virtio Michael S. Tsirkin
2022-04-07 6:12 ` Jason Wang
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=20220406083538.16274-6-jasowang@redhat.com \
--to=jasowang@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=mst@redhat.com \
--cc=paulmck@kernel.org \
--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
all inboxes | Powered by JetHome®