[PATCH 0/3] KVM, vfio: remove exported KVM symbols

[PATCH 0/3] KVM, vfio: remove exported KVM symbols

[Paolo Bonzini]

KVM right now exports three symbols for the whole kernel to manage
the reference count of "struct kvm"; these are used by VFIO just to
keep the "struct kvm" alive.  This can mostly be replaced
by inlines, and in fact the entire definition of struct kvm can be
made opaque to VFIO.

Besides the cleanup of removing the sort-of-deprecated symbol_get() and
the bidirectional dependency between KVM and VFIO, this is useful for
the recently posted support for Arm VMs on s390.  In that scenario each
KVM implementation module wants to have its own copy of kvm_put_kvm()
in order to call the "right" kvm_destroy_vm().  With multiple modules,
VFIO has no way to do a symbol_get() from the right module.

With this series, that problem is gone because kvm_put_kvm dispatches
through a function pointer, set by whichever implementation creates the VM.
The main issue is that symbol_get() was implicitly taking a reference
to the KVM module, and that has to be preserved.  This is the purpose
of patch 1.

Thanks,

Paolo

Paolo Bonzini (3):
  VFIO: take reference to the KVM module
  KVM, vfio: remove symbol_get(kvm_get_kvm_safe) from vfio
  KVM, vfio: remove symbol_get(kvm_put_kvm) from vfio

 arch/x86/kvm/mmu/tdp_mmu.c |  2 +-
 arch/x86/kvm/vmx/nested.h  |  4 +--
 drivers/vfio/device_cdev.c |  2 +-
 drivers/vfio/group.c       |  5 ++--
 drivers/vfio/vfio.h        | 15 +++++++----
 drivers/vfio/vfio_main.c   | 51 ++++++++++++++------------------------
 include/linux/kvm_host.h   |  9 +++----
 include/linux/kvm_types.h  | 30 ++++++++++++++++++++++
 include/linux/vfio.h       |  4 +--
 virt/kvm/kvm_main.c        | 34 ++++++-------------------
 virt/kvm/vfio.c            |  4 +--
 11 files changed, 80 insertions(+), 80 deletions(-)

-- 
2.53.0

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

[Paolo Bonzini]

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.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 drivers/vfio/device_cdev.c |  2 +-
 drivers/vfio/group.c       |  5 +++--
 drivers/vfio/vfio.h        | 15 ++++++++++-----
 drivers/vfio/vfio_main.c   | 39 +++++++++++++++++++++++---------------
 include/linux/vfio.h       |  3 ++-
 virt/kvm/vfio.c            |  4 ++--
 6 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
index 8ceca24ac136..a67d7215c239 100644
--- a/drivers/vfio/device_cdev.c
+++ b/drivers/vfio/device_cdev.c
@@ -56,7 +56,7 @@ int vfio_device_fops_cdev_open(struct inode *inode, struct file *filep)
 static void vfio_df_get_kvm_safe(struct vfio_device_file *df)
 {
 	spin_lock(&df->kvm_ref_lock);
-	vfio_device_get_kvm_safe(df->device, df->kvm);
+	vfio_device_get_kvm_safe(df->device, df->kvm, df->kvm_module);
 	spin_unlock(&df->kvm_ref_lock);
 }
 
diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index 4f15016d2a5f..7d28f45fefaa 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -158,7 +158,7 @@ static int vfio_group_ioctl_set_container(struct vfio_group *group,
 static void vfio_device_group_get_kvm_safe(struct vfio_device *device)
 {
 	spin_lock(&device->group->kvm_ref_lock);
-	vfio_device_get_kvm_safe(device, device->group->kvm);
+	vfio_device_get_kvm_safe(device, device->group->kvm, device->group->kvm_module);
 	spin_unlock(&device->group->kvm_ref_lock);
 }
 
@@ -858,10 +858,11 @@ bool vfio_group_enforced_coherent(struct vfio_group *group)
 	return ret;
 }
 
-void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
+void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm, struct module *kvm_module)
 {
 	spin_lock(&group->kvm_ref_lock);
 	group->kvm = kvm;
+	group->kvm_module = kvm_module;
 	spin_unlock(&group->kvm_ref_lock);
 }
 
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index 50128da18bca..a0c38f89b30a 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -22,8 +22,9 @@ struct vfio_device_file {
 
 	u8 access_granted;
 	u32 devid; /* only valid when iommufd is valid */
-	spinlock_t kvm_ref_lock; /* protect kvm field */
+	spinlock_t kvm_ref_lock; /* protect kvm and kvm_module fields */
 	struct kvm *kvm;
+	struct module *kvm_module;
 	struct iommufd_ctx *iommufd; /* protected by struct vfio_device_set::lock */
 };
 
@@ -89,6 +90,7 @@ struct vfio_group {
 	enum vfio_group_type		type;
 	struct mutex			group_lock;
 	struct kvm			*kvm;
+	struct module			*kvm_module;
 	struct file			*opened_file;
 	struct blocking_notifier_head	notifier;
 	struct iommufd_ctx		*iommufd;
@@ -108,7 +110,7 @@ void vfio_device_group_unuse_iommu(struct vfio_device *device);
 void vfio_df_group_close(struct vfio_device_file *df);
 struct vfio_group *vfio_group_from_file(struct file *file);
 bool vfio_group_enforced_coherent(struct vfio_group *group);
-void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm);
+void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm, struct module *kvm_module);
 bool vfio_device_has_container(struct vfio_device *device);
 int __init vfio_group_init(void);
 void vfio_group_cleanup(void);
@@ -171,7 +173,8 @@ static inline bool vfio_group_enforced_coherent(struct vfio_group *group)
 	return true;
 }
 
-static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
+static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm,
+				      struct module *kvm_module)
 {
 }
 
@@ -435,11 +438,13 @@ static inline void vfio_virqfd_exit(void)
 #endif
 
 #if IS_ENABLED(CONFIG_KVM)
-void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm);
+void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm,
+			      struct module *kvm_module);
 void vfio_device_put_kvm(struct vfio_device *device);
 #else
 static inline void vfio_device_get_kvm_safe(struct vfio_device *device,
-					    struct kvm *kvm)
+					    struct kvm *kvm,
+					    struct module *kvm_module)
 {
 }
 
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 742477546b15..d1bbc42d484a 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -433,7 +433,7 @@ void vfio_unregister_group_dev(struct vfio_device *device)
 EXPORT_SYMBOL_GPL(vfio_unregister_group_dev);
 
 #if IS_ENABLED(CONFIG_KVM)
-void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm)
+void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm, struct module *kvm_module)
 {
 	void (*pfn)(struct kvm *kvm);
 	bool (*fn)(struct kvm *kvm);
@@ -444,25 +444,31 @@ void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm)
 	if (!kvm)
 		return;
 
-	pfn = symbol_get(kvm_put_kvm);
-	if (WARN_ON(!pfn))
+	if (!try_module_get(kvm_module))
 		return;
 
+	pfn = symbol_get(kvm_put_kvm);
+	if (WARN_ON(!pfn))
+		goto out_put_mod;
+
 	fn = symbol_get(kvm_get_kvm_safe);
-	if (WARN_ON(!fn)) {
-		symbol_put(kvm_put_kvm);
-		return;
-	}
+	if (WARN_ON(!fn))
+		goto out_put_sym;
 
 	ret = fn(kvm);
 	symbol_put(kvm_get_kvm_safe);
-	if (!ret) {
-		symbol_put(kvm_put_kvm);
-		return;
-	}
+	if (!ret)
+		goto out_put_sym;
 
 	device->put_kvm = pfn;
 	device->kvm = kvm;
+	device->kvm_module = kvm_module;
+	return;
+
+out_put_sym:
+	symbol_put(kvm_put_kvm);
+out_put_mod:
+	module_put(kvm_module);
 }
 
 void vfio_device_put_kvm(struct vfio_device *device)
@@ -481,6 +487,8 @@ void vfio_device_put_kvm(struct vfio_device *device)
 
 clear:
 	device->kvm = NULL;
+	module_put(device->kvm_module);
+	device->kvm_module = NULL;
 }
 #endif
 
@@ -1483,7 +1491,7 @@ bool vfio_file_enforced_coherent(struct file *file)
 }
 EXPORT_SYMBOL_GPL(vfio_file_enforced_coherent);
 
-static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
+static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm, struct module *kvm_module)
 {
 	struct vfio_device_file *df = file->private_data;
 
@@ -1494,6 +1502,7 @@ static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
 	 */
 	spin_lock(&df->kvm_ref_lock);
 	df->kvm = kvm;
+	df->kvm_module = kvm_module;
 	spin_unlock(&df->kvm_ref_lock);
 }
 
@@ -1505,16 +1514,16 @@ static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
  * When a VFIO device is first opened the KVM will be available in
  * device->kvm if one was associated with the file.
  */
-void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
+void vfio_file_set_kvm(struct file *file, struct kvm *kvm, struct module *kvm_module)
 {
 	struct vfio_group *group;
 
 	group = vfio_group_from_file(file);
 	if (group)
-		vfio_group_set_kvm(group, kvm);
+		vfio_group_set_kvm(group, kvm, kvm_module);
 
 	if (vfio_device_from_file(file))
-		vfio_device_file_set_kvm(file, kvm);
+		vfio_device_file_set_kvm(file, kvm, kvm_module);
 }
 EXPORT_SYMBOL_GPL(vfio_file_set_kvm);
 
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index e90859956514..69a8d527b0e8 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -53,6 +53,7 @@ struct vfio_device {
 	struct list_head dev_set_list;
 	unsigned int migration_flags;
 	struct kvm *kvm;
+	struct module *kvm_module;
 
 	/* Members below here are private, not for driver use */
 	unsigned int index;
@@ -339,7 +340,7 @@ static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *devi
 #endif
 bool vfio_file_is_valid(struct file *file);
 bool vfio_file_enforced_coherent(struct file *file);
-void vfio_file_set_kvm(struct file *file, struct kvm *kvm);
+void vfio_file_set_kvm(struct file *file, struct kvm *kvm, struct module *kvm_module);
 
 #define VFIO_PIN_PAGES_MAX_ENTRIES	(PAGE_SIZE/sizeof(unsigned long))
 
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index 9f9acb66cc1e..515ed445d8e1 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -37,13 +37,13 @@ struct kvm_vfio {
 
 static void kvm_vfio_file_set_kvm(struct file *file, struct kvm *kvm)
 {
-	void (*fn)(struct file *file, struct kvm *kvm);
+	void (*fn)(struct file *file, struct kvm *kvm, struct module *kvm_module);
 
 	fn = symbol_get(vfio_file_set_kvm);
 	if (!fn)
 		return;
 
-	fn(file, kvm);
+	fn(file, kvm, kvm ? THIS_MODULE : NULL);
 
 	symbol_put(vfio_file_set_kvm);
 }
-- 
2.53.0

[PATCH 2/3] KVM, vfio: remove symbol_get(kvm_get_kvm_safe) from vfio

[Paolo Bonzini]

Right now, KVM and VFIO are using symbol_get to access each other's
symbols because of a circular reference between the modules, as well
as to avoid loading them unnecessarily.

However, usage of symbol_get is mostly deprecated and there are just a
handful of users left.  In the case of VFIO, in particular, the
functions it calls can be made inline.  Start with kvm_get_kvm_safe,
for which it is trivial to do so.

While at it, move the function from kvm_host.h to kvm_types.h.
Unlike e.g. drivers/s390/crypto/vfio_ap_ops.c, there's no need for
VFIO to know any implementation details of KVM, and struct kvm
can be treated as an opaque type.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/tdp_mmu.c |  2 +-
 arch/x86/kvm/vmx/nested.h  |  4 ++--
 drivers/vfio/vfio_main.c   |  8 +-------
 include/linux/kvm_host.h   |  8 ++++----
 include/linux/kvm_types.h  | 22 ++++++++++++++++++++++
 virt/kvm/kvm_main.c        | 27 ++++++---------------------
 6 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 9c26038f6b77..a88686b5db24 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1136,7 +1136,7 @@ void kvm_tdp_mmu_invalidate_roots(struct kvm *kvm,
 	 * being destroyed in an error path of KVM_CREATE_VM.
 	 */
 	if (IS_ENABLED(CONFIG_PROVE_LOCKING) &&
-	    refcount_read(&kvm->users_count) && kvm->created_vcpus)
+	    refcount_read(&kvm->rc.users_count) && kvm->created_vcpus)
 		lockdep_assert_held_write(&kvm->mmu_lock);
 
 	/*
diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
index 213a448104af..2c83fc905698 100644
--- a/arch/x86/kvm/vmx/nested.h
+++ b/arch/x86/kvm/vmx/nested.h
@@ -58,7 +58,7 @@ bool nested_vmx_check_io_bitmaps(struct kvm_vcpu *vcpu, unsigned int port,
 static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
 {
 	lockdep_assert_once(lockdep_is_held(&vcpu->mutex) ||
-			    !refcount_read(&vcpu->kvm->users_count));
+			    !refcount_read(&vcpu->kvm->rc.users_count));
 
 	return to_vmx(vcpu)->nested.cached_vmcs12;
 }
@@ -66,7 +66,7 @@ static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
 static inline struct vmcs12 *get_shadow_vmcs12(struct kvm_vcpu *vcpu)
 {
 	lockdep_assert_once(lockdep_is_held(&vcpu->mutex) ||
-			    !refcount_read(&vcpu->kvm->users_count));
+			    !refcount_read(&vcpu->kvm->rc.users_count));
 
 	return to_vmx(vcpu)->nested.cached_shadow_vmcs12;
 }
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index d1bbc42d484a..cb6eaabd64ce 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -436,7 +436,6 @@ EXPORT_SYMBOL_GPL(vfio_unregister_group_dev);
 void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm, struct module *kvm_module)
 {
 	void (*pfn)(struct kvm *kvm);
-	bool (*fn)(struct kvm *kvm);
 	bool ret;
 
 	lockdep_assert_held(&device->dev_set->lock);
@@ -451,12 +450,7 @@ void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm, struc
 	if (WARN_ON(!pfn))
 		goto out_put_mod;
 
-	fn = symbol_get(kvm_get_kvm_safe);
-	if (WARN_ON(!fn))
-		goto out_put_sym;
-
-	ret = fn(kvm);
-	symbol_put(kvm_get_kvm_safe);
+	ret = kvm_get_kvm_safe(kvm);
 	if (!ret)
 		goto out_put_sym;
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 6b76e7a6f4c2..5163b541c82d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -767,6 +767,9 @@ struct kvm_memslots {
 };
 
 struct kvm {
+	/* Must be the first field, see function definitions in kvm_types.h.  */
+	struct kvm_refcount rc;
+
 #ifdef KVM_HAVE_MMU_RWLOCK
 	rwlock_t mmu_lock;
 #else
@@ -830,7 +833,6 @@ struct kvm {
 	struct list_head ioeventfds;
 	struct kvm_vm_stat stat;
 	struct kvm_arch arch;
-	refcount_t users_count;
 #ifdef CONFIG_KVM_MMIO
 	struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
 	spinlock_t ring_lock;
@@ -1062,8 +1064,6 @@ static inline void kvm_irqfd_exit(void)
 int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module);
 void kvm_exit(void);
 
-void kvm_get_kvm(struct kvm *kvm);
-bool kvm_get_kvm_safe(struct kvm *kvm);
 void kvm_put_kvm(struct kvm *kvm);
 bool file_is_kvm(struct file *file);
 void kvm_put_kvm_no_destroy(struct kvm *kvm);
@@ -1073,7 +1073,7 @@ static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm, int as_id)
 	as_id = array_index_nospec(as_id, KVM_MAX_NR_ADDRESS_SPACES);
 	return srcu_dereference_check(kvm->memslots[as_id], &kvm->srcu,
 			lockdep_is_held(&kvm->slots_lock) ||
-			!refcount_read(&kvm->users_count));
+			!refcount_read(&kvm->rc.users_count));
 }
 
 static inline struct kvm_memslots *kvm_memslots(struct kvm *kvm)
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index a568d8e6f4e8..4cb68c71a13c 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -33,6 +33,7 @@
 
 #include <linux/mutex.h>
 #include <linux/spinlock_types.h>
