Linux s390 Architecture development
 help / color / mirror / Atom feed
* [PATCH v2] vfio: Use file-based reference counting for KVM
@ 2026-09-03  8:16 Steffen Eiden
  2026-09-03  8:42 ` sashiko-bot
  2026-09-03 17:20 ` Sean Christopherson
  0 siblings, 2 replies; 4+ messages in thread
From: Steffen Eiden @ 2026-09-03  8:16 UTC (permalink / raw)
  To: kvm, linux-kernel, linux-s390, x86
  Cc: Alex Williamson, Alexander Gordeev, Andreas Grapentin,
	Borislav Petkov, Christian Borntraeger, Claudio Imbrenda,
	Dave Hansen, Eric Farman, Farhan Ali, H. Peter Anvin, Halil Pasic,
	Harald Freudenberger, Heiko Carstens, Holger Dengler, Ingo Molnar,
	Janosch Frank, Jason Herne, Matthew Rosato, Paolo Bonzini,
	Sean Christopherson, Sven Schnelle, Thomas Gleixner, Tony Krowiak,
	Vasily Gorbik, Steffen Eiden, Jason Gunthorpe

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. Remove the put_kvm field in
struct vfio_device as is it no longer used.

Pass struct file pointers instead of struct kvm pointers throughout the
VFIO-KVM interface. This leverages the kernel's existing file reference
counting mechanism via get_file()/get_file_active() and fput(),
eliminating the need for manual module reference tracking. The
file->private_data field provides access to the underlying struct kvm
when needed.

group->kvm and df->kvm hold a reference of their own, taken when the
pointer is stored and dropped when it is overwritten or cleared. They
have to. The kvm-vfio device fd holds a VM reference of its own, so the
VM file can be closed and released while the kvm-vfio device is still
alive and still pointing at it.

kvm->file itself carries no reference, so that it does not pin the VM.
It is only ever read with get_file_active(), which is safe because
kvm_vm_release() clears it, i.e. before the struct file is freed.

This simplifies the code and removes all remaining externally exported
symbols for KVM, paving the path for a second concurrent KVM module.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
---
This is a spin-off for the arm-on-s390 series [1] to address all the
relevant persons w/o polluting their mailbox.

The motivation of this patch is to remove all exported symbols from KVM
to other modules. This patch helps us having multiple KVMs running in
parallel. The arm on s390 series creates a second kvm module (kvm-arm64)
for hosting arm guests on s390 with HW acceleration. For this we need
two concurrent KVM modules. One for s390 (native) and one for arm64
guests.

Steffen

[1] https://lore.kernel.org/all/20260831144802.834315-1-seiden@linux.ibm.com/

Changes in v2:
- Rebase on 7.3-rc1+ and resolve conflict with 9f240376d034 ("s390/pci: Store
  PCI error information for passthrough devices"),
- Fix stub prototype for vfio_group_set_kvm for !CONFIG_VFIO_GROUP
- Link to v1:   https://lore.kernel.org/r/20260812-vfio-v1-1-5cfe0b1fa4e7@linux.ibm.com
---
 arch/x86/include/asm/kvm_page_track.h |  8 ++---
 arch/x86/kvm/Makefile                 |  5 +--
 arch/x86/kvm/mmu/page_track.c         | 20 +++++++-----
 drivers/s390/crypto/vfio_ap_ops.c     | 20 ++++++++----
 drivers/vfio/group.c                  | 11 ++++++-
 drivers/vfio/pci/vfio_pci_zdev.c      |  8 ++++-
 drivers/vfio/vfio.h                   | 12 ++++----
 drivers/vfio/vfio_main.c              | 57 +++++++++++------------------------
 include/linux/kvm_host.h              |  8 +++++
 include/linux/vfio.h                  |  5 ++-
 virt/kvm/kvm_main.c                   | 20 ++++++++++--
 virt/kvm/vfio.c                       | 13 +++++---
 12 files changed, 108 insertions(+), 79 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/Makefile b/arch/x86/kvm/Makefile
index 0474604ab8a1..96f89d6f7e6a 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -60,10 +60,7 @@ exports_grep_trailer := --include='*.[ch]' -nrw $(srctree)/virt/kvm $(srctree)/a
 			grep -v -e kvm_page_track_register_notifier \
 				-e kvm_page_track_unregister_notifier \
 				-e kvm_write_track_add_gfn \
-				-e kvm_write_track_remove_gfn \
-				-e kvm_get_kvm \
-				-e kvm_get_kvm_safe \
-				-e kvm_put_kvm
+				-e kvm_write_track_remove_gfn
 
 # Force grep to emit a goofy group separator that can in turn be replaced with
 # the above newline macro (newlines in Make are a nightmare).  Note, grep only
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 7e8195a311bb..98f133e4f0b3 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -237,10 +237,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)
@@ -252,7 +253,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;
 
