Re: [PATCH v4 4/6] KVM/vfio: Use file-based reference counting for KVM

[Steffen Eiden]

On Tue, Sep 29, 2026 at 07:59:54PM +0200, Claudio Imbrenda wrote:
> On Mon, 28 Sep 2026 15:07:05 +0200
> Steffen Eiden <seiden@xxxxxxxxxxxxx> wrote:
> 
> > Replace manual module reference counting with file-based reference
> > counting for KVM integration. Previously, VFIO used symbol_get() to
> > obtain function pointers for kvm_get_kvm_safe() and kvm_put_kvm(),
> > then manually tracked module references through these symbols. This
> > approach required storing the put_kvm function pointer in each device
> > and carefully managing symbol references.
> > 
> > Pass struct file pointers instead of struct kvm pointers throughout the
> > VFIO-KVM interface, leveraging the kernel's existing file reference
> > counting via get_file()/get_file_active() and fput(). Convert the x86
> > page-track API and update s390 vfio to use file_to_kvm_<arch>(). This
> > simplifies the code.
> > 
> > Suggested-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > Co-developed-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > Signed-off-by: Steffen Eiden <seiden@xxxxxxxxxxxxx>
> > ---
> >  arch/s390/include/asm/kvm_host_s390.h |  2 +-
> >  arch/s390/kvm/s390/pci.c              |  9 ++++--
> >  arch/x86/include/asm/kvm_page_track.h | 10 +++---
> >  arch/x86/kvm/mmu/page_track.c         | 22 +++++++++-----
> >  drivers/s390/crypto/vfio_ap_ops.c     | 20 ++++++++----
> >  drivers/vfio/group.c                  | 11 ++++++-
> >  drivers/vfio/vfio.h                   | 12 ++++----
> >  drivers/vfio/vfio_main.c              | 57 +++++++++++------------------------
> >  include/linux/vfio.h                  |  5 ++-
> >  virt/kvm/vfio.c                       | 13 +++++---
> >  10 files changed, 87 insertions(+), 74 deletions(-)
> > 
> > diff --git a/arch/s390/include/asm/kvm_host_s390.h b/arch/s390/include/asm/kvm_host_s390.h
> > index 8a7eed5847e1..9519b8028b10 100644
> > --- a/arch/s390/include/asm/kvm_host_s390.h
> > +++ b/arch/s390/include/asm/kvm_host_s390.h
> > @@ -722,7 +722,7 @@ static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> >  void kvm_arch_free_vm(struct kvm *kvm);
> >  
> >  struct zpci_kvm_hook {
> > -	int (*kvm_register)(void *opaque, struct kvm *kvm);
> > +	int (*kvm_register)(void *opaque, struct file *kvm_file);
> >  	void (*kvm_unregister)(void *opaque);
> >  };
> >  
> > diff --git a/arch/s390/kvm/s390/pci.c b/arch/s390/kvm/s390/pci.c
> > index 82892e1e03d9..d79bd3bdc68e 100644
> > --- a/arch/s390/kvm/s390/pci.c
> > +++ b/arch/s390/kvm/s390/pci.c
> > @@ -498,17 +498,22 @@ static void kvm_s390_pci_dev_release(struct zpci_dev *zdev)
> >   * available, enable them and let userspace indicate whether or not they will
> >   * be used (specify SHM bit to disable).
> >   */
> > -static int kvm_s390_pci_register_kvm(void *opaque, struct kvm *kvm)
> > +static int kvm_s390_pci_register_kvm(void *opaque, struct file *kvm_file)
> >  {
> >  	struct zpci_dev *zdev = opaque;
> > +	struct kvm *kvm;
> >  	int rc;
> >  
> >  	if (!zdev)
> >  		return -EINVAL;
> >  
> > +	kvm = file_to_kvm_s390(kvm_file);
> > +	if (!kvm)
> > +		return -ENOENT;
> > +
> 
> the !kvm check (which you remove in this patch) returned -EINVAL, but
> now we return -ENOENT. Is this intended?

Yeah, kind of. This is now another kind of error. Before: if kvm was NULL
it was somehow not set by pci/kvm. Now: it might be an incorrect file handle as
file_to_kvm_s390 cheks for this. 

> 
> is kvm_file always guaranteed to be non-NULL? We checked for !kvm
> before, why don't we need to check for that now?
>

file_to_kvm_<arch> already tests for !kvm_file before it verifies it is
the correct file handle (by testing for kvm_fops).
So kvm_file can be null.

> >  	mutex_lock(&zdev->kzdev_lock);
> >  
> > -	if (zdev->kzdev || zdev->gisa != 0 || !kvm) {
> > +	if (zdev->kzdev || zdev->gisa != 0) {
> >  		mutex_unlock(&zdev->kzdev_lock);
> >  		return -EINVAL;
> >  	}
> 
> [...]

	Steffen