On Wed, Sep 3, 2025 at 5:58 AM Jason Wang [off-list ref] wrote:

On Mon, Sep 1, 2025 at 4:40 PM Eugenio Perez Martin [off-list ref] wrote:

quoted

On Mon, Sep 1, 2025 at 3:59 AM Jason Wang [off-list ref] wrote:

quoted

On Tue, Aug 26, 2025 at 7:27 PM Eugenio Pérez [off-list ref] wrote:

quoted

This allows sepparate the different virtqueues in groups that shares the
same address space.  Asking the VDUSE device for the groups of the vq at
the beginning as they're needed for the DMA API.

Allocating 3 vq groups as net is the device that need the most groups:
* Dataplane (guest passthrough)
* CVQ
* Shadowed vrings.

Future versions of the series can include dynamic allocation of the
groups array so VDUSE can declare more groups.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
v1: Fix: Remove BIT_ULL(VIRTIO_S_*), as _S_ is already the bit (Maxime)

RFC v3:
* Increase VDUSE_MAX_VQ_GROUPS to 0xffff (Jason). It was set to a lower
  value to reduce memory consumption, but vqs are already limited to
  that value and userspace VDUSE is able to allocate that many vqs.
* Remove the descs vq group capability as it will not be used and we can
  add it on top.
* Do not ask for vq groups in number of vq groups < 2.
* Move the valid vq groups range check to vduse_validate_config.

RFC v2:
* Cache group information in kernel, as we need to provide the vq map
  tokens properly.
* Add descs vq group to optimize SVQ forwarding and support indirect
  descriptors out of the box.
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 51 ++++++++++++++++++++++++++++--
 include/uapi/linux/vduse.h         | 21 +++++++++++-
 2 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index e7bced0b5542..0f4e36dd167e 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c

@@ -58,6 +58,7 @@ struct vduse_virtqueue {
        struct vdpa_vq_state state;
        bool ready;
        bool kicked;
+       u32 vq_group;
        spinlock_t kick_lock;
        spinlock_t irq_lock;
        struct eventfd_ctx *kickfd;

@@ -114,6 +115,7 @@ struct vduse_dev {
        u8 status;
        u32 vq_num;
        u32 vq_align;
+       u32 ngroups;
        struct vduse_umem *umem;
        struct mutex mem_lock;
        unsigned int bounce_size;

@@ -592,6 +594,13 @@ static int vduse_vdpa_set_vq_state(struct vdpa_device *vdpa, u16 idx,
        return 0;
 }

+static u32 vduse_get_vq_group(struct vdpa_device *vdpa, u16 idx)
+{
+       struct vduse_dev *dev = vdpa_to_vduse(vdpa);
+
+       return dev->vqs[idx]->vq_group;
+}
+
 static int vduse_vdpa_get_vq_state(struct vdpa_device *vdpa, u16 idx,
                                struct vdpa_vq_state *state)
 {

@@ -678,6 +687,28 @@ static u8 vduse_vdpa_get_status(struct vdpa_device *vdpa)
        return dev->status;
 }

+static int vduse_fill_vq_groups(struct vduse_dev *dev)
+{
+       /* All vqs and descs must be in vq group 0 if ngroups < 2 */
+       if (dev->ngroups < 2)
+               return 0;
+
+       for (int i = 0; i < dev->vdev->vdpa.nvqs; ++i) {
+               struct vduse_dev_msg msg = { 0 };
+               int ret;
+
+               msg.req.type = VDUSE_GET_VQ_GROUP;
+               msg.req.vq_group.index = i;
+               ret = vduse_dev_msg_sync(dev, &msg);
+               if (ret)
+                       return ret;
+
+               dev->vqs[i]->vq_group = msg.resp.vq_group.group;
+       }
+
+       return 0;
+}
+
 static void vduse_vdpa_set_status(struct vdpa_device *vdpa, u8 status)
 {
        struct vduse_dev *dev = vdpa_to_vduse(vdpa);

@@ -685,6 +716,11 @@ static void vduse_vdpa_set_status(struct vdpa_device *vdpa, u8 status)
        if (vduse_dev_set_status(dev, status))
                return;

+       if (((dev->status ^ status) & VIRTIO_CONFIG_S_FEATURES_OK) &&
+           (status & VIRTIO_CONFIG_S_FEATURES_OK))
+               if (vduse_fill_vq_groups(dev))
+                       return;

I may lose some context but I think we've agreed that we need to
extend the status response for this instead of having multiple
indepdent response.

My understanding was it is ok to start with this version by [1]. We
can even make it asynchronous on top if we find this is a bottleneck
and the VDUSE device would need no change, would that work?

I think I need to understand why we can not defer this to get_group_asid() call.

Because we need to know the vq_groups->asid mapping in other calls
like set_group_asid or get_vq_group.

We could add a boolean on each virtqueue to track if we know its
virtqueue group and then only ask VDUSE device it if needed, would
that work?