@@ -267,10 +268,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;
 
@@ -279,7 +281,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);
 
@@ -336,11 +338,12 @@ void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot)
  * add guest page to the tracking pool so that corresponding access on that
  * page will be intercepted.
  *
- * @kvm: the guest instance we are interested in.
+ * @file: the VM file of 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;
 
@@ -366,11 +369,12 @@ EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
  * remove the guest page from the tracking pool which stops the interception
  * of corresponding access on that page.
  *
- * @kvm: the guest instance we are interested in.
+ * @file: the VM file of 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/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 940c0ff668be..f160e1945fbc 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1822,17 +1822,27 @@ static const struct attribute_group *vfio_ap_mdev_attr_groups[] = {
 
 /**
  * vfio_ap_mdev_set_kvm - sets all data for @matrix_mdev that are needed
- * to manage AP resources for the guest whose state is represented by @kvm
+ * to manage AP resources for the guest whose state is represented by
+ * @kvm_file
  *
  * @matrix_mdev: a mediated matrix device
- * @kvm: reference to KVM instance
+ * @kvm_file: the KVM VM file this vfio device is associated with
  *
- * Return: 0 if no other mediated matrix device has a reference to @kvm;
+ * Return: 0 if no other mediated matrix device has a reference to the VM;
  * otherwise, returns an -EPERM.
  */
 static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
-				struct kvm *kvm)
+				struct file *kvm_file)
 {
+	struct kvm *kvm;
+
+	if (!kvm_file)
+		return -ENOENT;
+
+	kvm = kvm_file->private_data;
+	if (!kvm)
+		return -ENOENT;
+
 	if (kvm->arch.crypto.crycbd) {
 		get_update_locks_for_kvm(kvm);
 		if (kvm->arch.crypto.pqap_hook) {
@@ -1841,7 +1851,6 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
 		}
 		kvm->arch.crypto.pqap_hook = &matrix_mdev->pqap_hook;
 
-		kvm_get_kvm(kvm);
 		matrix_mdev->kvm = kvm;
 		vfio_ap_mdev_update_guest_apcb(matrix_mdev);
 		release_update_locks_for_kvm(kvm);
@@ -1894,7 +1903,6 @@ static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev)
 		matrix_mdev->kvm = NULL;
 
 		release_update_locks_for_kvm(kvm);
-		kvm_put_kvm(kvm);
 	}
 }
 
diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index b2299e5bc6df..5bf8cbdff377 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -860,11 +860,20 @@ 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)
 {
+	struct file *old;
+
+	if (kvm)
+		get_file(kvm);
+
 	spin_lock(&group->kvm_ref_lock);
+	old = group->kvm;
 	group->kvm = kvm;
 	spin_unlock(&group->kvm_ref_lock);
+
+	if (old)
+		fput(old);
 }
 
 /**
diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
index f47f36314a1c..045132e90bfe 100644
--- a/drivers/vfio/pci/vfio_pci_zdev.c
+++ b/drivers/vfio/pci/vfio_pci_zdev.c
@@ -184,6 +184,7 @@ int vfio_pci_zdev_feature_err(struct vfio_device *device, u32 flags,
 int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev)
 {
 	struct zpci_dev *zdev = to_zpci(vdev->pdev);
+	struct kvm *kvm;
 	int ret;
 
 	if (!zdev)
@@ -195,9 +196,14 @@ int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev)
 		return 0;
 
 	ret = -ENOENT;
+	kvm = vdev->vdev.kvm->private_data;
+	if (!kvm)
+		goto recovery;
+
 	if (zpci_kvm_hook.kvm_register)
-		ret = zpci_kvm_hook.kvm_register(zdev, vdev->vdev.kvm);
+		ret = zpci_kvm_hook.kvm_register(zdev, kvm);
 
+recovery:
 	if (ret)
 		zpci_stop_mediated_recovery(zdev);
 
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index 7728bc99b63d..9b619951a5d0 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 iommufd_ctx		*iommufd;
 	spinlock_t			kvm_ref_lock;
@@ -107,7 +107,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);
@@ -165,7 +165,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)
 {
 }
 
@@ -429,11 +429,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 423ead48aafe..ed96acfa8635 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -472,36 +472,14 @@ 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)
 		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;
+	device->kvm = get_file(kvm);
 }
 
 void vfio_device_put_kvm(struct vfio_device *device)
@@ -511,14 +489,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
@@ -1544,9 +1515,13 @@ 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;
+	struct file *old;
+
+	if (kvm)
+		get_file(kvm);
 
 	/*
 	 * The kvm is first recorded in the vfio_device_file, and will
@@ -1554,28 +1529,32 @@ static void vfio_device_file_set_kvm(struct file *file, struct kvm *kvm)
 	 * iommufd successfully in the vfio device cdev path.
 	 */
 	spin_lock(&df->kvm_ref_lock);
+	old = df->kvm;
 	df->kvm = kvm;
 	spin_unlock(&df->kvm_ref_lock);
+
+	if (old)
+		fput(old);
 }
 
 /**
  * vfio_file_set_kvm - Link a kvm with VFIO drivers
- * @file: VFIO group file or VFIO device file
- * @kvm: KVM to link
+ * @vfio_file: VFIO group file or VFIO device file
+ * @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 *vfio_file, struct file *kvm)
 {
 	struct vfio_group *group;
 
-	group = vfio_group_from_file(file);
+	group = vfio_group_from_file(vfio_file);
 	if (group)
 		vfio_group_set_kvm(group, kvm);
 
-	if (vfio_device_from_file(file))
-		vfio_device_file_set_kvm(file, kvm);
+	if (vfio_device_from_file(vfio_file))
+		vfio_device_file_set_kvm(vfio_file, kvm);
 }
 EXPORT_SYMBOL_GPL(vfio_file_set_kvm);
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 03bfc92864b6..bfecce1b5955 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -784,6 +784,13 @@ struct kvm {
 	 * kvm_swap_active_memslots().
 	 */
 	struct mutex slots_arch_lock;
+	/*
+	 * Back-reference to the VM file for subsystems (e.g., VFIO). Holds no
+	 * reference to avoid pinning the VM. Access via get_file_active(&kvm->file)
+	 * only—safe because cleared in kvm_vm_release() before file freed. Callers
+	 * storing the result must take their own reference via get_file()
+	 */
+	struct file *file;
 	struct mm_struct *mm; /* userspace tied to this vm */
 	unsigned long nr_memslot_pages;
 	/* The two memslot sets - active and inactive (per address space) */
@@ -1082,6 +1089,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)
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 45f08986359e..0cc91c6f96d2 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -54,7 +54,7 @@ struct vfio_device {
 	struct list_head dev_set_list;
 	unsigned int migration_flags;
 	u8 precopy_info_v2;
-	struct kvm *kvm;
+	struct file *kvm;
 
 	/* Members below here are private, not for driver use */
 	unsigned int index;
@@ -66,7 +66,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;
@@ -378,7 +377,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 *vfio_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 65eb26a0520d..34b4c43908b3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1314,7 +1314,7 @@ void kvm_get_kvm(struct kvm *kvm)
 {
 	refcount_inc(&kvm->users_count);
 }
-EXPORT_SYMBOL_GPL(kvm_get_kvm);
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_get_kvm);
 
 /*
  * Make sure the vm is not during destruction, which is a safe version of
@@ -1324,14 +1324,14 @@ bool kvm_get_kvm_safe(struct kvm *kvm)
 {
 	return refcount_inc_not_zero(&kvm->users_count);
 }
-EXPORT_SYMBOL_GPL(kvm_get_kvm_safe);
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_get_kvm_safe);
 
 void kvm_put_kvm(struct kvm *kvm)
 {
 	if (refcount_dec_and_test(&kvm->users_count))
 		kvm_destroy_vm(kvm);
 }
-EXPORT_SYMBOL_GPL(kvm_put_kvm);
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_put_kvm);
 
 /*
  * Used to put a reference that was taken on behalf of an object associated
@@ -1352,6 +1352,8 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
 
 	kvm_irqfd_release(kvm);
 
+	WRITE_ONCE(kvm->file, NULL);
+
 	kvm_put_kvm(kvm);
 	return 0;
 }
@@ -5496,6 +5498,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];
@@ -5527,6 +5538,9 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	 * cases it will be called by the final fput(file) and will take
 	 * care of doing kvm_put_kvm(kvm).
 	 */
