[PATCH v11 10/12] vduse: merge tree search logic of IOTLB_GET_FD and IOTLB_GET_INFO ioctls

Eugenio Pérez posted 12 patches 4 weeks, 1 day ago
There is a newer version of this series
[PATCH v11 10/12] vduse: merge tree search logic of IOTLB_GET_FD and IOTLB_GET_INFO ioctls
Posted by Eugenio Pérez 4 weeks, 1 day ago
The next patch adds new ioctl with the ASID member per entry.  Abstract
these two so it can be build on top easily.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
v11: New in v11
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 101 ++++++++++++++++-------------
 1 file changed, 55 insertions(+), 46 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 675da1465e0e..bf437816fd7d 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1247,6 +1247,50 @@ static void vduse_vq_update_effective_cpu(struct vduse_virtqueue *vq)
 	vq->irq_effective_cpu = curr_cpu;
 }
 
+static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
+				 struct vduse_iotlb_entry *entry,
+				 struct file **f, uint64_t *capability)
+{
+	int r = -EINVAL;
+	struct vhost_iotlb_map *map;
+	const struct vdpa_map_file *map_file;
+
+	if (entry->start > entry->last)
+		return -EINVAL;
+
+	mutex_lock(&dev->domain_lock);
+	if (!dev->domain)
+		goto out;
+
+	spin_lock(&dev->domain->iotlb_lock);
+	map = vhost_iotlb_itree_first(dev->domain->iotlb, entry->start,
+				      entry->last);
+	if (map) {
+		if (f) {
+			map_file = (struct vdpa_map_file *)map->opaque;
+			*f = get_file(map_file->file);
+		}
+		entry->offset = map_file->offset;
+		entry->start = map->start;
+		entry->last = map->last;
+		entry->perm = map->perm;
+		if (capability) {
+			*capability = 0;
+
+			if (dev->domain->bounce_map && map->start == 0 &&
+			    map->last == dev->domain->bounce_size - 1)
+				*capability |= VDUSE_IOVA_CAP_UMEM;
+		}
+
+		r = 0;
+	}
+	spin_unlock(&dev->domain->iotlb_lock);
+
+out:
+	mutex_unlock(&dev->domain_lock);
+	return r;
+}
+
 static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 			    unsigned long arg)
 {
@@ -1260,36 +1304,16 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 	switch (cmd) {
 	case VDUSE_IOTLB_GET_FD: {
 		struct vduse_iotlb_entry entry;
-		struct vhost_iotlb_map *map;
-		struct vdpa_map_file *map_file;
 		struct file *f = NULL;
 
 		ret = -EFAULT;
 		if (copy_from_user(&entry, argp, sizeof(entry)))
 			break;
 
-		ret = -EINVAL;
-		if (entry.start > entry.last)
+		ret = vduse_dev_iotlb_entry(dev, &entry, &f, NULL);
+		if (ret)
 			break;
 
-		mutex_lock(&dev->domain_lock);
-		if (!dev->domain) {
-			mutex_unlock(&dev->domain_lock);
-			break;
-		}
-		spin_lock(&dev->domain->iotlb_lock);
-		map = vhost_iotlb_itree_first(dev->domain->iotlb,
-					      entry.start, entry.last);
-		if (map) {
-			map_file = (struct vdpa_map_file *)map->opaque;
-			f = get_file(map_file->file);
-			entry.offset = map_file->offset;
-			entry.start = map->start;
-			entry.last = map->last;
-			entry.perm = map->perm;
-		}
-		spin_unlock(&dev->domain->iotlb_lock);
-		mutex_unlock(&dev->domain_lock);
 		ret = -EINVAL;
 		if (!f)
 			break;
@@ -1479,41 +1503,26 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 	}
 	case VDUSE_IOTLB_GET_INFO: {
 		struct vduse_iova_info info;
-		struct vhost_iotlb_map *map;
+		struct vduse_iotlb_entry entry;
 
 		ret = -EFAULT;
 		if (copy_from_user(&info, argp, sizeof(info)))
 			break;
 
-		ret = -EINVAL;
-		if (info.start > info.last)
-			break;
-
 		if (!is_mem_zero((const char *)info.reserved,
 				 sizeof(info.reserved)))
 			break;
 
-		mutex_lock(&dev->domain_lock);
-		if (!dev->domain) {
-			mutex_unlock(&dev->domain_lock);
-			break;
-		}
-		spin_lock(&dev->domain->iotlb_lock);
-		map = vhost_iotlb_itree_first(dev->domain->iotlb,
-					      info.start, info.last);
-		if (map) {
-			info.start = map->start;
-			info.last = map->last;
-			info.capability = 0;
-			if (dev->domain->bounce_map && map->start == 0 &&
-			    map->last == dev->domain->bounce_size - 1)
-				info.capability |= VDUSE_IOVA_CAP_UMEM;
-		}
-		spin_unlock(&dev->domain->iotlb_lock);
-		mutex_unlock(&dev->domain_lock);
-		if (!map)
+		entry.start = info.start;
+		entry.last = info.last;
+		ret = vduse_dev_iotlb_entry(dev, &entry, NULL,
+					    &info.capability);
+		if (ret < 0)
 			break;
 
+		info.start = entry.start;
+		info.last = entry.last;
+
 		ret = -EFAULT;
 		if (copy_to_user(argp, &info, sizeof(info)))
 			break;
-- 
2.52.0

Re: [PATCH v11 10/12] vduse: merge tree search logic of IOTLB_GET_FD and IOTLB_GET_INFO ioctls
Posted by kernel test robot 3 weeks, 2 days ago
Hi Eugenio,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.19-rc5 next-20260115]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Eugenio-P-rez/vduse-add-v1-API-definition/20260109-233435
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:    https://lore.kernel.org/r/20260109152430.512923-11-eperezma%40redhat.com
patch subject: [PATCH v11 10/12] vduse: merge tree search logic of IOTLB_GET_FD and IOTLB_GET_INFO ioctls
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20260116/202601161430.lDfgRE4U-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260116/202601161430.lDfgRE4U-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601161430.lDfgRE4U-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/vdpa/vdpa_user/vduse_dev.c:1269:7: warning: variable 'map_file' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
    1269 |                 if (f) {
         |                     ^
   drivers/vdpa/vdpa_user/vduse_dev.c:1273:19: note: uninitialized use occurs here
    1273 |                 entry->offset = map_file->offset;
         |                                 ^~~~~~~~
   drivers/vdpa/vdpa_user/vduse_dev.c:1269:3: note: remove the 'if' if its condition is always true
    1269 |                 if (f) {
         |                 ^~~~~~
   drivers/vdpa/vdpa_user/vduse_dev.c:1256:38: note: initialize the variable 'map_file' to silence this warning
    1256 |         const struct vdpa_map_file *map_file;
         |                                             ^
         |                                              = NULL
   1 warning generated.


vim +1269 drivers/vdpa/vdpa_user/vduse_dev.c

  1249	
  1250	static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
  1251					 struct vduse_iotlb_entry *entry,
  1252					 struct file **f, uint64_t *capability)
  1253	{
  1254		int r = -EINVAL;
  1255		struct vhost_iotlb_map *map;
  1256		const struct vdpa_map_file *map_file;
  1257	
  1258		if (entry->start > entry->last)
  1259			return -EINVAL;
  1260	
  1261		mutex_lock(&dev->domain_lock);
  1262		if (!dev->domain)
  1263			goto out;
  1264	
  1265		spin_lock(&dev->domain->iotlb_lock);
  1266		map = vhost_iotlb_itree_first(dev->domain->iotlb, entry->start,
  1267					      entry->last);
  1268		if (map) {
> 1269			if (f) {
  1270				map_file = (struct vdpa_map_file *)map->opaque;
  1271				*f = get_file(map_file->file);
  1272			}
  1273			entry->offset = map_file->offset;
  1274			entry->start = map->start;
  1275			entry->last = map->last;
  1276			entry->perm = map->perm;
  1277			if (capability) {
  1278				*capability = 0;
  1279	
  1280				if (dev->domain->bounce_map && map->start == 0 &&
  1281				    map->last == dev->domain->bounce_size - 1)
  1282					*capability |= VDUSE_IOVA_CAP_UMEM;
  1283			}
  1284	
  1285			r = 0;
  1286		}
  1287		spin_unlock(&dev->domain->iotlb_lock);
  1288	
  1289	out:
  1290		mutex_unlock(&dev->domain_lock);
  1291		return r;
  1292	}
  1293	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v11 10/12] vduse: merge tree search logic of IOTLB_GET_FD and IOTLB_GET_INFO ioctls
Posted by Michael S. Tsirkin 4 weeks ago
On Fri, Jan 09, 2026 at 04:24:28PM +0100, Eugenio Pérez wrote:
> The next patch adds new ioctl with the ASID member per entry.  Abstract
> these two so it can be build on top easily.
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
> v11: New in v11
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 101 ++++++++++++++++-------------
>  1 file changed, 55 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 675da1465e0e..bf437816fd7d 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1247,6 +1247,50 @@ static void vduse_vq_update_effective_cpu(struct vduse_virtqueue *vq)
>  	vq->irq_effective_cpu = curr_cpu;
>  }
>  
> +static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
> +				 struct vduse_iotlb_entry *entry,
> +				 struct file **f, uint64_t *capability)
> +{
> +	int r = -EINVAL;
> +	struct vhost_iotlb_map *map;
> +	const struct vdpa_map_file *map_file;
> +
> +	if (entry->start > entry->last)
> +		return -EINVAL;
> +
> +	mutex_lock(&dev->domain_lock);
> +	if (!dev->domain)
> +		goto out;
> +
> +	spin_lock(&dev->domain->iotlb_lock);
> +	map = vhost_iotlb_itree_first(dev->domain->iotlb, entry->start,
> +				      entry->last);
> +	if (map) {
> +		if (f) {
> +			map_file = (struct vdpa_map_file *)map->opaque;

map_file assigned value when f != NULL here ...

> +			*f = get_file(map_file->file);
> +		}
> +		entry->offset = map_file->offset;

but dereferenced unconditionally here.

> +		entry->start = map->start;
> +		entry->last = map->last;
> +		entry->perm = map->perm;
> +		if (capability) {
> +			*capability = 0;
> +
> +			if (dev->domain->bounce_map && map->start == 0 &&
> +			    map->last == dev->domain->bounce_size - 1)
> +				*capability |= VDUSE_IOVA_CAP_UMEM;
> +		}
> +
> +		r = 0;
> +	}
> +	spin_unlock(&dev->domain->iotlb_lock);
> +
> +out:
> +	mutex_unlock(&dev->domain_lock);
> +	return r;
> +}
> +
>  static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>  			    unsigned long arg)
>  {
Re: [PATCH v11 10/12] vduse: merge tree search logic of IOTLB_GET_FD and IOTLB_GET_INFO ioctls
Posted by Eugenio Perez Martin 3 weeks, 6 days ago
On Sun, Jan 11, 2026 at 12:56 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Jan 09, 2026 at 04:24:28PM +0100, Eugenio Pérez wrote:
> > The next patch adds new ioctl with the ASID member per entry.  Abstract
> > these two so it can be build on top easily.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > v11: New in v11
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 101 ++++++++++++++++-------------
> >  1 file changed, 55 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 675da1465e0e..bf437816fd7d 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1247,6 +1247,50 @@ static void vduse_vq_update_effective_cpu(struct vduse_virtqueue *vq)
> >       vq->irq_effective_cpu = curr_cpu;
> >  }
> >
> > +static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
> > +                              struct vduse_iotlb_entry *entry,
> > +                              struct file **f, uint64_t *capability)
> > +{
> > +     int r = -EINVAL;
> > +     struct vhost_iotlb_map *map;
> > +     const struct vdpa_map_file *map_file;
> > +
> > +     if (entry->start > entry->last)
> > +             return -EINVAL;
> > +
> > +     mutex_lock(&dev->domain_lock);
> > +     if (!dev->domain)
> > +             goto out;
> > +
> > +     spin_lock(&dev->domain->iotlb_lock);
> > +     map = vhost_iotlb_itree_first(dev->domain->iotlb, entry->start,
> > +                                   entry->last);
> > +     if (map) {
> > +             if (f) {
> > +                     map_file = (struct vdpa_map_file *)map->opaque;
>
> map_file assigned value when f != NULL here ...
>
> > +                     *f = get_file(map_file->file);
> > +             }
> > +             entry->offset = map_file->offset;
>
> but dereferenced unconditionally here.
>

Fixing in the next version, thanks!

> > +             entry->start = map->start;
> > +             entry->last = map->last;
> > +             entry->perm = map->perm;
> > +             if (capability) {
> > +                     *capability = 0;
> > +
> > +                     if (dev->domain->bounce_map && map->start == 0 &&
> > +                         map->last == dev->domain->bounce_size - 1)
> > +                             *capability |= VDUSE_IOVA_CAP_UMEM;
> > +             }
> > +
> > +             r = 0;
> > +     }
> > +     spin_unlock(&dev->domain->iotlb_lock);
> > +
> > +out:
> > +     mutex_unlock(&dev->domain_lock);
> > +     return r;
> > +}
> > +
> >  static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >                           unsigned long arg)
> >  {
>