On Tue, Feb 08, 2022 at 12:26:24PM -0700, Alex Williamson wrote: > > Personally, I think it is wrong layering for VFIO to be aware of KVM > > like this. This marks the first time that VFIO core code itself is > > being made aware of the KVM linkage. > > I agree, but I've resigned that I've lost that battle. Both mdev vGPU > vendors make specific assumptions about running on a VM. The vGPU's are not as egregious though, are they? > > Or, at the very least, everything needs to be described in some way > > that makes it clear what is happening to userspace, without kvm, > > through these ioctls. > > As I understand the discussion here: > > https://lore.kernel.org/all/20220204211536.321475-15-mjrosato@xxxxxxxxxxxxx/ > > The assumption is that there is no non-KVM userspace currently. This > seems like a regression to me. Indeed, I definitely don't like it either. This is not VFIO if is just driving KVM. I would prefer they add a function to get the 'struct device *' from a VFIO device fd and drive more of this from kvm, as appropriate. > > > this is meant to extend vfio-pci proper for the whole arch. Is there a > > > compromise in using #ifdefs in vfio_pci_ops to call into zpci specific > > > code that implements these arch specific hooks and the core for > > > everything else? SPAPR code could probably converted similarly, it > > > exists here for legacy reasons. [Cc Jason] > > > > I'm not sure I get what you are suggesting? Where would these ifdefs > > be? > > Essentially just: > > static const struct vfio_device_ops vfio_pci_ops = { > .name = "vfio-pci", > #ifdef CONFIG_S390 > .open_device = vfio_zpci_open_device, > .close_device = vfio_zpci_close_device, > .ioctl = vfio_zpci_ioctl, > #else > .open_device = vfio_pci_open_device, > .close_device = vfio_pci_core_close_device, > .ioctl = vfio_pci_core_ioctl, > #endif > .read = vfio_pci_core_read, > .write = vfio_pci_core_write, > .mmap = vfio_pci_core_mmap, > .request = vfio_pci_core_request, > .match = vfio_pci_core_match, > }; > > It would at least provide more validation/exercise of the core/vendor > split. Thanks, This would have to be in every pci driver - this is not just code the universal vfio-pci has to enable, but every migration driver/etc too. And we will need it again in vfio-cxl for s390 in 10 years too.. So, I think this approach is the right one, asided from the philosophical question of being so tightly linking s390 vfio to KVM. Jason