On Thu, Sep 18, 2025 at 2:58 PM Eugenio Perez Martin <[email protected]> wrote: > > On Thu, Sep 18, 2025 at 8:07 AM Jason Wang <[email protected]> wrote: > > > > On Thu, Sep 18, 2025 at 12:41 AM Eugenio Perez Martin > > <[email protected]> wrote: > > > > > > On Wed, Sep 17, 2025 at 10:56 AM Jason Wang <[email protected]> wrote: > > > > > > > > On Tue, Sep 16, 2025 at 9:09 PM Eugenio Pérez <[email protected]> > > > > 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 <[email protected]> > > > > > --- > > > > > 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
