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

Sean Christopherson
2026-04-13T21:21:36Z

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@google.com> 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;