+
+	kvm->file = file;
+
 	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
 
 	fd_install(fd, file);
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index 6cdc4e9a333a..19548a430942 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -35,15 +35,15 @@ 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 *vfio_file, struct file *kvm)
 {
-	void (*fn)(struct file *file, struct kvm *kvm);
+	void (*fn)(struct file *vfio_file, struct file *kvm);
 
 	fn = symbol_get(vfio_file_set_kvm);
 	if (!fn)
 		return;
 
-	fn(file, kvm);
+	fn(vfio_file, kvm);
 
 	symbol_put(vfio_file_set_kvm);
 }
@@ -144,6 +144,7 @@ static int kvm_vfio_file_add(struct kvm_device *dev, unsigned int fd)
 {
 	struct kvm_vfio *kv = dev->private;
 	struct kvm_vfio_file *kvf;
+	struct file *kvm_file __free(fput) = NULL;
 	struct file *filp __free(fput) = NULL;
 
 	filp = fget(fd);
@@ -154,6 +155,10 @@ static int kvm_vfio_file_add(struct kvm_device *dev, unsigned int fd)
 	if (!kvm_vfio_file_is_valid(filp))
 		return -EINVAL;
 
+	kvm_file = get_file_active(&dev->kvm->file);
+	if (!kvm_file)
+		return -ENOENT;
+
 	guard(mutex)(&kv->lock);
 
 	list_for_each_entry(kvf, &kv->file_list, node) {
@@ -168,7 +173,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, kvm_file);
 	kvm_vfio_update_coherency(dev);
 
 	return 0;

---
base-commit: 940de590b839f71d6dc846160534bf202401b8b7
change-id: 20260812-vfio-f9069d8ab545

Best regards,
-- 
Steffen Eiden <seiden@linux.ibm.com>


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] vfio: Use file-based reference counting for KVM
  2026-09-03  8:16 [PATCH v2] vfio: Use file-based reference counting for KVM Steffen Eiden
