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

[Claudio Imbrenda]

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?

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

>  	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;
>  	}

[...]