RE: [PATCH 09/13] vfio: Add infrastructure for bind_iommufd and attach

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Tian, Kevin <kevin.tian@xxxxxxxxx>
> Sent: Thursday, January 19, 2023 5:45 PM
> 
> > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > Sent: Tuesday, January 17, 2023 9:50 PM
> >  static int vfio_device_group_open(struct vfio_device_file *df)
> >  {
> >  	struct vfio_device *device = df->device;
> > +	u32 ioas_id;
> > +	u32 *pt_id = NULL;
> >  	int ret;
> >
> >  	mutex_lock(&device->group->group_lock);
> > @@ -165,6 +167,14 @@ static int vfio_device_group_open(struct
> > vfio_device_file *df)
> >  		goto err_unlock_group;
> >  	}
> >
> > +	if (device->group->iommufd) {
> > +		ret = iommufd_vfio_compat_ioas_id(device->group-
> > >iommufd,
> > +						  &ioas_id);
> > +		if (ret)
> > +			goto err_unlock_group;
> > +		pt_id = &ioas_id;
> > +	}
> > +
> >  	mutex_lock(&device->dev_set->lock);
> >  	/*
> >  	 * Here we pass the KVM pointer with the group under the lock.  If
> > the
> > @@ -174,7 +184,7 @@ static int vfio_device_group_open(struct
> > vfio_device_file *df)
> >  	df->kvm = device->group->kvm;
> >  	df->iommufd = device->group->iommufd;
> >
> > -	ret = vfio_device_open(df);
> > +	ret = vfio_device_open(df, NULL, pt_id);
> 
> having both ioas_id and pt_id in one function is a bit confusing.
> 
> Does it read better with below?
> 
> if (device->group->iommufd)
> 	ret = vfio_device_open(df, NULL, &ioas_id);
> else
> 	ret = vfio_device_open(df, NULL, NULL);

Yes. 😊

> > +/* @pt_id == NULL implies detach */
> > +int vfio_iommufd_attach(struct vfio_device *vdev, u32 *pt_id)
> > +{
> > +	lockdep_assert_held(&vdev->dev_set->lock);
> > +
> > +	return vdev->ops->attach_ioas(vdev, pt_id);
> > +}
> 
> what benefit does this one-line wrapper give actually?
> 
> especially pt_id==NULL is checked in the callback instead of in this
> wrapper.

Yep, will just open code in the caller.

Regards,
Yi Liu




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux