On Thu, Sep 18, 2025 at 12:17 AM Eugenio Perez Martin
<epere...@redhat.com> wrote:
>
> On Wed, Sep 17, 2025 at 10:37 AM Jason Wang <jasow...@redhat.com> wrote:
> >
> > On Tue, Sep 16, 2025 at 9:09 PM Eugenio Pérez <epere...@redhat.com> wrote:
> > >
> > > Return the internal struct that represents the vq group as virtqueue map
> > > token, instead of the device. This allows the map functions to access
> > > the information per group.
> > >
> > > At this moment all the virtqueues share the same vq group, that only
> > > can point to ASID 0. This change prepares the infrastructure for actual
> > > per-group address space handling
> > >
> > > Signed-off-by: Eugenio Pérez <epere...@redhat.com>
> > > ---
> > > RFC v3:
> > > * Make the vq groups a dynamic array to support an arbitrary number of
> > > them.
> > > ---
> > > drivers/vdpa/vdpa_user/vduse_dev.c | 52 ++++++++++++++++++++++++------
> > > include/linux/virtio.h | 6 ++--
> > > 2 files changed, 46 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > index 42f8807911d4..9c12ae72abc2 100644
> > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > @@ -23,6 +23,7 @@
> > > #include <linux/uio.h>
> > > #include <linux/vdpa.h>
> > > #include <linux/nospec.h>
> > > +#include <linux/virtio.h>
> > > #include <linux/vmalloc.h>
> > > #include <linux/sched/mm.h>
> > > #include <uapi/linux/vduse.h>
> > > @@ -85,6 +86,10 @@ struct vduse_umem {
> > > struct mm_struct *mm;
> > > };
> > >
> > > +struct vduse_vq_group_int {
> > > + struct vduse_dev *dev;
> > > +};
> >
> > I remember we had some discussion over this, and the conclusion is to
> > have a better name.
> >
> > Maybe just vduse_vq_group?
> >
>
> Good catch, I also hate the _int suffix :). vduse_vq_group was used in
> the vduse uapi in previous series, but now there is no reason for it.
> Replacing it, thanks!
>
> > And to be conceptually correct, we need to use iova_domain here
> > instead of the vduse_dev. More below.
> >
> > > +
> > > struct vduse_dev {
> > > struct vduse_vdpa *vdev;
> > > struct device *dev;
> > > @@ -118,6 +123,7 @@ struct vduse_dev {
> > > u32 vq_align;
> > > u32 ngroups;
> > > struct vduse_umem *umem;
> > > + struct vduse_vq_group_int *groups;
> > > struct mutex mem_lock;
> > > unsigned int bounce_size;
> > > rwlock_t domain_lock;
> > > @@ -602,6 +608,15 @@ static u32 vduse_get_vq_group(struct vdpa_device
> > > *vdpa, u16 idx)
> > > return dev->vqs[idx]->vq_group;
> > > }
> > >
> > > +static union virtio_map vduse_get_vq_map(struct vdpa_device *vdpa, u16
> > > idx)
> > > +{
> > > + struct vduse_dev *dev = vdpa_to_vduse(vdpa);
> > > + u32 vq_group = dev->vqs[idx]->vq_group;
> > > + union virtio_map ret = { .group = &dev->groups[vq_group] };
> > > +
> > > + return ret;
> > > +}
> > > +
> > > static int vduse_vdpa_get_vq_state(struct vdpa_device *vdpa, u16 idx,
> > > struct vdpa_vq_state *state)
> > > {
> > > @@ -822,6 +837,7 @@ static const struct vdpa_config_ops
> > > vduse_vdpa_config_ops = {
> > > .get_vq_affinity = vduse_vdpa_get_vq_affinity,
> > > .reset = vduse_vdpa_reset,
> > > .set_map = vduse_vdpa_set_map,
> > > + .get_vq_map = vduse_get_vq_map,
> > > .free = vduse_vdpa_free,
> > > };
> > >
> > > @@ -829,7 +845,8 @@ static void vduse_dev_sync_single_for_device(union
> > > virtio_map token,
> > > dma_addr_t dma_addr, size_t
> > > size,
> > > enum dma_data_direction dir)
> > > {
> > > - struct vduse_iova_domain *domain = token.iova_domain;
> > > + struct vduse_dev *vdev = token.group->dev;
> > > + struct vduse_iova_domain *domain = vdev->domain;
> >
> > If we really want to do this, we need to move the iova_domian into the
> > group.
> >
>
> It's done in patches on top to make each patch smaller. This patch is
> focused on just changing the type of the union. Would you prefer me to
> reorder the patches so that part is done earlier?
I think it would be better for logical completeness.
Thanks
