On Wed, Feb 10, 2021 at 09:37:46AM -0700, Alex Williamson wrote: > > > register a migration region and intercept guest writes to specific > > > registers. [PATCH 4/9] demonstrates the former but not the latter > > > (which is allowed in v1). > > > > And this is why, the ROI to wrapper every vfio op in a PCI op just to > > keep vfio_pci_device completely private is poor :( > > Says someone who doesn't need to maintain the core, fixing bugs and > adding features, while not breaking vendor driver touching private data > in unexpected ways ;) Said as someone that maintains a driver subsystem 25x larger than VFIO that is really experienced in "crazy things drivers do". :) Private/public is rarely at the top of my worries, and I'm confident to say this is the general kernel philosophy. Again, look anywhere, we rarely have private data split out of major structs like struct device, struct netdev, struct pci_device, etc. This data has to be public because we are using C and we expect inline functions, container_of() and so on to work. It is rarely done with hidden structs. If we can get private data in some places it is a nice win, but not worth making a mess to achieve. eg I would not give up the normal container_of pattern just to obscure some struct, the overall ROI is bad. Drivers always do unexpected and crazy things, I wouldn't get fixated on touching private data as the worst sin a driver can do :( So, no, I don't agree that exposing a struct vfio_pci_device is the end of the world - it is normal in the kernel to do this kind of thing, and yes drivers can do crazy things with that if crazy slips past the review process. Honestly I expect people to test their drivers and fix things if a core change broke them. It happens, QA finds it, it gets fixed, normal stuff for Linux, IMHO. > > > Then what exact extension is talked here by creating another subsystem > > > module? or are we talking about some general library which can be > > > shared by underlying mdev device drivers to reduce duplicated > > > emulation code? > > > > IMHO it is more a design philosophy that the end driver should > > implement the vfio_device_ops directly vs having a stack of ops > > structs. > Like Kevin though, I don't really understand the hand-wave > application to mdev. Sure, vfio-mdev could be collapsed now that > we've rejected that there could be other drivers binding to mdev > devices, Again, I think the point Max was trying to make is only that vfio_mdev can follow the same design as proposed here and replace the mdev_parent_ops with the vfio_device_ops. Jason
