linux-kernel - Re: [PATCH v2 6/7] vduse: add vq group asid support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACGkMEsUXQEVZK52yEnD0rUumb4Sqeia=JGSjP0KnKX3=d0=1A@mail.gmail.com>
Date: Fri, 19 Sep 2025 10:15:14 +0800
From: Jason Wang <jasowang@...hat.com>
To: Eugenio Perez Martin <eperezma@...hat.com>
Cc: "Michael S . Tsirkin" <mst@...hat.com>, Stefano Garzarella <sgarzare@...hat.com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, linux-kernel@...r.kernel.org, 
	Maxime Coquelin <mcoqueli@...hat.com>, Yongji Xie <xieyongji@...edance.com>, 
	Cindy Lu <lulu@...hat.com>, Laurent Vivier <lvivier@...hat.com>, virtualization@...ts.linux.dev
Subject: Re: [PATCH v2 6/7] vduse: add vq group asid support

On Thu, Sep 18, 2025 at 2:58 PM Eugenio Perez Martin
<eperezma@...hat.com> wrote:
>
> On Thu, Sep 18, 2025 at 8:07 AM Jason Wang <jasowang@...hat.com> wrote:
> >
> > On Thu, Sep 18, 2025 at 12:41 AM Eugenio Perez Martin
> > <eperezma@...hat.com> wrote:
> > >
> > > On Wed, Sep 17, 2025 at 10:56 AM Jason Wang <jasowang@...hat.com> wrote:
> > > >
> > > > On Tue, Sep 16, 2025 at 9:09 PM Eugenio Pérez <eperezma@...hat.com> wrote:
> > > > >
> > > > > Add support for assigning Address Space Identifiers (ASIDs) to each VQ
> > > > > group.  This enables mapping each group into a distinct memory space.
> > > > >
> > > > > Now that the driver can change ASID in the middle of operation, the
> > > > > domain that each vq address point is also protected by domain_lock.
> > > > >
> > > > > Signed-off-by: Eugenio Pérez <eperezma@...hat.com>
> > > > > ---
> > > > > v2:
> > > > > * Convert the use of mutex to rwlock.
> > > > >
> > > > > 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 TODO about merging VDUSE_IOTLB_GET_FD ioctl with
> > > > >   VDUSE_IOTLB_GET_INFO.
> > > > > * Use of array_index_nospec in VDUSE device ioctls.
> > > > > * Embed vduse_iotlb_entry into vduse_iotlb_entry_v2.
> > > > > * Move the umem mutex to asid struct so there is no contention between
> > > > >   ASIDs.
> > > > >
> > > > > RFC v2:
> > > > > * Make iotlb entry the last one of vduse_iotlb_entry_v2 so the first
> > > > >   part of the struct is the same.
> > > > > ---
> > > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 324 +++++++++++++++++++++--------
> > > > >  include/uapi/linux/vduse.h         |  51 ++++-
> > > > >  2 files changed, 284 insertions(+), 91 deletions(-)
> > > > >
> > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > index b45b1d22784f..06b7790380b7 100644
> > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > @@ -93,6 +93,7 @@ struct vduse_as {
> > > > >  };
> > > > >
> > > > >  struct vduse_vq_group_int {
> > > > > +       struct vduse_iova_domain *domain;
> > > > >         struct vduse_dev *dev;
> > > >
> > > > This confuses me, I think it should be an asid. And the vduse_dev
> > > > pointer seems to be useless here.
> > > >
> > >
> > > The *dev pointer is used to take the rwlock, in case the vhost driver
> > > calls VHOST_VDPA_SET_GROUP_ASID (or equivalent) at the same time
> > > vduse_dev_sync_single_for_device (or _for_cpu, or equivalent) run.
> >
> > Ok, but having it in a group seems odd. A better place is to move it to the as?
> >
>
> If we move to the as, we need to take the lock this way:
>
> read_lock(vduse_vq_group->as->dev->domain_lock)
>
> But the lock is protecting the vduse_vq_group->as change itself, as
> other ioctls (SET_GROUP_ASID mainly) can modify it concurrently.
>
> We can assume that *as will always point to a valid as, its change
> will be atomic, and all the as->dev points to the exact same
> vduse_dev. I can buy that, but it is a race anyway.
>

I see, let's keep the struct vduse_dev *dev pointer as is.

Thanks