Re: [PATCH 1/3] VFIO: take reference to the KVM module

[Jason Gunthorpe <jgg@ziepe.ca>]

On Fri, Apr 10, 2026 at 10:16:09AM +0200, Paolo Bonzini wrote:
> Il gio 9 apr 2026, 20:59 Sean Christopherson <seanjc@google.com> ha scritto:
> >
> > On Tue, Apr 07, 2026, Paolo Bonzini wrote:
> > > VFIO is implicitly taking a reference to the KVM module between
> > > vfio_device_get_kvm_safe and vfio_device_put_kvm, thanks to
> > > symbol_get and symbol_put.
> > >
> > > In preparation for removing symbol_get and symbol_put themselves
> > > from VFIO, actually store a pointer to the KVM module and use
> > > module_get()/module_put() to keep KVM alive.
> >
> > NAK?  :-)
> >
> > I really don't think we should do this.  We're reinventing the wheel, and probably
> > doing so poorly.  As Jason suggested, the proper way to handle this is to pass
> > a "struct file" so that e.g. fops_get() pins kvm.ko for us.
> 
> We could get rid of the reference count completely (get_file() as a
> replacement for kvm_get_kvm(), get_file_active() as a replacement for
> kvm_get_kvm_safe()). struct kvm would need to add a back pointer from
> struct kvm to struct file, therefore adding and removing a reference
> count would have some additional pointer chasing. KVM has too many
> kinds of files to seriously consider passing around struct file* in
> virt/kvm/ and arch/*/kvm/, and you'd also have pointer chasing to get
> filp->private_data so it wouldn't win much.
> 
> Passing both struct kvm and struct file, instead, would be worse
> conceptually than this patch (because VFIO doesn't really care about
> the fops as opposed to the module) and uglier:
> - you can introduce a back pointer to struct kvm
> - you can change all struct kvm_device_ops implementations to receive
> a struct file*
> 
> While I can look at using the file reference for struct kvm, I'm not
> sure it's a win overall.

I would be much happier if vfio could just hang onto the file and kvm
could use file->private to get its stuff. It is so much cleaner and
simpler and keeps all the kvm stuff away from VFIO. We are trying to
do the same design for iommufd's kvm handle too, so I would prefer the
symmetry.

It seems to hinge on if virt/kvm/vfio.c can call vfio_file_set_kvm()
with a file * instead of a kvm *? A back pointer seems like an easy
way to do that?

Jason