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

[Sean Christopherson]

On Mon, Apr 13, 2026, Paolo Bonzini wrote:
> On 4/10/26 17:45, Sean Christopherson wrote:
> > On Fri, Apr 10, 2026, Paolo Bonzini wrote:
> > > On Fri, Apr 10, 2026 at 4:13 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > > > 
> > > > +Dan
> > > > > 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,
> > > > 
> > > > I wasn't thinking of dropping kvm_get_kvm() entirely, rather just not exporting
> > > > it.  Forcing internal KVM usage to grab a reference to the file doesn't add a
> > > > whole lot value.
> > > 
> > > It adds not doing things in two different ways. The kvm_file is not
> > > always available (and if we need to add it, it should be added in
> > > struct kvm not struct kvm_device).
> > 
> > My thought was to deliberately avoid putting it in "kvm", because as you're
> > effectively pointing out, the file really shouldn't be passed around within KVM.
> > 
> > Aha!  What if we bury it in kvm_vfio?  As an acknowledgement that passing around
> > a kvm_file is only intended for cases where an external, non-KVM entity needs to
> > to propagate the VM reference.
> 
> That would indeed be best but it doesn't compile as there's no file argument
> to device_ops.create.  And adding it to device_ops is ugly as well.

FWIW, extending device_ops.create() doesn't seem all that ugly to me.  It's not
beautiful, but I think I'd vote for that over a kvm->file backpointer.  I'm a-ok
with either though.

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9faf70ccae7a..4e4e6b5c3b9c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4811,7 +4811,7 @@ void kvm_unregister_device_ops(u32 type)
                kvm_device_ops_table[type] = NULL;
 }
 
-static int kvm_ioctl_create_device(struct kvm *kvm,
+static int kvm_ioctl_create_device(struct kvm *kvm, struct file *kvm_file,
                                   struct kvm_create_device *cd)
 {
        const struct kvm_device_ops *ops;
@@ -4839,7 +4839,7 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
        dev->kvm = kvm;
 
        mutex_lock(&kvm->lock);
-       ret = ops->create(dev, type);
+       ret = ops->create(dev, kvm_file, type);
        if (ret < 0) {
                mutex_unlock(&kvm->lock);
                kfree(dev);
@@ -5354,7 +5354,7 @@ static long kvm_vm_ioctl(struct file *filp,
                if (copy_from_user(&cd, argp, sizeof(cd)))
                        goto out;
 
-               r = kvm_ioctl_create_device(kvm, &cd);
+               r = kvm_ioctl_create_device(kvm, filp, &cd);
                if (r)
                        goto out;