Christoph Hellwig
2023-Oct-11 14:17 UTC
[PATCH vfio 10/11] vfio/virtio: Expose admin commands over virtio device
On Wed, Oct 11, 2023 at 10:57:09AM -0300, Jason Gunthorpe wrote:> > Independent of my above points on the doubts on VF-controlled live > > migration for PCe device I absolutely agree with your that the Linux > > abstraction and user interface should be VF based. Which further > > reinforeces my point that the VFIO driver for the controlled function > > (PF or VF) and the Linux driver for the controlling function (better > > be a PF in practice) must be very tightly integrated. And the best > > way to do that is to export the vfio nodes from the Linux driver > > that knowns the hardware and not split out into a separate one. > > I'm not sure how we get to "very tightly integrated". We have many > examples of live migration vfio drivers now and they do not seem to > require tight integration. The PF driver only has to provide a way to > execute a small number of proxied operations.Yes. And for that I need to know what VF it actually is dealing with. Which is tight integration in my book.> Regardless, I'm not too fussed about what directory the implementation > lives in, though I do prefer the current arrangement where VFIO only > stuff is in drivers/vfio. I like the process we have where subsystems > are responsible for the code that implements the subsystem ops.I really don't care about where the code lives (in the directory tree) either. But as you see with virtio trying to split it out into an arbitrary module causes all kinds of pain.> > E800 also made some significant security mistakes that VFIO side > caught. I think would have been missed if it went into a netdev > tree. > > Even unrelated to mdev, Intel GPU is still not using the vfio side > properly, and the way it hacked into KVM to try to get page tracking > is totally logically wrong (but Works For Me (tm)) > > Aside from technical concerns, I do have a big process worry > here. vfio is responsible for the security side of the review of > things implementing its ops.Yes, anytjing exposing a vfio node needs vfio review, period. And I don't think where the code lived was the i915 problem. The problem was they they were the first open user of the mdev API, which was just a badly deisgned hook for never published code at that time, and they then shoehorned it into a weird hypervisor abstraction. There's no good way to succeed with that.