virtio_pci_modern: use completion instead of busy loop to wait on admin cmd result

Currently, the code waits in a busy loop on every admin virtqueue issued
command to get a reply. That prevents callers from issuing multiple
commands in parallel.

To overcome this limitation, introduce a virtqueue event callback for
admin virtqueue. For every issued command, use completion mechanism
to wait on a reply. In the event callback, trigger the completion
is done for every incoming reply.

Alongside with that, introduce a spin lock to protect the admin
virtqueue operations.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Message-Id: <20240716113552.80599-13-jiri@resnulli.us>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 267643bb1cd5335397a7d81276cd4fbb45a7c22d..c44d8ba00c02c38f9d641b8ccf423efbf86ea83b 100644 (file)
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -395,6 +395,8 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
                        if (vqi->name && vqi->callback)
                                ++nvectors;
                }
+               if (avq_num && vector_policy == VP_VQ_VECTOR_POLICY_EACH)
+                       ++nvectors;
        } else {
                /* Second best: one for change, shared for all vqs. */
                nvectors = 2;
@@ -425,9 +427,9 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
        if (!avq_num)
                return 0;
        sprintf(avq->name, "avq.%u", avq->vq_index);
-       vq = vp_find_one_vq_msix(vdev, avq->vq_index, NULL, avq->name, false,
-                                true, &allocated_vectors, vector_policy,
-                                &vp_dev->admin_vq.info);
+       vq = vp_find_one_vq_msix(vdev, avq->vq_index, vp_modern_avq_done,
+                                avq->name, false, true, &allocated_vectors,
+                                vector_policy, &vp_dev->admin_vq.info);
        if (IS_ERR(vq)) {
                err = PTR_ERR(vq);
                goto error_find;
@@ -486,8 +488,9 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs,
        if (!avq_num)
                return 0;
        sprintf(avq->name, "avq.%u", avq->vq_index);
-       vq = vp_setup_vq(vdev, queue_idx++, NULL, avq->name, false,
-                        VIRTIO_MSI_NO_VECTOR, &vp_dev->admin_vq.info);
+       vq = vp_setup_vq(vdev, queue_idx++, vp_modern_avq_done, avq->name,
+                        false, VIRTIO_MSI_NO_VECTOR,
+                        &vp_dev->admin_vq.info);
        if (IS_ERR(vq)) {
                err = PTR_ERR(vq);
                goto out_del_vqs;

diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index de59bb06ec3c9e12ed54e73ed0c9e0d1a7eaacee..90df381fbbcfd64ed8fef0a5e5eb22d863591e5a 100644 (file)
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -47,6 +47,8 @@ struct virtio_pci_admin_vq {
        struct virtio_pci_vq_info *info;
        /* serializing admin commands execution. */
        struct mutex cmd_lock;
+       /* Protects virtqueue access. */
+       spinlock_t lock;
        u64 supported_cmds;
        /* Name of the admin queue: avq.$vq_index. */
        char name[10];
@@ -178,6 +180,7 @@ struct virtio_device *virtio_pci_vf_get_pf_dev(struct pci_dev *pdev);
 #define VIRTIO_ADMIN_CMD_BITMAP 0
 #endif
 
+void vp_modern_avq_done(struct virtqueue *vq);
 int vp_modern_admin_cmd_exec(struct virtio_device *vdev,
                             struct virtio_admin_cmd *cmd);
 

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 0fd344d1eaf924ae44360008a79d9ea52a16120a..608df3263df14fc365b7a7eda4739a2a743f8326 100644 (file)
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -53,6 +53,23 @@ static bool vp_is_avq(struct virtio_device *vdev, unsigned int index)
        return index == vp_dev->admin_vq.vq_index;
 }
 
+void vp_modern_avq_done(struct virtqueue *vq)
+{
+       struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
+       struct virtio_pci_admin_vq *admin_vq = &vp_dev->admin_vq;
+       struct virtio_admin_cmd *cmd;
+       unsigned long flags;
+       unsigned int len;
+
+       spin_lock_irqsave(&admin_vq->lock, flags);
+       do {
+               virtqueue_disable_cb(vq);
+               while ((cmd = virtqueue_get_buf(vq, &len)))
+                       complete(&cmd->completion);
+       } while (!virtqueue_enable_cb(vq));
+       spin_unlock_irqrestore(&admin_vq->lock, flags);
+}
+
 static int virtqueue_exec_admin_cmd(struct virtio_pci_admin_vq *admin_vq,
                                    u16 opcode,
                                    struct scatterlist **sgs,
@@ -61,7 +78,8 @@ static int virtqueue_exec_admin_cmd(struct virtio_pci_admin_vq *admin_vq,
                                    struct virtio_admin_cmd *cmd)
 {
        struct virtqueue *vq;
-       int ret, len;
+       unsigned long flags;
+       int ret;
 
        vq = admin_vq->info->vq;
        if (!vq)
@@ -72,21 +90,33 @@ static int virtqueue_exec_admin_cmd(struct virtio_pci_admin_vq *admin_vq,
            !((1ULL << opcode) & admin_vq->supported_cmds))
                return -EOPNOTSUPP;
 
-       ret = virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, GFP_KERNEL);
-       if (ret < 0)
-               return -EIO;
+       init_completion(&cmd->completion);
 
-       if (unlikely(!virtqueue_kick(vq)))
+again:
+       if (virtqueue_is_broken(vq))
                return -EIO;
 
-       while (!virtqueue_get_buf(vq, &len) &&
-              !virtqueue_is_broken(vq))
-               cpu_relax();
+       spin_lock_irqsave(&admin_vq->lock, flags);
+       ret = virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, GFP_KERNEL);
+       if (ret < 0) {
+               if (ret == -ENOSPC) {
+                       spin_unlock_irqrestore(&admin_vq->lock, flags);
+                       cpu_relax();
+                       goto again;
+               }
+               goto unlock_err;
+       }
+       if (!virtqueue_kick(vq))
+               goto unlock_err;
+       spin_unlock_irqrestore(&admin_vq->lock, flags);
 
-       if (virtqueue_is_broken(vq))
-               return -EIO;
+       wait_for_completion(&cmd->completion);
 
-       return 0;
+       return cmd->ret;
+
+unlock_err:
+       spin_unlock_irqrestore(&admin_vq->lock, flags);
+       return -EIO;
 }
 
 int vp_modern_admin_cmd_exec(struct virtio_device *vdev,
@@ -209,6 +239,25 @@ static void vp_modern_avq_activate(struct virtio_device *vdev)
        virtio_pci_admin_cmd_list_init(vdev);
 }
 
+static void vp_modern_avq_cleanup(struct virtio_device *vdev)
+{
+       struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+       struct virtio_admin_cmd *cmd;
+       struct virtqueue *vq;
+
+       if (!virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ))
+               return;
+
+       vq = vp_dev->vqs[vp_dev->admin_vq.vq_index]->vq;
+       if (!vq)
+               return;
+
+       while ((cmd = virtqueue_detach_unused_buf(vq))) {
+               cmd->ret = -EIO;
+               complete(&cmd->completion);
+       }
+}
+
 static void vp_transport_features(struct virtio_device *vdev, u64 features)
 {
        struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -403,6 +452,8 @@ static void vp_reset(struct virtio_device *vdev)
        while (vp_modern_get_status(mdev))
                msleep(1);
 
+       vp_modern_avq_cleanup(vdev);
+
        /* Flush pending VQ/configuration callbacks. */
        vp_synchronize_vectors(vdev);
 }
@@ -785,6 +836,7 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
        vp_dev->isr = mdev->isr;
        vp_dev->vdev.id = mdev->id;
 
+       spin_lock_init(&vp_dev->admin_vq.lock);
        mutex_init(&vp_dev->admin_vq.cmd_lock);
        return 0;
 }

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 96fea920873bffd896fdc21ec4660ade7717e1a0..999ff5934392fe5920fd1a17e5c2c31111d6d566 100644 (file)
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -10,6 +10,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/gfp.h>
 #include <linux/dma-mapping.h>
+#include <linux/completion.h>
 
 /**
  * struct virtqueue - a queue to register buffers for sending or receiving.
@@ -109,6 +110,8 @@ struct virtio_admin_cmd {
        __le64 group_member_id;
        struct scatterlist *data_sg;
        struct scatterlist *result_sg;
+       struct completion completion;
+       int ret;
 };
 
 /**