[PATCH 4/6] iommu/virtio: stop queueing and draining once the device is removed

[Anlai Lu]

viommu_remove() resets the device and deletes the virtqueues while
userspace may still hold domains that it maps or attaches, and nothing
stopped a request from being queued - or a drain from walking the queue -
on a virtqueue that was being torn down.

Set ->removed under request_lock before the teardown - the lock a
queueing path holds while it checks liveness and adds the request, and
the one the drain holds for its whole run - check it in the queueing
helper, and let the drain return early when it is set.

Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
Signed-off-by: Anlai Lu <agicy@xxxxxx>
---
 drivers/iommu/virtio-iommu.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 1a4366458751..ba1d78e4daa0 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -42,6 +42,11 @@ struct viommu_dev {
 	spinlock_t			request_lock;
 	struct list_head		requests;
 	void				*evts;
+	/*
+	 * Set before the teardown: nothing may be queued or drained after
+	 * that.
+	 */
+	bool				removed;
 
 	/* Device configuration */
 	struct iommu_domain_geometry	geometry;
@@ -116,6 +121,16 @@ static struct viommu_domain viommu_identity_domain;
 #define to_viommu_domain(domain)	\
 	container_of(domain, struct viommu_domain, domain)
 
+/*
+ * The device can be removed while its domains still exist (userspace may hold
+ * them for a while), and the queues go away with it: nothing may be queued or
+ * drained after that.
+ */
+static bool viommu_device_live(struct viommu_dev *viommu)
+{
+	return !viommu->removed;
+}
+
 static int viommu_get_req_errno(void *buf, size_t len)
 {
 	struct virtio_iommu_req_tail *tail = buf + len - sizeof(*tail);
@@ -206,6 +221,10 @@ static int viommu_sync_req(struct viommu_dev *viommu)
 	unsigned long flags;
 
 	spin_lock_irqsave(&viommu->request_lock, flags);
+	if (!viommu_device_live(viommu)) {
+		spin_unlock_irqrestore(&viommu->request_lock, flags);
+		return 0;
+	}
 	ret = __viommu_sync_req(viommu);
 	if (ret)
 		dev_dbg(viommu->dev, "could not sync requests (%d)\n", ret);
@@ -229,6 +248,9 @@ static int __viommu_queue_req(struct viommu_dev *viommu,
 
 	assert_spin_locked(&viommu->request_lock);
 
+	if (!viommu_device_live(viommu))
+		return -ENODEV;
+
 	sg_init_one(&top_sg, req->buf, write_offset);
 	sg_init_one(&bottom_sg, req->buf + write_offset,
 		    req->len - write_offset);
@@ -1555,10 +1577,20 @@ static int viommu_probe(struct virtio_device *vdev)
 static void viommu_remove(struct virtio_device *vdev)
 {
 	struct viommu_dev *viommu = vdev->priv;
+	unsigned long flags;
 
 	iommu_device_sysfs_remove(&viommu->iommu);
 	iommu_device_unregister(&viommu->iommu);
 
+	/*
+	 * The queues go away here: nothing may be queued or drained from now
+	 * on.  Taking request_lock is what tells a drain in flight that it has
+	 * to finish before the teardown, since the drain holds it throughout.
+	 */
+	spin_lock_irqsave(&viommu->request_lock, flags);
+	viommu->removed = true;
+	spin_unlock_irqrestore(&viommu->request_lock, flags);
+
 	/* Stop all virtqueues */
 	virtio_reset_device(vdev);
 	vdev->config->del_vqs(vdev);
-- 
2.55.0