On Tue, Feb 14, 2023 at 03:26:27PM -0700, Alex Williamson wrote:
> > index 857d6ba349e1..d869913baafd 100644
> > --- a/virt/kvm/vfio.c
> > +++ b/virt/kvm/vfio.c
> > @@ -286,18 +286,18 @@ static int kvm_vfio_set_file(struct kvm_device *dev, long attr,
> > int32_t fd;
> >
> > switch (attr) {
> > - case KVM_DEV_VFIO_GROUP_ADD:
> > + case KVM_DEV_VFIO_FILE_ADD:
> > if (get_user(fd, argp))
> > return -EFAULT;
> > return kvm_vfio_file_add(dev, fd);
> >
> > - case KVM_DEV_VFIO_GROUP_DEL:
> > + case KVM_DEV_VFIO_FILE_DEL:
> > if (get_user(fd, argp))
> > return -EFAULT;
> > return kvm_vfio_file_del(dev, fd);
> >
> > #ifdef CONFIG_SPAPR_TCE_IOMMU
> > - case KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE:
> > + case KVM_DEV_VFIO_FILE_SET_SPAPR_TCE:
> > return kvm_vfio_file_set_spapr_tce(dev, arg);
>
> I don't see that the SPAPR code is so easily fungible to a device
> file descriptor. The kvm_vfio_spapr_tce data structure includes a
> groupfd, which is required to match a groupfd on the file_list. So
> a SPAPR user cannot pass a cdev via FILE_ADD if they intend to use
> this TCE code.
SPAPR cannot use cdev at all, cdev mode only works with iommufd.
So with my other remark about blocking unbound cdevs, in SPAPR mode
you can never open a cdev and make it bound thus
kvm_vfio_file_iommu_group() and others will return NULL always for
cdev.
Thus AFAICT this is all fine.
Yi, you should also add some kconfig stuff to ensure that SPAPR always
has the group interface compiled in.
> That also makes me wonder what we're really gaining in forcing this
> generalization from group to file.
I think it is just less code overall. Otherwise we need to needlessly
double quite a lot of stuff, rather pointlessly, IMHO.
I'm still thinking about proposing to just delete all this SPAPR
stuff. Power still hasn't had the patches applied to make it work
again so it seems to all be dead.
Jason