@ 2026-09-03  8:42 ` sashiko-bot
  2026-09-03 17:07   ` Sean Christopherson
  2026-09-03 17:20 ` Sean Christopherson
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-09-03  8:42 UTC (permalink / raw)
  To: Steffen Eiden
  Cc: Alexander Gordeev, Christian Borntraeger, kvm, Heiko Carstens,
	linux-s390, Vasily Gorbik

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Untyped `struct file *` for KVM VM leads to type confusion and out-of-bounds access in VFIO device drivers when multiple concurrent KVM modules are loaded.

Pre-existing issues:
- [High] Pre-existing bug in the cdev IOMMUFD binding path allows concurrent opens to leak the KVM file reference and prematurely clear `device->kvm`.
--

commit dd4fd68a7d7d9888bc552f388401d95d1f137c5f
Author: Steffen Eiden <seiden@linux.ibm.com>

vfio: Use file-based reference counting for KVM

This patch replaces manual module reference counting with file-based reference
counting for KVM integration in VFIO. It passes struct file pointers instead
of struct kvm pointers across the interface to leverage the kernel's existing
file reference counting mechanism. This removes exported symbols and supports
multiple concurrent KVM modules.

Link: https://lore.kernel.org/all/20260831144802.834315-1-seiden@linux.ibm.com/
Link: https://lore.kernel.org/r/20260812-vfio-v1-1-5cfe0b1fa4e7@linux.ibm.com

> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 940c0ff668bee..f160e1945fbce 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c

[ ... ]

> @@ -1822,17 +1822,27 @@ static const struct attribute_group *vfio_ap_mdev_attr_groups[] = {
>  
>  /**
>   * vfio_ap_mdev_set_kvm - sets all data for @matrix_mdev that are needed
> - * to manage AP resources for the guest whose state is represented by @kvm
> + * to manage AP resources for the guest whose state is represented by
> + * @kvm_file
>   *
>   * @matrix_mdev: a mediated matrix device
> - * @kvm: reference to KVM instance
> + * @kvm_file: the KVM VM file this vfio device is associated with
>   *
> - * Return: 0 if no other mediated matrix device has a reference to @kvm;
> + * Return: 0 if no other mediated matrix device has a reference to the VM;
>   * otherwise, returns an -EPERM.
>   */
>  static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
> -				struct kvm *kvm)
> +				struct file *kvm_file)
>  {
> +	struct kvm *kvm;
> +
> +	if (!kvm_file)
> +		return -ENOENT;
> +
> +	kvm = kvm_file->private_data;
> +	if (!kvm)
> +		return -ENOENT;
> +
>  	if (kvm->arch.crypto.crycbd) {

[Severity: Critical]
Does this code adequately verify that the file is an s390 KVM file before
casting its private_data in vfio_ap_mdev_set_kvm() inside
drivers/s390/crypto/vfio_ap_ops.c?

If a user instantiates a VM using a concurrent KVM-arm64 module and attaches
an s390-specific VFIO device to its KVM device, could this driver extract the
untyped private_data and mistakenly treat an arm64 struct kvm as an s390 one?

Because the memory layouts of struct kvm_arch differ across architectures,
would accessing s390-specific fields in the arch member (like
kvm->arch.crypto.pqap_hook) result in arbitrary memory corruption?

>  		get_update_locks_for_kvm(kvm);
>  		if (kvm->arch.crypto.pqap_hook) {

[ ... ]

> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
> index f47f36314a1cc..045132e90bfee 100644
> --- a/drivers/vfio/pci/vfio_pci_zdev.c
> +++ b/drivers/vfio/pci/vfio_pci_zdev.c
> @@ -184,6 +184,7 @@ int vfio_pci_zdev_feature_err(struct vfio_device *device, u32 flags,
>  int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev)
>  {
>  	struct zpci_dev *zdev = to_zpci(vdev->pdev);
> +	struct kvm *kvm;
>  	int ret;
>  
>  	if (!zdev)
> @@ -195,9 +196,14 @@ int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev)
>  		return 0;
>  
>  	ret = -ENOENT;
> +	kvm = vdev->vdev.kvm->private_data;
> +	if (!kvm)
> +		goto recovery;
> +
>  	if (zpci_kvm_hook.kvm_register)
> -		ret = zpci_kvm_hook.kvm_register(zdev, vdev->vdev.kvm);
> +		ret = zpci_kvm_hook.kvm_register(zdev, kvm);

[Severity: Critical]
Is it possible for a similar regression to occur here in
vfio_pci_zdev_open_device() within drivers/vfio/pci/vfio_pci_zdev.c?

If an arm64 VM file is passed via vdev->vdev.kvm, passing it to
zpci_kvm_hook.kvm_register() could potentially cause type confusion in the
host kernel.

>  
> +recovery:
>  	if (ret)
>  		zpci_stop_mediated_recovery(zdev);

[ ... ]

> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 423ead48aafe3..ed96acfa86358 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c

[ ... ]

> @@ -511,14 +489,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;

[Severity: High]
This is a pre-existing issue, but does this error path leave the first active
file descriptor with a NULL KVM pointer during concurrent cdev IOMMUFD binds?

Looking at vfio_df_ioctl_bind_iommufd() in drivers/vfio/device_cdev.c, if a
user opens a VFIO cdev twice, obtaining two vfio_device_file (df) instances:

1. bind_iommufd on the first df succeeds, setting device->kvm and
device->open_count.

2. bind_iommufd on the second df bypasses the df->access_granted check and
unconditionally calls vfio_df_get_kvm_safe(df). If a KVM file was linked to
the second df, this overwrites device->kvm with the second file's reference
without releasing the first, causing a resource leak.

3. The subsequent vfio_df_open(df) fails because device->open_count != 0.

4. The error path then calls vfio_device_put_kvm() in
drivers/vfio/vfio_main.c, which drops the reference and sets
device->kvm = NULL.

Could this sequence leave the first df operating with a NULL device->kvm,
leading to potential NULL pointer dereferences in drivers?

>  }
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-vfio-v2-1-ef4cd4190ae7@linux.ibm.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] vfio: Use file-based reference counting for KVM
  2026-09-03  8:42 ` sashiko-bot
