RE: [PATCH v6 21/24] vfio: Add VFIO_DEVICE_BIND_IOMMUFD

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

 



> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Sent: Wednesday, March 15, 2023 12:40 PM
> 
> > From: Tian, Kevin <kevin.tian@xxxxxxxxx>
> > Sent: Friday, March 10, 2023 6:07 PM
> >
> > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > > Sent: Friday, March 10, 2023 5:58 PM
> > >
> > > > From: Tian, Kevin <kevin.tian@xxxxxxxxx>
> > > > Sent: Friday, March 10, 2023 5:02 PM
> > > >
> > > > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > > > > Sent: Wednesday, March 8, 2023 9:29 PM
> > > > > +
> > > > > +static int vfio_device_cdev_probe_noiommu(struct vfio_device
> > *device)
> > > > > +{
> > > > > +	struct iommu_group *iommu_group;
> > > > > +	int ret = 0;
> > > > > +
> > > > > +	if (!IS_ENABLED(CONFIG_VFIO_NOIOMMU)
> || !vfio_noiommu)
> > > > > +		return -EINVAL;
> > > > > +
> > > > > +	if (!capable(CAP_SYS_RAWIO))
> > > > > +		return -EPERM;
> > > > > +
> > > > > +	iommu_group = iommu_group_get(device->dev);
> > > > > +	if (!iommu_group)
> > > > > +		return 0;
> > > > > +
> > > > > +	/*
> > > > > +	 * We cannot support noiommu mode for devices that are
> > > > protected
> > > > > +	 * by IOMMU.  So check the iommu_group, if it is a no-iommu
> group
> > > > > +	 * created by VFIO, we support. If not, we refuse.
> > > > > +	 */
> > > > > +	if
> > > > (!vfio_group_find_noiommu_group_from_iommu(iommu_group))
> > > > > +		ret = -EINVAL;
> > > > > +	iommu_group_put(iommu_group);
> > > > > +	return ret;
> > > >
> > > > can check whether group->name == "vfio-noiommu"?
> > >
> > > But VFIO names it to be "vfio-noiommu" for both
> > VFIO_EMULATED_IOMMU
> > > and VFIO_NO_IOMMU. And we don't support no-iommu mode for
> > emulated
> > > devices since VFIO_MAP/UNMAP, pin_page(), dam_rw() won't work in
> > the
> > > no-iommu mode.
> >
> > correct.
> >
> > >
> > > So maybe something like below in drivers/vfio/vfio.h. It can be used
> > > to replace the code from iommu_group_get() to
> > > vfio_group_find_noiommu_group_from_iommu() In my patch.
> > >
> > > #if IS_ENABLED(CONFIG_VFIO_GROUP)
> > > static inline bool vfio_device_group_allow_noiommu(struct vfio_device
> > > *device)
> > > {
> > > 	lockdep_assert_held(&device->dev_set->lock);
> > >
> > > 	return device->group->type == VFIO_NO_IOMMU;
> > > }
> > > #else
> > > static inline bool vfio_device_group_allow_noiommu(struct vfio_device
> > > *device)
> > > {
> > > 	struct iommu_group *iommu_group;
> > >
> > > 	lockdep_assert_held(&device->dev_set->lock);
> > >
> > > 	iommu_group = iommu_group_get(device->dev);
> > > 	if (iommu_group)
> > > 		iommu_group_put(iommu_group);
> > >
> > > 	return !iommu_group;
> > > }
> > > #endif
> >
> > this makes sense.
> 
> Just have one more think. vfio_device_is_noiommu() is already able
> to cover above vfio_device_group_allow_noiommu(), just needs
> to make it work when !VFIO_GROUP. In the group code, group->type
> == VFIO_NO_IOMMU means vfio_noiommu==true. So no need to
> check it. While in the case !VFIO_GROUP, needs to check it. So the
> code is as below. I can use vfio_device_is_noiommu() in cdev path.
> 
> # if IS_ENABLED(CONFIG_VFIO_GROUP)
> static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
> {
>         return IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
>                vdev->group->type == VFIO_NO_IOMMU;
> }
> #else
> static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
> {
>         struct iommu_group *iommu_group;
> 
>         if (!IS_ENABLED(CONFIG_VFIO_NOIOMMU) || !vfio_noiommu)
>                 return -EINVAL;
> 
>         iommu_group = iommu_group_get(vdev->dev);
>         if (iommu_group)
>                 iommu_group_put(iommu_group);
> 
>         return !iommu_group;
> }
> #endif

works for me.




[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