+#include <linux/refcount.h>
 
 struct kvm;
 struct kvm_async_pf;
@@ -140,6 +141,27 @@ struct kvm_vcpu_stat_generic {
 };
 
 #define KVM_STATS_NAME_SIZE	48
+
+struct kvm_refcount {
+	refcount_t users_count;
+};
+
+static inline void kvm_get_kvm(struct kvm *kvm)
+{
+	struct kvm_refcount *rc = (struct kvm_refcount *)kvm;
+	refcount_inc(&rc->users_count);
+}
+
+/*
+ * A safe version of kvm_get_kvm(), making sure the vm is not being destroyed.
+ * Return true if kvm referenced successfully, false otherwise.
+ */
+static inline bool kvm_get_kvm_safe(struct kvm *kvm)
+{
+	struct kvm_refcount *rc = (struct kvm_refcount *)kvm;
+	return refcount_inc_not_zero(&rc->users_count);
+}
+
 #endif /* !__ASSEMBLER__ */
 
 #endif /* __KVM_TYPES_H__ */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9093251beb39..6e3796814da7 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1099,7 +1099,7 @@ static inline struct kvm_io_bus *kvm_get_bus_for_destruction(struct kvm *kvm,
 							     enum kvm_bus idx)
 {
 	return rcu_dereference_protected(kvm->buses[idx],
-					 !refcount_read(&kvm->users_count));
+					 !refcount_read(&kvm->rc.users_count));
 }
 
 static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
@@ -1153,7 +1153,8 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 	if (r)
 		goto out_err_no_irq_routing;
 
-	refcount_set(&kvm->users_count, 1);
+	BUILD_BUG_ON(offsetof(struct kvm, rc) != 0);
+	refcount_set(&kvm->rc.users_count, 1);
 
 	for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
 		for (j = 0; j < 2; j++) {
@@ -1223,7 +1224,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 out_err_no_disable:
 	kvm_arch_destroy_vm(kvm);
 out_err_no_arch_destroy_vm:
-	WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count));
+	WARN_ON_ONCE(!refcount_dec_and_test(&kvm->rc.users_count));
 	for (i = 0; i < KVM_NR_BUSES; i++)
 		kfree(kvm_get_bus_for_destruction(kvm, i));
 	kvm_free_irq_routing(kvm);
@@ -1316,25 +1317,9 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	mmdrop(mm);
 }
 
-void kvm_get_kvm(struct kvm *kvm)
-{
-	refcount_inc(&kvm->users_count);
-}
-EXPORT_SYMBOL_GPL(kvm_get_kvm);
-
-/*
- * Make sure the vm is not during destruction, which is a safe version of
- * kvm_get_kvm().  Return true if kvm referenced successfully, false otherwise.
- */
-bool kvm_get_kvm_safe(struct kvm *kvm)
-{
-	return refcount_inc_not_zero(&kvm->users_count);
-}
-EXPORT_SYMBOL_GPL(kvm_get_kvm_safe);
-
 void kvm_put_kvm(struct kvm *kvm)
 {
-	if (refcount_dec_and_test(&kvm->users_count))
+	if (refcount_dec_and_test(&kvm->rc.users_count))
 		kvm_destroy_vm(kvm);
 }
 EXPORT_SYMBOL_GPL(kvm_put_kvm);
@@ -1348,7 +1333,7 @@ EXPORT_SYMBOL_GPL(kvm_put_kvm);
  */
 void kvm_put_kvm_no_destroy(struct kvm *kvm)
 {
-	WARN_ON(refcount_dec_and_test(&kvm->users_count));
+	WARN_ON(refcount_dec_and_test(&kvm->rc.users_count));
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_put_kvm_no_destroy);
 
-- 
2.53.0

[PATCH 3/3] KVM, vfio: remove symbol_get(kvm_put_kvm) from vfio

[Paolo Bonzini]

Right now, KVM and VFIO are using symbol_get to access each other's
symbols because of a circular reference between the modules, as well
as to avoid loading them unnecessarily.

The remaining use in VFIO is for kvm_put_kvm, which is not inline
because it needs to call kvm_destroy_vm.  However, storing the
address of kvm_destroy_vm in the "struct kvm" is enough to remove
the dependency from VFIO.

This also makes it possible to direct kvm_put_kvm to either the arm64
or the s390 implementation of kvm_destroy_vm.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 drivers/vfio/vfio_main.c  | 24 +++---------------------
 include/linux/kvm_host.h  |  1 -
 include/linux/kvm_types.h |  8 ++++++++
 include/linux/vfio.h      |  1 -
 virt/kvm/kvm_main.c       |  9 ++-------
 5 files changed, 13 insertions(+), 30 deletions(-)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index cb6eaabd64ce..f27a1d00c21e 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -17,7 +17,7 @@
 #include <linux/idr.h>
 #include <linux/iommu.h>
 #if IS_ENABLED(CONFIG_KVM)
-#include <linux/kvm_host.h>
+#include <linux/kvm_types.h>
 #endif
 #include <linux/list.h>
 #include <linux/miscdevice.h>
@@ -435,9 +435,6 @@ EXPORT_SYMBOL_GPL(vfio_unregister_group_dev);
 #if IS_ENABLED(CONFIG_KVM)
 void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm, struct module *kvm_module)
 {
-	void (*pfn)(struct kvm *kvm);
-	bool ret;
-
 	lockdep_assert_held(&device->dev_set->lock);
 
 	if (!kvm)
@@ -446,21 +443,13 @@ void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm, struc
 	if (!try_module_get(kvm_module))
 		return;
 
-	pfn = symbol_get(kvm_put_kvm);
-	if (WARN_ON(!pfn))
+	if (!kvm_get_kvm_safe(kvm))
 		goto out_put_mod;
 
-	ret = kvm_get_kvm_safe(kvm);
-	if (!ret)
-		goto out_put_sym;
-
-	device->put_kvm = pfn;
 	device->kvm = kvm;
 	device->kvm_module = kvm_module;
 	return;
 
-out_put_sym:
-	symbol_put(kvm_put_kvm);
 out_put_mod:
 	module_put(kvm_module);
 }
@@ -472,14 +461,7 @@ void vfio_device_put_kvm(struct vfio_device *device)
 	if (!device->kvm)
 		return;
 
-	if (WARN_ON(!device->put_kvm))
-		goto clear;
-
-	device->put_kvm(device->kvm);
-	device->put_kvm = NULL;
-	symbol_put(kvm_put_kvm);
-
-clear:
+	kvm_put_kvm(device->kvm);
 	device->kvm = NULL;
 	module_put(device->kvm_module);
 	device->kvm_module = NULL;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5163b541c82d..ca06f9bda028 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1064,7 +1064,6 @@ static inline void kvm_irqfd_exit(void)
 int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module);
 void kvm_exit(void);
 
-void kvm_put_kvm(struct kvm *kvm);
 bool file_is_kvm(struct file *file);
 void kvm_put_kvm_no_destroy(struct kvm *kvm);
 
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 4cb68c71a13c..8a9be07cb8dd 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -144,6 +144,7 @@ struct kvm_vcpu_stat_generic {
 
 struct kvm_refcount {
 	refcount_t users_count;
+	void (*destroy)(struct kvm *kvm);
 };
 
 static inline void kvm_get_kvm(struct kvm *kvm)
@@ -162,6 +163,13 @@ static inline bool kvm_get_kvm_safe(struct kvm *kvm)
 	return refcount_inc_not_zero(&rc->users_count);
 }
 
+static inline void kvm_put_kvm(struct kvm *kvm)
+{
+	struct kvm_refcount *rc = (struct kvm_refcount *)kvm;
+	if (refcount_dec_and_test(&rc->users_count))
+		rc->destroy(kvm);
+}
+
 #endif /* !__ASSEMBLER__ */
 
 #endif /* __KVM_TYPES_H__ */
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 69a8d527b0e8..5c69532d6127 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -65,7 +65,6 @@ struct vfio_device {
 	unsigned int open_count;
 	struct completion comp;
 	struct iommufd_access *iommufd_access;
-	void (*put_kvm)(struct kvm *kvm);
 	struct inode *inode;
 #if IS_ENABLED(CONFIG_IOMMUFD)
 	struct iommufd_device *iommufd_device;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6e3796814da7..27a718a0bc01 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -120,6 +120,7 @@ static struct dentry *kvm_debugfs_dir;
 
 static const struct file_operations stat_fops_per_vm;
 
+static void kvm_destroy_vm(struct kvm *kvm);
 static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
 			   unsigned long arg);
 #ifdef CONFIG_KVM_COMPAT
@@ -1155,6 +1156,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 
 	BUILD_BUG_ON(offsetof(struct kvm, rc) != 0);
 	refcount_set(&kvm->rc.users_count, 1);
+	kvm->rc.destroy = kvm_destroy_vm;
 
 	for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
 		for (j = 0; j < 2; j++) {
@@ -1317,13 +1319,6 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	mmdrop(mm);
 }
 
-void kvm_put_kvm(struct kvm *kvm)
-{
-	if (refcount_dec_and_test(&kvm->rc.users_count))
-		kvm_destroy_vm(kvm);
-}
-EXPORT_SYMBOL_GPL(kvm_put_kvm);
-
 /*
  * Used to put a reference that was taken on behalf of an object associated
  * with a user-visible file descriptor, e.g. a vcpu or device, if installation
-- 
2.53.0

Re: [PATCH 0/3] KVM, vfio: remove exported KVM symbols

[Alex Williamson]

On Tue,  7 Apr 2026 20:01:04 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> KVM right now exports three symbols for the whole kernel to manage
> the reference count of "struct kvm"; these are used by VFIO just to
> keep the "struct kvm" alive.  This can mostly be replaced
> by inlines, and in fact the entire definition of struct kvm can be
> made opaque to VFIO.
> 
> Besides the cleanup of removing the sort-of-deprecated symbol_get() and
> the bidirectional dependency between KVM and VFIO, this is useful for
> the recently posted support for Arm VMs on s390.  In that scenario each
> KVM implementation module wants to have its own copy of kvm_put_kvm()
> in order to call the "right" kvm_destroy_vm().  With multiple modules,
> VFIO has no way to do a symbol_get() from the right module.
> 
> With this series, that problem is gone because kvm_put_kvm dispatches
> through a function pointer, set by whichever implementation creates the VM.
> The main issue is that symbol_get() was implicitly taking a reference
> to the KVM module, and that has to be preserved.  This is the purpose
> of patch 1.
> 
> Thanks,
> 
> Paolo
> 
> Paolo Bonzini (3):
>   VFIO: take reference to the KVM module
>   KVM, vfio: remove symbol_get(kvm_get_kvm_safe) from vfio
>   KVM, vfio: remove symbol_get(kvm_put_kvm) from vfio
> 
>  arch/x86/kvm/mmu/tdp_mmu.c |  2 +-
>  arch/x86/kvm/vmx/nested.h  |  4 +--
>  drivers/vfio/device_cdev.c |  2 +-
>  drivers/vfio/group.c       |  5 ++--
>  drivers/vfio/vfio.h        | 15 +++++++----
>  drivers/vfio/vfio_main.c   | 51 ++++++++++++++------------------------
>  include/linux/kvm_host.h   |  9 +++----
>  include/linux/kvm_types.h  | 30 ++++++++++++++++++++++
>  include/linux/vfio.h       |  4 +--
>  virt/kvm/kvm_main.c        | 34 ++++++-------------------
>  virt/kvm/vfio.c            |  4 +--
>  11 files changed, 80 insertions(+), 80 deletions(-)
> 

Looks clean.

Acked-by: Alex Williamson <alex@shazbot.org>

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

[Steffen Eiden]

On Tue, Apr 07, 2026 at 08:01:05PM +0200, 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.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>

Re: [PATCH 2/3] KVM, vfio: remove symbol_get(kvm_get_kvm_safe) from vfio

[Steffen Eiden]

On Tue, Apr 07, 2026 at 08:01:06PM +0200, Paolo Bonzini wrote:
> Right now, KVM and VFIO are using symbol_get to access each other's
> symbols because of a circular reference between the modules, as well
> as to avoid loading them unnecessarily.
> 
> However, usage of symbol_get is mostly deprecated and there are just a
> handful of users left.  In the case of VFIO, in particular, the
> functions it calls can be made inline.  Start with kvm_get_kvm_safe,
> for which it is trivial to do so.
> 
> While at it, move the function from kvm_host.h to kvm_types.h.
> Unlike e.g. drivers/s390/crypto/vfio_ap_ops.c, there's no need for
> VFIO to know any implementation details of KVM, and struct kvm
> can be treated as an opaque type.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>

Re: [PATCH 3/3] KVM, vfio: remove symbol_get(kvm_put_kvm) from vfio

[Steffen Eiden]

On Tue, Apr 07, 2026 at 08:01:07PM +0200, Paolo Bonzini wrote:
> Right now, KVM and VFIO are using symbol_get to access each other's
> symbols because of a circular reference between the modules, as well
> as to avoid loading them unnecessarily.
> 
> The remaining use in VFIO is for kvm_put_kvm, which is not inline
> because it needs to call kvm_destroy_vm.  However, storing the
> address of kvm_destroy_vm in the "struct kvm" is enough to remove
> the dependency from VFIO.
> 
> This also makes it possible to direct kvm_put_kvm to either the arm64
> or the s390 implementation of kvm_destroy_vm.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>

Re: [PATCH 0/3] KVM, vfio: remove exported KVM symbols

[Steffen Eiden]

On Tue, Apr 07, 2026 at 08:01:04PM +0200, Paolo Bonzini wrote:
> KVM right now exports three symbols for the whole kernel to manage
> the reference count of "struct kvm"; these are used by VFIO just to
> keep the "struct kvm" alive.  This can mostly be replaced
> by inlines, and in fact the entire definition of struct kvm can be
> made opaque to VFIO.
> 
> Besides the cleanup of removing the sort-of-deprecated symbol_get() and
> the bidirectional dependency between KVM and VFIO, this is useful for
> the recently posted support for Arm VMs on s390.  In that scenario each
> KVM implementation module wants to have its own copy of kvm_put_kvm()
> in order to call the "right" kvm_destroy_vm().  With multiple modules,
> VFIO has no way to do a symbol_get() from the right module.
> 
> With this series, that problem is gone because kvm_put_kvm dispatches
> through a function pointer, set by whichever implementation creates the VM.
> The main issue is that symbol_get() was implicitly taking a reference
> to the KVM module, and that has to be preserved.  This is the purpose
> of patch 1.
> 
> Thanks,
> 
> Paolo
> 
> Paolo Bonzini (3):
>   VFIO: take reference to the KVM module
>   KVM, vfio: remove symbol_get(kvm_get_kvm_safe) from vfio
>   KVM, vfio: remove symbol_get(kvm_put_kvm) from vfio
> 

...

Thanks for sending/fixing the issue. LGTM.

For v2 of the arm-on-s390 series I'll drop the (first )three patches and
depend on this series. So we can merge them early. OK?

	Steffen

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

[Sean Christopherson]

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.

https://lore.kernel.org/all/20231203140756.GI1489931@ziepe.ca

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

[Paolo Bonzini]

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.

Paolo

>
> https://lore.kernel.org/all/20231203140756.GI1489931@ziepe.ca
>
>

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

[Sean Christopherson]

+Dan

On Fri, Apr 10, 2026, 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,

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.

> 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.

Again, I'm not suggesting we do that.  The conversion looks pretty straightforward;
the compile-tested only, but it doesn't seem overly complex.

drivers/s390/crypto/vfio_ap_ops.c would need similar treatment.  I didn't try to
handle it in my sketch because it's not clear to me how we want to deal with that
thing, as it's accessing a pile of state/fields that really shouldn't be used by
non-KVM code :-(

I added Dan because the PCI TSM stuff is picking up "struct kvm *kvm" references,
and I want to head that off too, i.e. have it use the file approach instead of
whatever it plans on doing (can't tell from the code, because there are no users).

---
 arch/x86/include/asm/kvm_page_track.h |  8 +++---
 arch/x86/kvm/mmu/page_track.c         | 16 +++++++----
 drivers/vfio/group.c                  |  2 +-
 drivers/vfio/vfio.h                   | 12 ++++----
 drivers/vfio/vfio_main.c              | 41 ++++-----------------------
 include/linux/kvm_host.h              |  2 ++
 include/linux/vfio.h                  |  5 ++--
 virt/kvm/kvm_main.c                   | 14 +++++++--
 virt/kvm/vfio.c                       |  6 ++--
 9 files changed, 46 insertions(+), 60 deletions(-)

diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
index 3d040741044b..046a25c8fe4f 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -44,13 +44,13 @@ struct kvm_page_track_notifier_node {
 				    struct kvm_page_track_notifier_node *node);
 };
 
-int kvm_page_track_register_notifier(struct kvm *kvm,
+int kvm_page_track_register_notifier(struct file *file,
 				     struct kvm_page_track_notifier_node *n);
-void kvm_page_track_unregister_notifier(struct kvm *kvm,
+void kvm_page_track_unregister_notifier(struct file *file,
 					struct kvm_page_track_notifier_node *n);
 
-int kvm_write_track_add_gfn(struct kvm *kvm, gfn_t gfn);
-int kvm_write_track_remove_gfn(struct kvm *kvm, gfn_t gfn);
+int kvm_write_track_add_gfn(struct file *file, gfn_t gfn);
+int kvm_write_track_remove_gfn(struct file *file, gfn_t gfn);
 #else
 /*
  * Allow defining a node in a structure even if page tracking is disabled, e.g.
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 1b17b12393a8..f070dacc4028 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -217,10 +217,11 @@ static int kvm_enable_external_write_tracking(struct kvm *kvm)
  * register the notifier so that event interception for the tracked guest
  * pages can be received.
  */
-int kvm_page_track_register_notifier(struct kvm *kvm,
+int kvm_page_track_register_notifier(struct file *file,
 				     struct kvm_page_track_notifier_node *n)
 {
 	struct kvm_page_track_notifier_head *head;
+	struct kvm *kvm = file_to_kvm(file);
 	int r;
 
 	if (!kvm || kvm->mm != current->mm)
@@ -232,7 +233,7 @@ int kvm_page_track_register_notifier(struct kvm *kvm,
 			return r;
 	}
 
-	kvm_get_kvm(kvm);
+	get_file(file);
 
 	head = &kvm->arch.track_notifier_head;
 
@@ -247,10 +248,11 @@ EXPORT_SYMBOL_GPL(kvm_page_track_register_notifier);
  * stop receiving the event interception. It is the opposed operation of
  * kvm_page_track_register_notifier().
  */
-void kvm_page_track_unregister_notifier(struct kvm *kvm,
+void kvm_page_track_unregister_notifier(struct file *file,
 					struct kvm_page_track_notifier_node *n)
 {
 	struct kvm_page_track_notifier_head *head;
+	struct kvm *kvm = file_to_kvm(file);
 
 	head = &kvm->arch.track_notifier_head;
 
@@ -259,7 +261,7 @@ void kvm_page_track_unregister_notifier(struct kvm *kvm,
 	write_unlock(&kvm->mmu_lock);
 	synchronize_srcu(&head->track_srcu);
 
-	kvm_put_kvm(kvm);
+	fput(file);
 }
 EXPORT_SYMBOL_GPL(kvm_page_track_unregister_notifier);
 
@@ -319,8 +321,9 @@ void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot)
  * @kvm: the guest instance we are interested in.
  * @gfn: the guest page.
  */
-int kvm_write_track_add_gfn(struct kvm *kvm, gfn_t gfn)
+int kvm_write_track_add_gfn(struct file *file, gfn_t gfn)
 {
+	struct kvm *kvm = file_to_kvm(file);
 	struct kvm_memory_slot *slot;
 	int idx;
 
@@ -349,8 +352,9 @@ EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
  * @kvm: the guest instance we are interested in.
  * @gfn: the guest page.
  */
-int kvm_write_track_remove_gfn(struct kvm *kvm, gfn_t gfn)
+int kvm_write_track_remove_gfn(struct file *file, gfn_t gfn)
 {
+	struct kvm *kvm = file_to_kvm(file);
 	struct kvm_memory_slot *slot;
 	int idx;
 
diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index 4f15016d2a5f..f4a81d1b63fa 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -858,7 +858,7 @@ bool vfio_group_enforced_coherent(struct vfio_group *group)
 	return ret;
 }
 
-void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
+void vfio_group_set_kvm(struct vfio_group *group, struct file *kvm)
 {
 	spin_lock(&group->kvm_ref_lock);
 	group->kvm = kvm;
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index 50128da18bca..aa93c1ec6b10 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -23,7 +23,7 @@ struct vfio_device_file {
 	u8 access_granted;
 	u32 devid; /* only valid when iommufd is valid */
 	spinlock_t kvm_ref_lock; /* protect kvm field */
-	struct kvm *kvm;
+	struct file *kvm;
 	struct iommufd_ctx *iommufd; /* protected by struct vfio_device_set::lock */
 };
 
@@ -88,7 +88,7 @@ struct vfio_group {
 #endif
 	enum vfio_group_type		type;
 	struct mutex			group_lock;
-	struct kvm			*kvm;
+	struct file			*kvm;
 	struct file			*opened_file;
 	struct blocking_notifier_head	notifier;
 	struct iommufd_ctx		*iommufd;
@@ -108,7 +108,7 @@ void vfio_device_group_unuse_iommu(struct vfio_device *device);
 void vfio_df_group_close(struct vfio_device_file *df);
 struct vfio_group *vfio_group_from_file(struct file *file);
 bool vfio_group_enforced_coherent(struct vfio_group *group);
-void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm);
+void vfio_group_set_kvm(struct vfio_group *group, struct file *kvm);
 bool vfio_device_has_container(struct vfio_device *device);
 int __init vfio_group_init(void);
 void vfio_group_cleanup(void);
@@ -171,7 +171,7 @@ static inline bool vfio_group_enforced_coherent(struct vfio_group *group)
 	return true;
 }
 
-static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
+static inline void vfio_group_set_kvm(struct vfio_group *group, struct file *kvm)
 {
 }
 
@@ -435,11 +435,11 @@ static inline void vfio_virqfd_exit(void)
 #endif
 
 #if IS_ENABLED(CONFIG_KVM)
-void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm);
+void vfio_device_get_kvm_safe(struct vfio_device *device, struct file *kvm);
 void vfio_device_put_kvm(struct vfio_device *device);
 #else
 static inline void vfio_device_get_kvm_safe(struct vfio_device *device,
-					    struct kvm *kvm)
+					    struct file *kvm)
 {
 }
 
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 742477546b15..7af53d1288f8 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -433,35 +433,13 @@ void vfio_unregister_group_dev(struct vfio_device *device)
 EXPORT_SYMBOL_GPL(vfio_unregister_group_dev);
 
 #if IS_ENABLED(CONFIG_KVM)
-void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm)
+void vfio_device_get_kvm_safe(struct vfio_device *device, struct file *kvm)
 {
-	void (*pfn)(struct kvm *kvm);
-	bool (*fn)(struct kvm *kvm);
-	bool ret;
-
 	lockdep_assert_held(&device->dev_set->lock);
 
-	if (!kvm)
+	if (!get_file_active(&kvm))
 		return;
 
-	pfn = symbol_get(kvm_put_kvm);
-	if (WARN_ON(!pfn))
-		return;
-
-	fn = symbol_get(kvm_get_kvm_safe);
-	if (WARN_ON(!fn)) {
-		symbol_put(kvm_put_kvm);
-		return;
-	}
-
-	ret = fn(kvm);
-	symbol_put(kvm_get_kvm_safe);
-	if (!ret) {
-		symbol_put(kvm_put_kvm);
-		return;
-	}
-
-	device->put_kvm = pfn;
 	device->kvm = kvm;
 }
 
@@ -472,14 +450,7 @@ void vfio_device_put_kvm(struct vfio_device *device)
 	if (!device->kvm)
 		return;
 
-	if (WARN_ON(!device->put_kvm))
-		goto clear;
-
-	device->put_kvm(device->kvm);
-	device->put_kvm = NULL;
-	symbol_put(kvm_put_kvm);
-
-clear:
+	fput(device->kvm);
 	device->kvm = NULL;
 }
 #endif
@@ -1483,7 +1454,7 @@ bool vfio_file_enforced_coherent(struct file *file)
 }
 EXPORT_SYMBOL_GPL(vfio_file_enforced_coherent);
 
-static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
+static void vfio_device_file_set_kvm(struct file *file, struct file *kvm)
 {
 	struct vfio_device_file *df = file->private_data;
 
@@ -1500,12 +1471,12 @@ static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
 /**
  * vfio_file_set_kvm - Link a kvm with VFIO drivers
  * @file: VFIO group file or VFIO device file
- * @kvm: KVM to link
+ * @kvm: KVM file to link
  *
  * When a VFIO device is first opened the KVM will be available in
  * device->kvm if one was associated with the file.
  */
-void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
+void vfio_file_set_kvm(struct file *file, struct file *kvm)
 {
 	struct vfio_group *group;
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 6b76e7a6f4c2..7ea398d33e9f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1066,6 +1066,7 @@ void kvm_get_kvm(struct kvm *kvm);
 bool kvm_get_kvm_safe(struct kvm *kvm);
 void kvm_put_kvm(struct kvm *kvm);
 bool file_is_kvm(struct file *file);
+struct kvm *file_to_kvm(struct file *file);
 void kvm_put_kvm_no_destroy(struct kvm *kvm);
 
 static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm, int as_id)
@@ -2310,6 +2311,7 @@ extern unsigned int halt_poll_ns_shrink;
 
 struct kvm_device {
 	const struct kvm_device_ops *ops;
+	struct file *kvm_file;
 	struct kvm *kvm;
 	void *private;
 	struct list_head vm_node;
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index e90859956514..6dd5dd2550a0 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -52,7 +52,7 @@ struct vfio_device {
 	struct vfio_device_set *dev_set;
 	struct list_head dev_set_list;
 	unsigned int migration_flags;
-	struct kvm *kvm;
+	struct file *kvm;
 
 	/* Members below here are private, not for driver use */
 	unsigned int index;
@@ -64,7 +64,6 @@ struct vfio_device {
 	unsigned int open_count;
 	struct completion comp;
 	struct iommufd_access *iommufd_access;
-	void (*put_kvm)(struct kvm *kvm);
 	struct inode *inode;
 #if IS_ENABLED(CONFIG_IOMMUFD)
 	struct iommufd_device *iommufd_device;
@@ -339,7 +338,7 @@ static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *devi
 #endif
 bool vfio_file_is_valid(struct file *file);
 bool vfio_file_enforced_coherent(struct file *file);
-void vfio_file_set_kvm(struct file *file, struct kvm *kvm);
+void vfio_file_set_kvm(struct file *file, struct file *kvm);
 
 #define VFIO_PIN_PAGES_MAX_ENTRIES	(PAGE_SIZE/sizeof(unsigned long))
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9093251beb39..b240a3cc5995 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4808,7 +4808,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 file *file, struct kvm *kvm,
 				   struct kvm_create_device *cd)
 {
 	const struct kvm_device_ops *ops;
@@ -4834,6 +4834,7 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
 
 	dev->ops = ops;
 	dev->kvm = kvm;
+	dev->kvm_file = file;
 
 	mutex_lock(&kvm->lock);
 	ret = ops->create(dev, type);
@@ -5351,7 +5352,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(filp, kvm, &cd);
 		if (r)
 			goto out;
 
@@ -5483,6 +5484,15 @@ bool file_is_kvm(struct file *file)
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(file_is_kvm);
 
+struct kvm *file_to_kvm(struct file *file)
+{
+	if (!file_is_kvm(file))
+		return NULL;
+
+	return file->private_data;
+}
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(file_to_kvm);
+
 static int kvm_dev_ioctl_create_vm(unsigned long type)
 {
 	char fdname[ITOA_MAX_LEN + 1];
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index 9f9acb66cc1e..f6681dacf7b9 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -35,9 +35,9 @@ struct kvm_vfio {
 	bool noncoherent;
 };
 
-static void kvm_vfio_file_set_kvm(struct file *file, struct kvm *kvm)
+static void kvm_vfio_file_set_kvm(struct file *file, struct file *kvm)
 {
-	void (*fn)(struct file *file, struct kvm *kvm);
+	void (*fn)(struct file *file, struct file *kvm);
 
 	fn = symbol_get(vfio_file_set_kvm);
 	if (!fn)
@@ -175,7 +175,7 @@ static int kvm_vfio_file_add(struct kvm_device *dev, unsigned int fd)
 	kvf->file = get_file(filp);
 	list_add_tail(&kvf->node, &kv->file_list);
 
-	kvm_vfio_file_set_kvm(kvf->file, dev->kvm);
+	kvm_vfio_file_set_kvm(kvf->file, dev->kvm_file);
 	kvm_vfio_update_coherency(dev);
 
 out_unlock:

base-commit: df83746075778958954aa0460cca55f4b3fc9c02
--

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

[Paolo Bonzini]

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).

> > 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.
>
> Again, I'm not suggesting we do that.  The conversion looks pretty straightforward;
> the compile-tested only, but it doesn't seem overly complex.

I don't like losing the type safety - though file_is_kvm() works as a
run-time check, we could keep a struct kvm_file wrapper and
kvm_file_get/kvm_file_get_kvm/kvm_file_put to manage the reference
count.

But yeah, it's better in other ways. The relatively new
get_file_active()/get_file() APIs make the change nicely transparent.

> drivers/s390/crypto/vfio_ap_ops.c would need similar treatment.  I didn't try to
> handle it in my sketch because it's not clear to me how we want to deal with that
> thing, as it's accessing a pile of state/fields that really shouldn't be used by
> non-KVM code :-(

It accessing vcpu->run is not too bad, because vfio_ap_ops.c needs to
do what userspace usually does. Except it's not userspace and does not
have access to (say) QEMU's data structure for the memory map, which
is where the memslot hacks lie. We can provide a clean API for that
but I think it's unavoidable. :(

Paolo

> I added Dan because the PCI TSM stuff is picking up "struct kvm *kvm" references,
> and I want to head that off too, i.e. have it use the file approach instead of
> whatever it plans on doing (can't tell from the code, because there are no users).
>
> ---
>  arch/x86/include/asm/kvm_page_track.h |  8 +++---
>  arch/x86/kvm/mmu/page_track.c         | 16 +++++++----
>  drivers/vfio/group.c                  |  2 +-
>  drivers/vfio/vfio.h                   | 12 ++++----
>  drivers/vfio/vfio_main.c              | 41 ++++-----------------------
>  include/linux/kvm_host.h              |  2 ++
>  include/linux/vfio.h                  |  5 ++--
>  virt/kvm/kvm_main.c                   | 14 +++++++--
>  virt/kvm/vfio.c                       |  6 ++--
>  9 files changed, 46 insertions(+), 60 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
> index 3d040741044b..046a25c8fe4f 100644
> --- a/arch/x86/include/asm/kvm_page_track.h
> +++ b/arch/x86/include/asm/kvm_page_track.h
> @@ -44,13 +44,13 @@ struct kvm_page_track_notifier_node {
>                                     struct kvm_page_track_notifier_node *node);
>  };
>
> -int kvm_page_track_register_notifier(struct kvm *kvm,
> +int kvm_page_track_register_notifier(struct file *file,
>                                      struct kvm_page_track_notifier_node *n);
> -void kvm_page_track_unregister_notifier(struct kvm *kvm,
> +void kvm_page_track_unregister_notifier(struct file *file,
>                                         struct kvm_page_track_notifier_node *n);
>
> -int kvm_write_track_add_gfn(struct kvm *kvm, gfn_t gfn);
> -int kvm_write_track_remove_gfn(struct kvm *kvm, gfn_t gfn);
> +int kvm_write_track_add_gfn(struct file *file, gfn_t gfn);
> +int kvm_write_track_remove_gfn(struct file *file, gfn_t gfn);
>  #else
>  /*
>   * Allow defining a node in a structure even if page tracking is disabled, e.g.
> diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
> index 1b17b12393a8..f070dacc4028 100644
> --- a/arch/x86/kvm/mmu/page_track.c
> +++ b/arch/x86/kvm/mmu/page_track.c
> @@ -217,10 +217,11 @@ static int kvm_enable_external_write_tracking(struct kvm *kvm)
>   * register the notifier so that event interception for the tracked guest
>   * pages can be received.
>   */
> -int kvm_page_track_register_notifier(struct kvm *kvm,
> +int kvm_page_track_register_notifier(struct file *file,
>                                      struct kvm_page_track_notifier_node *n)
>  {
>         struct kvm_page_track_notifier_head *head;
> +       struct kvm *kvm = file_to_kvm(file);
>         int r;
>
>         if (!kvm || kvm->mm != current->mm)
> @@ -232,7 +233,7 @@ int kvm_page_track_register_notifier(struct kvm *kvm,
>                         return r;
>         }
>
> -       kvm_get_kvm(kvm);
> +       get_file(file);
>
>         head = &kvm->arch.track_notifier_head;
>
> @@ -247,10 +248,11 @@ EXPORT_SYMBOL_GPL(kvm_page_track_register_notifier);
>   * stop receiving the event interception. It is the opposed operation of
>   * kvm_page_track_register_notifier().
>   */
> -void kvm_page_track_unregister_notifier(struct kvm *kvm,
> +void kvm_page_track_unregister_notifier(struct file *file,
>                                         struct kvm_page_track_notifier_node *n)
>  {
>         struct kvm_page_track_notifier_head *head;
> +       struct kvm *kvm = file_to_kvm(file);
>
>         head = &kvm->arch.track_notifier_head;
>
> @@ -259,7 +261,7 @@ void kvm_page_track_unregister_notifier(struct kvm *kvm,
>         write_unlock(&kvm->mmu_lock);
>         synchronize_srcu(&head->track_srcu);
>
> -       kvm_put_kvm(kvm);
> +       fput(file);
>  }
>  EXPORT_SYMBOL_GPL(kvm_page_track_unregister_notifier);
>
> @@ -319,8 +321,9 @@ void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot)
>   * @kvm: the guest instance we are interested in.
>   * @gfn: the guest page.
>   */
> -int kvm_write_track_add_gfn(struct kvm *kvm, gfn_t gfn)
> +int kvm_write_track_add_gfn(struct file *file, gfn_t gfn)
>  {
> +       struct kvm *kvm = file_to_kvm(file);
>         struct kvm_memory_slot *slot;
>         int idx;
>
> @@ -349,8 +352,9 @@ EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
>   * @kvm: the guest instance we are interested in.
>   * @gfn: the guest page.
>   */
> -int kvm_write_track_remove_gfn(struct kvm *kvm, gfn_t gfn)
> +int kvm_write_track_remove_gfn(struct file *file, gfn_t gfn)
>  {
> +       struct kvm *kvm = file_to_kvm(file);
>         struct kvm_memory_slot *slot;
>         int idx;
>
> diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
> index 4f15016d2a5f..f4a81d1b63fa 100644
> --- a/drivers/vfio/group.c
> +++ b/drivers/vfio/group.c
> @@ -858,7 +858,7 @@ bool vfio_group_enforced_coherent(struct vfio_group *group)
>         return ret;
>  }
>
> -void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
> +void vfio_group_set_kvm(struct vfio_group *group, struct file *kvm)
>  {
>         spin_lock(&group->kvm_ref_lock);
>         group->kvm = kvm;
> diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
> index 50128da18bca..aa93c1ec6b10 100644
> --- a/drivers/vfio/vfio.h
> +++ b/drivers/vfio/vfio.h
> @@ -23,7 +23,7 @@ struct vfio_device_file {
>         u8 access_granted;
>         u32 devid; /* only valid when iommufd is valid */
>         spinlock_t kvm_ref_lock; /* protect kvm field */
> -       struct kvm *kvm;
> +       struct file *kvm;
>         struct iommufd_ctx *iommufd; /* protected by struct vfio_device_set::lock */
>  };
>
> @@ -88,7 +88,7 @@ struct vfio_group {
>  #endif
>         enum vfio_group_type            type;
>         struct mutex                    group_lock;
> -       struct kvm                      *kvm;
> +       struct file                     *kvm;
>         struct file                     *opened_file;
>         struct blocking_notifier_head   notifier;
>         struct iommufd_ctx              *iommufd;
> @@ -108,7 +108,7 @@ void vfio_device_group_unuse_iommu(struct vfio_device *device);
>  void vfio_df_group_close(struct vfio_device_file *df);
>  struct vfio_group *vfio_group_from_file(struct file *file);
>  bool vfio_group_enforced_coherent(struct vfio_group *group);
> -void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm);
> +void vfio_group_set_kvm(struct vfio_group *group, struct file *kvm);
>  bool vfio_device_has_container(struct vfio_device *device);
>  int __init vfio_group_init(void);
>  void vfio_group_cleanup(void);
> @@ -171,7 +171,7 @@ static inline bool vfio_group_enforced_coherent(struct vfio_group *group)
>         return true;
>  }
>
> -static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm)
> +static inline void vfio_group_set_kvm(struct vfio_group *group, struct file *kvm)
>  {
>  }
>
> @@ -435,11 +435,11 @@ static inline void vfio_virqfd_exit(void)
>  #endif
>
>  #if IS_ENABLED(CONFIG_KVM)
> -void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm);
> +void vfio_device_get_kvm_safe(struct vfio_device *device, struct file *kvm);
>  void vfio_device_put_kvm(struct vfio_device *device);
>  #else
>  static inline void vfio_device_get_kvm_safe(struct vfio_device *device,
> -                                           struct kvm *kvm)
> +                                           struct file *kvm)
>  {
>  }
>
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 742477546b15..7af53d1288f8 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -433,35 +433,13 @@ void vfio_unregister_group_dev(struct vfio_device *device)
>  EXPORT_SYMBOL_GPL(vfio_unregister_group_dev);
>
>  #if IS_ENABLED(CONFIG_KVM)
> -void vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm)
> +void vfio_device_get_kvm_safe(struct vfio_device *device, struct file *kvm)
>  {
> -       void (*pfn)(struct kvm *kvm);
> -       bool (*fn)(struct kvm *kvm);
> -       bool ret;
> -
>         lockdep_assert_held(&device->dev_set->lock);
>
> -       if (!kvm)
> +       if (!get_file_active(&kvm))
>                 return;
>
> -       pfn = symbol_get(kvm_put_kvm);
> -       if (WARN_ON(!pfn))
> -               return;
> -
> -       fn = symbol_get(kvm_get_kvm_safe);
> -       if (WARN_ON(!fn)) {
> -               symbol_put(kvm_put_kvm);
> -               return;
> -       }
> -
> -       ret = fn(kvm);
> -       symbol_put(kvm_get_kvm_safe);
> -       if (!ret) {
> -               symbol_put(kvm_put_kvm);
> -               return;
> -       }
> -
> -       device->put_kvm = pfn;
>         device->kvm = kvm;
>  }
>
> @@ -472,14 +450,7 @@ void vfio_device_put_kvm(struct vfio_device *device)
>         if (!device->kvm)
>                 return;
>
> -       if (WARN_ON(!device->put_kvm))
> -               goto clear;
> -
> -       device->put_kvm(device->kvm);
> -       device->put_kvm = NULL;
> -       symbol_put(kvm_put_kvm);
> -
> -clear:
> +       fput(device->kvm);
>         device->kvm = NULL;
>  }
>  #endif
> @@ -1483,7 +1454,7 @@ bool vfio_file_enforced_coherent(struct file *file)
>  }
>  EXPORT_SYMBOL_GPL(vfio_file_enforced_coherent);
>
> -static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
> +static void vfio_device_file_set_kvm(struct file *file, struct file *kvm)
>  {
>         struct vfio_device_file *df = file->private_data;
>
> @@ -1500,12 +1471,12 @@ static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
>  /**
>   * vfio_file_set_kvm - Link a kvm with VFIO drivers
>   * @file: VFIO group file or VFIO device file
> - * @kvm: KVM to link
> + * @kvm: KVM file to link
>   *
>   * When a VFIO device is first opened the KVM will be available in
>   * device->kvm if one was associated with the file.
>   */
> -void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
> +void vfio_file_set_kvm(struct file *file, struct file *kvm)
>  {
>         struct vfio_group *group;
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 6b76e7a6f4c2..7ea398d33e9f 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1066,6 +1066,7 @@ void kvm_get_kvm(struct kvm *kvm);
>  bool kvm_get_kvm_safe(struct kvm *kvm);
>  void kvm_put_kvm(struct kvm *kvm);
>  bool file_is_kvm(struct file *file);
> +struct kvm *file_to_kvm(struct file *file);
>  void kvm_put_kvm_no_destroy(struct kvm *kvm);
>
>  static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm, int as_id)
> @@ -2310,6 +2311,7 @@ extern unsigned int halt_poll_ns_shrink;
>
>  struct kvm_device {
>         const struct kvm_device_ops *ops;
> +       struct file *kvm_file;
>         struct kvm *kvm;
>         void *private;
>         struct list_head vm_node;
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index e90859956514..6dd5dd2550a0 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -52,7 +52,7 @@ struct vfio_device {
>         struct vfio_device_set *dev_set;
>         struct list_head dev_set_list;
>         unsigned int migration_flags;
> -       struct kvm *kvm;
> +       struct file *kvm;
>
>         /* Members below here are private, not for driver use */
>         unsigned int index;
> @@ -64,7 +64,6 @@ struct vfio_device {
>         unsigned int open_count;
>         struct completion comp;
>         struct iommufd_access *iommufd_access;
> -       void (*put_kvm)(struct kvm *kvm);
>         struct inode *inode;
>  #if IS_ENABLED(CONFIG_IOMMUFD)
>         struct iommufd_device *iommufd_device;
> @@ -339,7 +338,7 @@ static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *devi
>  #endif
>  bool vfio_file_is_valid(struct file *file);
>  bool vfio_file_enforced_coherent(struct file *file);
> -void vfio_file_set_kvm(struct file *file, struct kvm *kvm);
> +void vfio_file_set_kvm(struct file *file, struct file *kvm);
>
>  #define VFIO_PIN_PAGES_MAX_ENTRIES     (PAGE_SIZE/sizeof(unsigned long))
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 9093251beb39..b240a3cc5995 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4808,7 +4808,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 file *file, struct kvm *kvm,
>                                    struct kvm_create_device *cd)
>  {
>         const struct kvm_device_ops *ops;
> @@ -4834,6 +4834,7 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
>
>         dev->ops = ops;
>         dev->kvm = kvm;
> +       dev->kvm_file = file;
>
>         mutex_lock(&kvm->lock);
>         ret = ops->create(dev, type);
> @@ -5351,7 +5352,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(filp, kvm, &cd);
>                 if (r)
>                         goto out;
>
> @@ -5483,6 +5484,15 @@ bool file_is_kvm(struct file *file)
>  }
>  EXPORT_SYMBOL_FOR_KVM_INTERNAL(file_is_kvm);
>
> +struct kvm *file_to_kvm(struct file *file)
> +{
> +       if (!file_is_kvm(file))
> +               return NULL;
> +
> +       return file->private_data;
> +}
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(file_to_kvm);
> +
>  static int kvm_dev_ioctl_create_vm(unsigned long type)
>  {
>         char fdname[ITOA_MAX_LEN + 1];
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index 9f9acb66cc1e..f6681dacf7b9 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -35,9 +35,9 @@ struct kvm_vfio {
>         bool noncoherent;
>  };
>
> -static void kvm_vfio_file_set_kvm(struct file *file, struct kvm *kvm)
> +static void kvm_vfio_file_set_kvm(struct file *file, struct file *kvm)
>  {
> -       void (*fn)(struct file *file, struct kvm *kvm);
> +       void (*fn)(struct file *file, struct file *kvm);
>
>         fn = symbol_get(vfio_file_set_kvm);
>         if (!fn)
> @@ -175,7 +175,7 @@ static int kvm_vfio_file_add(struct kvm_device *dev, unsigned int fd)
>         kvf->file = get_file(filp);
>         list_add_tail(&kvf->node, &kv->file_list);
>
> -       kvm_vfio_file_set_kvm(kvf->file, dev->kvm);
> +       kvm_vfio_file_set_kvm(kvf->file, dev->kvm_file);
>         kvm_vfio_update_coherency(dev);
>
>  out_unlock:
>
> base-commit: df83746075778958954aa0460cca55f4b3fc9c02
> --
>

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

[Sean Christopherson]

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.

diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index 9f9acb66cc1e..2d9bce646136 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -30,14 +30,15 @@ struct kvm_vfio_file {
 };
 
 struct kvm_vfio {
+       struct file *kvm_file;
        struct list_head file_list;
        struct mutex lock;
        bool noncoherent;
 };
 
-static void kvm_vfio_file_set_kvm(struct file *file, struct kvm *kvm)
+static void kvm_vfio_file_set_kvm(struct file *file, struct file *kvm)
 {
-       void (*fn)(struct file *file, struct kvm *kvm);
+       void (*fn)(struct file *file, struct file *kvm);
 
        fn = symbol_get(vfio_file_set_kvm);
        if (!fn)
@@ -175,7 +176,7 @@ static int kvm_vfio_file_add(struct kvm_device *dev, unsigned int fd)
        kvf->file = get_file(filp);
        list_add_tail(&kvf->node, &kv->file_list);
 
-       kvm_vfio_file_set_kvm(kvf->file, dev->kvm);
+       kvm_vfio_file_set_kvm(kvf->file, kv->kvm_file);
        kvm_vfio_update_coherency(dev);
 
 out_unlock:
@@ -372,6 +373,7 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
        mutex_init(&kv->lock);
 
        dev->private = kv;
+       kv->kvm_file = file;
 
        return 0;
 }

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

[Jason Gunthorpe]

On Tue, Apr 07, 2026 at 08:01:05PM +0200, 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.

Why should VFIO have to deal with this? If it has a refcounted struct
kvm then it is kvm's problem to make sure that module doesn't
disappear while the refounct is valid..

I would expect this to happen naturally because the file_operations
are still live and they will hold the module refcount.

Jason

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

[Jason Gunthorpe]

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

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

[Dan Williams]

Sean Christopherson wrote:
> +Dan

+Yilun

[..]
> I added Dan because the PCI TSM stuff is picking up "struct kvm *kvm" references,
> and I want to head that off too, i.e. have it use the file approach instead of
> whatever it plans on doing (can't tell from the code, because there are no users).

The PCI TSM *reference* for 'struct kvm *' will be inherited from
vfio/iommufd. However, the TSM driver needs some context to manipulate
the VM. For example, TDX effectively needs:

    to_kvm_tdx(kvm)->td.tdr_page

...for operations like TDH.TDI.CREATE that sets up the context for the
privately assigned device.

It could follow the example of arch/x86/kvm/mmu/page_track.c and do:

    kvm_tdx_tdi_create(struct file *kvm, ...)

...and use file_to_kvm() for that limited helper that does not need to expose
'struct kvm_tdx' outside of arch/x86/.