@ 2026-09-03 17:07   ` Sean Christopherson
  0 siblings, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2026-09-03 17:07 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Steffen Eiden, Alexander Gordeev, Christian Borntraeger, kvm,
	Heiko Carstens, linux-s390, Vasily Gorbik

On Thu, Sep 03, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> > index 940c0ff668bee..f160e1945fbce 100644
> > --- a/drivers/s390/crypto/vfio_ap_ops.c
> > +++ b/drivers/s390/crypto/vfio_ap_ops.c
> 
> [ ... ]
> 
> > @@ -1822,17 +1822,27 @@ static const struct attribute_group *vfio_ap_mdev_attr_groups[] = {
> >  
> >  /**
> >   * vfio_ap_mdev_set_kvm - sets all data for @matrix_mdev that are needed
> > - * to manage AP resources for the guest whose state is represented by @kvm
> > + * to manage AP resources for the guest whose state is represented by
> > + * @kvm_file
> >   *
> >   * @matrix_mdev: a mediated matrix device
> > - * @kvm: reference to KVM instance
> > + * @kvm_file: the KVM VM file this vfio device is associated with
> >   *
> > - * Return: 0 if no other mediated matrix device has a reference to @kvm;
> > + * Return: 0 if no other mediated matrix device has a reference to the VM;
> >   * otherwise, returns an -EPERM.
> >   */
> >  static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
> > -				struct kvm *kvm)
> > +				struct file *kvm_file)
> >  {
> > +	struct kvm *kvm;
> > +
> > +	if (!kvm_file)
> > +		return -ENOENT;
> > +
> > +	kvm = kvm_file->private_data;
> > +	if (!kvm)
> > +		return -ENOENT;
> > +
> >  	if (kvm->arch.crypto.crycbd) {
> 
> [Severity: Critical]
> Does this code adequately verify that the file is an s390 KVM file before
> casting its private_data in vfio_ap_mdev_set_kvm() inside
> drivers/s390/crypto/vfio_ap_ops.c?
> 
> If a user instantiates a VM using a concurrent KVM-arm64 module and attaches
> an s390-specific VFIO device to its KVM device, could this driver extract the
> untyped private_data and mistakenly treat an arm64 struct kvm as an s390 one?
> 
> Because the memory layouts of struct kvm_arch differ across architectures,
> would accessing s390-specific fields in the arch member (like
> kvm->arch.crypto.pqap_hook) result in arbitrary memory corruption?

This is effectively the same concern I raised[1] in the s390+arm64 series:

 : Side topic #2, this entire approach seems extremely brittle unless you make it
 : all but impossible for non-KVM code to get at KVM structure definitions.  Outside
 : of KVM, all compilation units will see the s390 version of KVM structures.  Which
 : is "fine", but obviously dangerous and IMO asking for maintenance issues down the
 : road.

I don't think we need to go to the super extreme lengths I proposed[2] back when
we were exploring multi-KVM on x86, but the direct dereference of ->private_data
is a huge red flag.

Given that external usage of "struct kvm" should be *super* rare, and IMO is
something we should actively discourage, I think we should make it opt-in.  Then
at the same time, define the API so that it's arch-specific, e.g. to yield
file_to_kvm_s390() so that drivers/s390/crypto/vfio_ap_ops.c can get exactly
what it wants.

Diff below, though it needs to be split into multiple patches (I'll respond with
more to the full patch).

[1] https://lore.kernel.org/all/aphLXQL1H2zYZgTi@google.com
[2] https://lore.kernel.org/all/20230916003118.2540661-1-seanjc@google.com

diff --git a/arch/s390/include/asm/kvm_host_s390.h b/arch/s390/include/asm/kvm_host_s390.h
index cd692f8fb764..8a7eed5847e1 100644
--- a/arch/s390/include/asm/kvm_host_s390.h
+++ b/arch/s390/include/asm/kvm_host_s390.h
@@ -27,6 +27,8 @@
 #include <asm/isc.h>
 #include <asm/guarded_storage.h>
 
+#define kvm_file_to_kvm_arch s390
+
 #define KVM_HAVE_MMU_RWLOCK
 #define KVM_MAX_VCPUS 255
 
diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
index 046a25c8fe4f..acc301f5e265 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -5,6 +5,9 @@
 #include <linux/kvm_types.h>
 
 #ifdef CONFIG_KVM_EXTERNAL_WRITE_TRACKING
+
+#define kvm_file_to_kvm_arch x86
+
 /*
  * The notifier represented by @kvm_page_track_notifier_node is linked into
  * the head which will be notified when guest is triggering the track event.
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 96f89d6f7e6a..c6bf463f5032 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -60,7 +60,8 @@ exports_grep_trailer := --include='*.[ch]' -nrw $(srctree)/virt/kvm $(srctree)/a
 			grep -v -e kvm_page_track_register_notifier \
 				-e kvm_page_track_unregister_notifier \
 				-e kvm_write_track_add_gfn \
-				-e kvm_write_track_remove_gfn
+				-e kvm_write_track_remove_gfn \
+				-e kvm_file_to_kvm_fn
 
 # Force grep to emit a goofy group separator that can in turn be replaced with
 # the above newline macro (newlines in Make are a nightmare).  Note, grep only
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
index 98f133e4f0b3..ebf941b6378d 100644
--- a/arch/x86/kvm/mmu/page_track.c
+++ b/arch/x86/kvm/mmu/page_track.c
@@ -241,7 +241,7 @@ 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);
+	struct kvm *kvm = file_to_kvm_x86(file);
 	int r;
 
 	if (!kvm || kvm->mm != current->mm)
@@ -272,7 +272,7 @@ 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);
+	struct kvm *kvm = file_to_kvm_x86(file);
 
 	head = &kvm->arch.track_notifier_head;
 
@@ -343,7 +343,7 @@ void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot)
  */
 int kvm_write_track_add_gfn(struct file *file, gfn_t gfn)
 {
-	struct kvm *kvm = file_to_kvm(file);
+	struct kvm *kvm = file_to_kvm_x86(file);
 	struct kvm_memory_slot *slot;
 	int idx;
 
@@ -374,7 +374,7 @@ EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
  */
 int kvm_write_track_remove_gfn(struct file *file, gfn_t gfn)
 {
-	struct kvm *kvm = file_to_kvm(file);
+	struct kvm *kvm = file_to_kvm_x86(file);
 	struct kvm_memory_slot *slot;
 	int idx;
 
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 5705723f1f41..6b0eacde4f06 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2145,10 +2145,10 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd)
 	if (fd_empty(f))
 		return -EBADF;
 
-	if (!file_is_kvm(fd_file(f)))
+	source_kvm = file_to_kvm_x86(fd_file(f));
+	if (!source_kvm)
 		return -EBADF;
 
-	source_kvm = fd_file(f)->private_data;
 	ret = sev_lock_two_vms(kvm, source_kvm);
 	if (ret)
 		return ret;
@@ -2866,10 +2866,10 @@ int sev_vm_copy_enc_context_from(struct kvm *kvm, unsigned int source_fd)
 	if (fd_empty(f))
 		return -EBADF;
 
-	if (!file_is_kvm(fd_file(f)))
+	source_kvm = file_to_kvm_x86(fd_file(f));
+	if (!source_kvm)
 		return -EBADF;
 
-	source_kvm = fd_file(f)->private_data;
 	ret = sev_lock_two_vms(kvm, source_kvm);
 	if (ret)
 		return ret;
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index f160e1945fbc..96bcc47b824f 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1834,12 +1834,8 @@ static const struct attribute_group *vfio_ap_mdev_attr_groups[] = {
 static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
 				struct file *kvm_file)
 {
-	struct kvm *kvm;
+	struct kvm *kvm = file_to_kvm_s390(kvm_file);
 
-	if (!kvm_file)
-		return -ENOENT;
-
-	kvm = kvm_file->private_data;
 	if (!kvm)
 		return -ENOENT;
 
diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
index 045132e90bfe..332bc66db55b 100644
--- a/drivers/vfio/pci/vfio_pci_zdev.c
+++ b/drivers/vfio/pci/vfio_pci_zdev.c
@@ -196,7 +196,7 @@ int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev)
 		return 0;
 
 	ret = -ENOENT;
-	kvm = vdev->vdev.kvm->private_data;
+	kvm = file_to_kvm_s390(vdev->vdev.kvm);
 	if (!kvm)
 		goto recovery;
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index bfecce1b5955..09e327843e6c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1088,8 +1088,11 @@ 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);
-struct kvm *file_to_kvm(struct file *file);
+#ifdef kvm_file_to_kvm_arch
+#define kvm_file_to_kvm_fn CONCATENATE(file_to_kvm_, kvm_file_to_kvm_arch)
+struct kvm *kvm_file_to_kvm_fn(struct file *file);
+#endif
+
 void kvm_put_kvm_no_destroy(struct kvm *kvm);
 
 static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm, int as_id)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 34b4c43908b3..9557026299d3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5492,20 +5492,16 @@ static struct file_operations kvm_vm_fops = {
 	KVM_COMPAT(kvm_vm_compat_ioctl),
 };
 
-bool file_is_kvm(struct file *file)
+#ifdef kvm_file_to_kvm_fn
+struct kvm *kvm_file_to_kvm_fn(struct file *file)
 {
-	return file && file->f_op == &kvm_vm_fops;
-}
-EXPORT_SYMBOL_FOR_KVM_INTERNAL(file_is_kvm);
-
-struct kvm *file_to_kvm(struct file *file)
-{
-	if (!file_is_kvm(file))
+	if (!file || file->f_op != &kvm_vm_fops)
 		return NULL;
 
 	return file->private_data;
 }
-EXPORT_SYMBOL_FOR_KVM_INTERNAL(file_to_kvm);
+EXPORT_SYMBOL_GPL(kvm_file_to_kvm_fn);
+#endif
 
 static int kvm_dev_ioctl_create_vm(unsigned long type)
 {


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] vfio: Use file-based reference counting for KVM
  2026-09-03  8:16 [PATCH v2] vfio: Use file-based reference counting for KVM Steffen Eiden
  2026-09-03  8:42 ` sashiko-bot
