> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Sent: Wednesday, May 3, 2023 5:49 PM
>
> > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > 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
