On Tue, Jun 13, 2023 at 11:15:11AM -0600, Alex Williamson wrote: > [Sorry for breaking threading, replying to my own message id with reply > content from Yi since the Cc list got broken] Yikes it is really busted, I think I fixed it? > If we renamed your function above to vfio_device_has_iommu_group(), > couldn't we just wrap device_add like below instead to not have cdev > setup for a noiommu device, generate an error for a physical device w/o > IOMMU backing, and otherwise setup the cdev device? > > static inline int vfio_device_add(struct vfio_device *device, enum vfio_group_type type) > { > #if IS_ENABLED(CONFIG_VFIO_GROUP) > if (device->group->type == VFIO_NO_IOMMU) > return device_add(&device->device); vfio_device_is_noiommu() embeds the IS_ENABLED > #else > if (type == VFIO_IOMMU && !vfio_device_has_iommu_group(device)) > return -EINVAL; > #endif The require test is this from the group code: if (!device_iommu_capable(dev, IOMMU_CAP_CACHE_COHERENCY)) { We could lift it out of the group code and call it from vfio_main.c like: if (type == VFIO_IOMMU && !vfio_device_is_noiommu(vdev) && !device_iommu_capable(dev, IOMMU_CAP_CACHE_COHERENCY)) FAIL Jason