@ 2026-09-03 17:20 ` Sean Christopherson
  1 sibling, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2026-09-03 17:20 UTC (permalink / raw)
  To: Steffen Eiden
  Cc: kvm, linux-kernel, linux-s390, x86, Alex Williamson,
	Alexander Gordeev, Andreas Grapentin, Borislav Petkov,
	Christian Borntraeger, Claudio Imbrenda, Dave Hansen, Eric Farman,
	Farhan Ali, H. Peter Anvin, Halil Pasic, Harald Freudenberger,
	Heiko Carstens, Holger Dengler, Ingo Molnar, Janosch Frank,
	Jason Herne, Matthew Rosato, Paolo Bonzini, Sven Schnelle,
	Thomas Gleixner, Tony Krowiak, Vasily Gorbik, Jason Gunthorpe

On Thu, Sep 03, 2026, Steffen Eiden wrote:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 65eb26a0520d..34b4c43908b3 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1314,7 +1314,7 @@ void kvm_get_kvm(struct kvm *kvm)
>  {
>  	refcount_inc(&kvm->users_count);
>  }
> -EXPORT_SYMBOL_GPL(kvm_get_kvm);
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_get_kvm);
>  
>  /*
>   * Make sure the vm is not during destruction, which is a safe version of
> @@ -1324,14 +1324,14 @@ bool kvm_get_kvm_safe(struct kvm *kvm)
>  {
>  	return refcount_inc_not_zero(&kvm->users_count);
>  }
> -EXPORT_SYMBOL_GPL(kvm_get_kvm_safe);
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_get_kvm_safe);
>  
>  void kvm_put_kvm(struct kvm *kvm)
>  {
>  	if (refcount_dec_and_test(&kvm->users_count))
>  		kvm_destroy_vm(kvm);
>  }
> -EXPORT_SYMBOL_GPL(kvm_put_kvm);
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_put_kvm);

Please isolate the export changes.  They don't *need* to happen at the same time
as the VFIO changes, i.e. can be done on top.

>  /*
>   * Used to put a reference that was taken on behalf of an object associated
> @@ -1352,6 +1352,8 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
>  
>  	kvm_irqfd_release(kvm);
>  
> +	WRITE_ONCE(kvm->file, NULL);

Please move tracking the file in "struct kvm" to its own patch as well.

> +
>  	kvm_put_kvm(kvm);
>  	return 0;
>  }
> @@ -5496,6 +5498,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))

file_is_kvm() should exist at the end of this series.  The only reason to ever
use file_is_kvm() is in advance of getting at "struct kvm", i.e. this should be
open coded in file_to_kvm().  Though as I suggested in the other subtread, it
woudl be kvm_file_to_kvm_fn(), i.e. kvm_file_to_kvm_x86() or kvm_file_to_kvm_s390().

And those changes can and should be done as prep work, i.e. in separate patches,
e.g. to end up with something like:

  1. Add kvm_file_to_kvm_fn() and use it on x86, i.e. replace the use of
     file_is_kvm() in arch/x86/kvm/svm/sev.c.
  2. Add kvm->file tracking.
  3. Switch VFIO to tracking the file.
  4. s/EXPORT_SYMBOL_GPL/EXPORT_SYMBOL_FOR_KVM_INTERNAL on the get/put APIs.
  5. Enable kvm_file_to_kvm_fn() on s390 and use kvm_file_to_kvm_s390() in the
     relevant code to prepare for s390+arm64.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-03 17:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03  8:16 [PATCH v2] vfio: Use file-based reference counting for KVM Steffen Eiden
2026-09-03  8:42 ` sashiko-bot
2026-09-03 17:07   ` Sean Christopherson
2026-09-03 17:20 ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox