[Intel-gfx] [PATCH v4 2/9] vfio-iommufd: Create iommufd_access for noiommu devices

[Liu, Yi L]

> From: Liu, Yi L <yi.l.liu at intel.com>
> Sent: Wednesday, May 3, 2023 5:49 PM
> 
> > From: Jason Gunthorpe <jgg at nvidia.com>
> > Sent: Wednesday, May 3, 2023 2:12 AM
> >
> > On Sat, Apr 29, 2023 at 12:07:24AM +0800, Yi Liu wrote:
> > > > The emulated stuff is for mdev only, it should not be confused with
> > > > no-iommu
> > >
> > > hmmm. I guess the confusion is due to the reuse of
> > > vfio_iommufd_emulated_bind().
> >
> > This is probabl y not a good direction
> 
> I see. But if not reusing, then there may be a few code duplications.
> I'm fine to add separate _bind/unbind() functions for noiommu devices
> if Alex and you prefer it.
> 
> > > > Eg if you had a no_iommu_access value to store the access it would be
> > > > fine and could serve as the 'this is no_iommu' flag
> > >
> > > So this no_iommu_access shall be created per iommufd bind, and call the
> > > iommufd_access_create() with iommufd_access_ops. is it? If so, this is
> > > not 100% the same with no_iommu flag as this flag is static after device
> > > registration.
> >
> > Something like that, yes
> >
> > I don't think it is any real difference with the current flag, both
> > are determined at the first ioctl when the iommufd is presented and
> > both would state permanently until the fd close
> 
> Well, noiommu flag would be static from registration till unregistration.:-)
> While no_iommu_access's life circle is between the bind and fd close. But
> given that the major usage of it is during the duration between fd is bound
> to iommufd and closed, so it's still possible to let no_iommu_access serve
> as noiommu flag. 😊

Hi Jason,

I found another reason to use noiommu flag here.

Existing vfio will fail the vfio_device registration if there is no iommu_group
and neither CONFIG_VFIO_NOIOMMU and vfio_noiommu is set. But such
logic is compiled out when !CONFIG_VFIO_GROUP.

So cdev path needs to check noiommu explicitly. Just like below code.
It is called by vfio_device registration. If iommu_group is null, and
noiommu is not enabled, then it failed, hence vfio_device registration
failed. As we have such a check for noiommu at registration, so it seems
more reasonable to record this result in a flag instead of using
no_iommu_access. Is it?

+static inline int vfio_device_set_noiommu(struct vfio_device *device)
+{
+	struct iommu_group *iommu_group;
+
+	device->noiommu = false;
+
+	iommu_group = iommu_group_get(device->dev);
+	if (!iommu_group) {
+		if (!IS_ENABLED(CONFIG_VFIO_NOIOMMU) || !vfio_noiommu)
+			return -EINVAL;
+		device->noiommu = true;
+	} else {
+		iommu_group_put(iommu_group);
+	}
+
+	return 0;
+}

Regards,
Yi Liu