[PATCH 0/5] KVM: Clean up debugfs+stats init/destroy

[PATCH 0/5] KVM: Clean up debugfs+stats init/destroy

[Oliver Upton]

The way KVM handles debugfs initialization and destruction is somewhat
sloppy. Although the debugfs + stats bits get initialized *after*
kvm_create_vm(), they are torn down from kvm_destroy_vm(). And yes,
there is a window where we could theoretically destroy a VM before
debugfs is ever instantiated.

This series does away with the mess by coupling debugfs+stats to the
overall VM create/destroy pattern. We already fail the VM creation if
kvm_create_vm_debugfs() fails, so there really isn't a need to do these
separately in the first place.

The first two patches hoist some unrelated tidbits of stats state into
the debugfs constructors just so its all handled under one roof.

The second two patches realize the the intention of the series, changing
the initialization order so we can get an FD for the vm early.

Lastly, patch 5 is essentially a revert of Sean's proposed fix [1], but
I deliberately am not proposing a revert outright, in case alarm bells
go off that a stable patch got reverted (it is correct).

Applies to the following commit w/ the addition of Sean's patch:

  fb649bda6f56 ("Merge tag 'block-5.18-2022-04-15' of git://git.kernel.dk/linux-block")

Tested (I promise) on an Intel Skylake machine with KVM selftests. I
poked around in debugfs to make sure there were no stragglers, and I ran
the reproducer for [1] to confirm the null ptr deref wasn't introduced
yet again.

Oliver Upton (5):
  KVM: Shove vm stats_id init into kvm_create_vm_debugfs()
  KVM: Shove vcpu stats_id init into kvm_vcpu_create_debugfs()
  KVM: Get an fd before creating the VM
  KVM: Actually create debugfs in kvm_create_vm()
  KVM: Hoist debugfs_dentry init to kvm_create_vm_debugfs() (again)

 virt/kvm/kvm_main.c | 92 ++++++++++++++++++++++-----------------------
 1 file changed, 46 insertions(+), 46 deletions(-)

-- 
2.36.0.rc0.470.gd361397f0d-goog

[PATCH 1/5] KVM: Shove vm stats_id init into kvm_create_vm_debugfs()

[Oliver Upton]

The field is only ever used for debugfs; put the initialization where
it belongs.

Signed-off-by: Oliver Upton <oupton@google.com>
---
 virt/kvm/kvm_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d292c4397579..ec9c6aad041b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -955,6 +955,9 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
 	int kvm_debugfs_num_entries = kvm_vm_stats_header.num_desc +
 				      kvm_vcpu_stats_header.num_desc;
 
+	snprintf(kvm->stats_id, sizeof(kvm->stats_id),
+			"kvm-%d", task_pid_nr(current));
+
 	if (!debugfs_initialized())
 		return 0;
 
@@ -4765,9 +4768,6 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	if (r < 0)
 		goto put_kvm;
 
-	snprintf(kvm->stats_id, sizeof(kvm->stats_id),
-			"kvm-%d", task_pid_nr(current));
-
 	file = anon_inode_getfile("kvm-vm", &kvm_vm_fops, kvm, O_RDWR);
 	if (IS_ERR(file)) {
 		put_unused_fd(r);
-- 
2.36.0.rc0.470.gd361397f0d-goog

[PATCH 2/5] KVM: Shove vcpu stats_id init into kvm_vcpu_create_debugfs()

[Oliver Upton]

Again, the stats_id is only ever used by the stats code; put it where it
belongs with the rest of the stats initialization.

No functional change intended.

Signed-off-by: Oliver Upton <oupton@google.com>
---
 virt/kvm/kvm_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ec9c6aad041b..aaf8de62b897 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3711,6 +3711,10 @@ static void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
 	struct dentry *debugfs_dentry;
 	char dir_name[ITOA_MAX_LEN * 2];
 
+	/* Fill the stats id string for the vcpu */
+	snprintf(vcpu->stats_id, sizeof(vcpu->stats_id), "kvm-%d/vcpu-%d",
+		 task_pid_nr(current), vcpu->vcpu_id);
+
 	if (!debugfs_initialized())
 		return;
 
@@ -3786,10 +3790,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 	if (r)
 		goto unlock_vcpu_destroy;
 
-	/* Fill the stats id string for the vcpu */
-	snprintf(vcpu->stats_id, sizeof(vcpu->stats_id), "kvm-%d/vcpu-%d",
-		 task_pid_nr(current), id);
-
 	/* Now it's all set up, let userspace reach it */
 	kvm_get_kvm(kvm);
 	r = create_vcpu_fd(vcpu);
-- 
2.36.0.rc0.470.gd361397f0d-goog

[PATCH 3/5] KVM: Get an fd before creating the VM

[Oliver Upton]

Hoist fd init to the very beginning of kvm_create_vm() so we can make
use of it in other init routines.

Signed-off-by: Oliver Upton <oupton@google.com>
---
 virt/kvm/kvm_main.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index aaf8de62b897..1abbc6b07c19 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4752,25 +4752,27 @@ EXPORT_SYMBOL_GPL(file_is_kvm);
 
 static int kvm_dev_ioctl_create_vm(unsigned long type)
 {
-	int r;
+	int r, fd;
 	struct kvm *kvm;
 	struct file *file;
 
+	fd = get_unused_fd_flags(O_CLOEXEC);
+	if (fd < 0)
+		return fd;
+
 	kvm = kvm_create_vm(type);
-	if (IS_ERR(kvm))
-		return PTR_ERR(kvm);
+	if (IS_ERR(kvm)) {
+		r = PTR_ERR(kvm);
+		goto put_fd;
+	}
+
 #ifdef CONFIG_KVM_MMIO
 	r = kvm_coalesced_mmio_init(kvm);
 	if (r < 0)
 		goto put_kvm;
 #endif
-	r = get_unused_fd_flags(O_CLOEXEC);
-	if (r < 0)
-		goto put_kvm;
-
 	file = anon_inode_getfile("kvm-vm", &kvm_vm_fops, kvm, O_RDWR);
 	if (IS_ERR(file)) {
-		put_unused_fd(r);
 		r = PTR_ERR(file);
 		goto put_kvm;
 	}
@@ -4782,17 +4784,19 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	 * care of doing kvm_put_kvm(kvm).
 	 */
 	if (kvm_create_vm_debugfs(kvm, r) < 0) {
-		put_unused_fd(r);
 		fput(file);
-		return -ENOMEM;
+		r = -ENOMEM;
+		goto put_fd;
 	}
 	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
 
-	fd_install(r, file);
-	return r;
+	fd_install(fd, file);
+	return fd;
 
 put_kvm:
 	kvm_put_kvm(kvm);
+put_fd:
+	put_unused_fd(fd);
 	return r;
 }
 
-- 
2.36.0.rc0.470.gd361397f0d-goog

[PATCH 4/5] KVM: Actually create debugfs in kvm_create_vm()

[Oliver Upton]

Doing debugfs creation after vm creation leaves things in a
quasi-initialized state for a while. This is further complicated by the
fact that we tear down debugfs from kvm_destroy_vm(). Align debugfs and
stats init/destroy with the vm init/destroy pattern to avoid any
headaches.

Note the fix for a benign mistake in error handling for calls to
kvm_arch_create_vm_debugfs() rolled in. Since all implementations of
the function return 0 unconditionally it isn't actually a bug at
the moment.

Lastly, tear down debugfs/stats data in the kvm_create_vm_debugfs()
error path. Previously it was safe to assume that kvm_destroy_vm() would
take out the garbage, that is no longer the case.

Signed-off-by: Oliver Upton <oupton@google.com>
---
 virt/kvm/kvm_main.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1abbc6b07c19..54793de42d14 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -951,7 +951,7 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
 	char dir_name[ITOA_MAX_LEN * 2];
 	struct kvm_stat_data *stat_data;
 	const struct _kvm_stats_desc *pdesc;
-	int i, ret;
+	int i, ret = -ENOMEM;
 	int kvm_debugfs_num_entries = kvm_vm_stats_header.num_desc +
 				      kvm_vcpu_stats_header.num_desc;
 
@@ -980,13 +980,13 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
 					 sizeof(*kvm->debugfs_stat_data),
 					 GFP_KERNEL_ACCOUNT);
 	if (!kvm->debugfs_stat_data)
-		return -ENOMEM;
+		goto out_err;
 
 	for (i = 0; i < kvm_vm_stats_header.num_desc; ++i) {
 		pdesc = &kvm_vm_stats_desc[i];
 		stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL_ACCOUNT);
 		if (!stat_data)
-			return -ENOMEM;
+			goto out_err;
 
 		stat_data->kvm = kvm;
 		stat_data->desc = pdesc;
@@ -1001,7 +1001,7 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
 		pdesc = &kvm_vcpu_stats_desc[i];
 		stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL_ACCOUNT);
 		if (!stat_data)
-			return -ENOMEM;
+			goto out_err;
 
 		stat_data->kvm = kvm;
 		stat_data->desc = pdesc;
@@ -1013,12 +1013,13 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
 	}
 
 	ret = kvm_arch_create_vm_debugfs(kvm);
-	if (ret) {
-		kvm_destroy_vm_debugfs(kvm);
-		return i;
-	}
+	if (ret)
+		goto out_err;
 
 	return 0;
+out_err:
+	kvm_destroy_vm_debugfs(kvm);
+	return ret;
 }
 
 /*
@@ -1049,7 +1050,7 @@ int __weak kvm_arch_create_vm_debugfs(struct kvm *kvm)
 	return 0;
 }
 
-static struct kvm *kvm_create_vm(unsigned long type)
+static struct kvm *kvm_create_vm(unsigned long type, int fd)
 {
 	struct kvm *kvm = kvm_arch_alloc_vm();
 	struct kvm_memslots *slots;
@@ -1134,7 +1135,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
 
 	r = kvm_arch_post_init_vm(kvm);
 	if (r)
-		goto out_err;
+		goto out_err_mmu_notifier;
 
 	mutex_lock(&kvm_lock);
 	list_add(&kvm->vm_list, &vm_list);
@@ -1150,12 +1151,18 @@ static struct kvm *kvm_create_vm(unsigned long type)
 	 */
 	if (!try_module_get(kvm_chardev_ops.owner)) {
 		r = -ENODEV;
-		goto out_err;
+		goto out_err_mmu_notifier;
 	}
 
+	r = kvm_create_vm_debugfs(kvm, fd);
+	if (r)
+		goto out_err;
+
 	return kvm;
 
 out_err:
+	module_put(kvm_chardev_ops.owner);
+out_err_mmu_notifier:
 #if defined(CONFIG_MMU_NOTIFIER) && defined(KVM_ARCH_WANT_MMU_NOTIFIER)
 	if (kvm->mmu_notifier.ops)
 		mmu_notifier_unregister(&kvm->mmu_notifier, current->mm);
@@ -4760,7 +4767,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	if (fd < 0)
 		return fd;
 
-	kvm = kvm_create_vm(type);
+	kvm = kvm_create_vm(type, fd);
 	if (IS_ERR(kvm)) {
 		r = PTR_ERR(kvm);
 		goto put_fd;
@@ -4777,17 +4784,6 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 		goto put_kvm;
 	}
 
-	/*
-	 * Don't call kvm_put_kvm anymore at this point; file->f_op is
-	 * already set, with ->release() being kvm_vm_release().  In error
-	 * cases it will be called by the final fput(file) and will take
-	 * care of doing kvm_put_kvm(kvm).
-	 */
-	if (kvm_create_vm_debugfs(kvm, r) < 0) {
-		fput(file);
-		r = -ENOMEM;
-		goto put_fd;
-	}
 	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
 
 	fd_install(fd, file);
-- 
2.36.0.rc0.470.gd361397f0d-goog

[PATCH 5/5] KVM: Hoist debugfs_dentry init to kvm_create_vm_debugfs() (again)

[Oliver Upton]

Since KVM now sanely handles debugfs init/destroy w.r.t. the VM, it is
safe to hoist kvm_create_vm_debugfs() back into kvm_create_vm(). The
author of this commit remains bitter for having been burned by the old
wreck in commit a44a4cc1c969 ("KVM: Don't create VM debugfs files
outside of the VM directory").

Signed-off-by: Oliver Upton <oupton@google.com>
---
 virt/kvm/kvm_main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 54793de42d14..61e727c15c1a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -958,6 +958,12 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
 	snprintf(kvm->stats_id, sizeof(kvm->stats_id),
 			"kvm-%d", task_pid_nr(current));
 
+	/*
+	 * Force subsequent debugfs file creations to fail if the VM directory
+	 * is not created.
+	 */
+	kvm->debugfs_dentry = ERR_PTR(-ENOENT);
+
 	if (!debugfs_initialized())
 		return 0;
 
@@ -1079,12 +1085,6 @@ static struct kvm *kvm_create_vm(unsigned long type, int fd)
 
 	BUILD_BUG_ON(KVM_MEM_SLOTS_NUM > SHRT_MAX);
 
-	/*
-	 * Force subsequent debugfs file creations to fail if the VM directory
-	 * is not created (by kvm_create_vm_debugfs()).
-	 */
-	kvm->debugfs_dentry = ERR_PTR(-ENOENT);
-
 	if (init_srcu_struct(&kvm->srcu))
 		goto out_err_no_srcu;
 	if (init_srcu_struct(&kvm->irq_srcu))
-- 
2.36.0.rc0.470.gd361397f0d-goog

Re: [PATCH 0/5] KVM: Clean up debugfs+stats init/destroy

[Oliver Upton]

+cc the actual kvmarm mailing list, what I had before was wishful
thinking that we moved off of the list that always goes to my spam :-)

On Fri, Apr 15, 2022 at 1:15 PM Oliver Upton <oupton@google.com> wrote:
>
> The way KVM handles debugfs initialization and destruction is somewhat
> sloppy. Although the debugfs + stats bits get initialized *after*
> kvm_create_vm(), they are torn down from kvm_destroy_vm(). And yes,
> there is a window where we could theoretically destroy a VM before
> debugfs is ever instantiated.
>
> This series does away with the mess by coupling debugfs+stats to the
> overall VM create/destroy pattern. We already fail the VM creation if
> kvm_create_vm_debugfs() fails, so there really isn't a need to do these
> separately in the first place.
>
> The first two patches hoist some unrelated tidbits of stats state into
> the debugfs constructors just so its all handled under one roof.
>
> The second two patches realize the the intention of the series, changing
> the initialization order so we can get an FD for the vm early.
>
> Lastly, patch 5 is essentially a revert of Sean's proposed fix [1], but
> I deliberately am not proposing a revert outright, in case alarm bells
> go off that a stable patch got reverted (it is correct).
>
> Applies to the following commit w/ the addition of Sean's patch:
>
>   fb649bda6f56 ("Merge tag 'block-5.18-2022-04-15' of git://git.kernel.dk/linux-block")
>
> Tested (I promise) on an Intel Skylake machine with KVM selftests. I
> poked around in debugfs to make sure there were no stragglers, and I ran
> the reproducer for [1] to confirm the null ptr deref wasn't introduced
> yet again.
>
> Oliver Upton (5):
>   KVM: Shove vm stats_id init into kvm_create_vm_debugfs()
>   KVM: Shove vcpu stats_id init into kvm_vcpu_create_debugfs()
>   KVM: Get an fd before creating the VM
>   KVM: Actually create debugfs in kvm_create_vm()
>   KVM: Hoist debugfs_dentry init to kvm_create_vm_debugfs() (again)
>
>  virt/kvm/kvm_main.c | 92 ++++++++++++++++++++++-----------------------
>  1 file changed, 46 insertions(+), 46 deletions(-)
>
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>

Re: [PATCH 0/5] KVM: Clean up debugfs+stats init/destroy

[Oliver Upton]

+cc the actual kvmarm mailing list, what I had before was wishful
thinking that we moved off of the list that always goes to my spam :-)

On Fri, Apr 15, 2022 at 1:15 PM Oliver Upton <oupton@google.com> wrote:
>
> The way KVM handles debugfs initialization and destruction is somewhat
> sloppy. Although the debugfs + stats bits get initialized *after*
> kvm_create_vm(), they are torn down from kvm_destroy_vm(). And yes,
> there is a window where we could theoretically destroy a VM before
> debugfs is ever instantiated.
>
> This series does away with the mess by coupling debugfs+stats to the
> overall VM create/destroy pattern. We already fail the VM creation if
> kvm_create_vm_debugfs() fails, so there really isn't a need to do these
> separately in the first place.
>
> The first two patches hoist some unrelated tidbits of stats state into
> the debugfs constructors just so its all handled under one roof.
>
> The second two patches realize the the intention of the series, changing
> the initialization order so we can get an FD for the vm early.
>
> Lastly, patch 5 is essentially a revert of Sean's proposed fix [1], but
> I deliberately am not proposing a revert outright, in case alarm bells
> go off that a stable patch got reverted (it is correct).
>
> Applies to the following commit w/ the addition of Sean's patch:
>
>   fb649bda6f56 ("Merge tag 'block-5.18-2022-04-15' of git://git.kernel.dk/linux-block")
>
> Tested (I promise) on an Intel Skylake machine with KVM selftests. I
> poked around in debugfs to make sure there were no stragglers, and I ran
> the reproducer for [1] to confirm the null ptr deref wasn't introduced
> yet again.
>
> Oliver Upton (5):
>   KVM: Shove vm stats_id init into kvm_create_vm_debugfs()
>   KVM: Shove vcpu stats_id init into kvm_vcpu_create_debugfs()
>   KVM: Get an fd before creating the VM
>   KVM: Actually create debugfs in kvm_create_vm()
>   KVM: Hoist debugfs_dentry init to kvm_create_vm_debugfs() (again)
>
>  virt/kvm/kvm_main.c | 92 ++++++++++++++++++++++-----------------------
>  1 file changed, 46 insertions(+), 46 deletions(-)
>
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/5] KVM: Shove vm stats_id init into kvm_create_vm_debugfs()

[Sean Christopherson]

On Fri, Apr 15, 2022, Oliver Upton wrote:
> The field is only ever used for debugfs; put the initialization where
> it belongs.

No?

static ssize_t kvm_vm_stats_read(struct file *file, char __user *user_buffer,
			      size_t size, loff_t *offset)
{
	struct kvm *kvm = file->private_data;

	return kvm_stats_read(kvm->stats_id, &kvm_vm_stats_header,
				&kvm_vm_stats_desc[0], &kvm->stat,
				sizeof(kvm->stat), user_buffer, size, offset);
}

static const struct file_operations kvm_vm_stats_fops = {
	.read = kvm_vm_stats_read,
	.llseek = noop_llseek,
};


And with a name like kvm->stats_id, debugfs seems like it's piggbacking stats,
not the other way 'round.

Re: [PATCH 2/5] KVM: Shove vcpu stats_id init into kvm_vcpu_create_debugfs()

[Sean Christopherson]

On Fri, Apr 15, 2022, Oliver Upton wrote:
> Again, the stats_id is only ever used by the stats code; put it where it
> belongs with the rest of the stats initialization.

Heh, again, no?  Ah, but here you've conflated debugfs and stats in the changelog.
They are two different things.  And most critically, stats is not dependent on debugfs.

Re: [PATCH 4/5] KVM: Actually create debugfs in kvm_create_vm()

[Sean Christopherson]

On Fri, Apr 15, 2022, Oliver Upton wrote:
> @@ -1049,7 +1050,7 @@ int __weak kvm_arch_create_vm_debugfs(struct kvm *kvm)
>  	return 0;
>  }
>  
> -static struct kvm *kvm_create_vm(unsigned long type)
> +static struct kvm *kvm_create_vm(unsigned long type, int fd)

I don't love passing in @fd, because actually doing anything but printing the
@fd in a string is doomed to fail.

Rather than pass the raw fd, what about passing in just its name?

---
 virt/kvm/kvm_main.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d94c1d9ecaa9..ac76fc7f2e4d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -964,7 +964,7 @@ static void kvm_destroy_vm_debugfs(struct kvm *kvm)
 	}
 }

-static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
+static int kvm_create_vm_debugfs(struct kvm *kvm, const char *fdname)
 {
 	static DEFINE_MUTEX(kvm_debugfs_lock);
 	struct dentry *dent;
@@ -987,7 +987,7 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
 	if (!debugfs_initialized())
 		return 0;

-	snprintf(dir_name, sizeof(dir_name), "%d-%d", task_pid_nr(current), fd);
+	snprintf(dir_name, sizeof(dir_name), "%d-%s", task_pid_nr(current), fdname);
 	mutex_lock(&kvm_debugfs_lock);
 	dent = debugfs_lookup(dir_name, kvm_debugfs_dir);
 	if (dent) {
@@ -1076,7 +1076,7 @@ int __weak kvm_arch_create_vm_debugfs(struct kvm *kvm)
 	return 0;
 }

-static struct kvm *kvm_create_vm(unsigned long type, int fd)
+static struct kvm *kvm_create_vm(unsigned long type, const char * fdname)
 {
 	struct kvm *kvm = kvm_arch_alloc_vm();
 	struct kvm_memslots *slots;
@@ -1174,7 +1174,7 @@ static struct kvm *kvm_create_vm(unsigned long type, int fd)
 		goto out_err_mmu_notifier;
 	}

-	r = kvm_create_vm_debugfs(kvm, fd);
+	r = kvm_create_vm_debugfs(kvm, fdname);
 	if (r)
 		goto out_err;

@@ -4781,6 +4781,7 @@ EXPORT_SYMBOL_GPL(file_is_kvm);

 static int kvm_dev_ioctl_create_vm(unsigned long type)
 {
+	char fdname[ITOA_MAX_LEN + 1];
 	int r, fd;
 	struct kvm *kvm;
 	struct file *file;
@@ -4789,7 +4790,9 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	if (fd < 0)
 		return fd;

-	kvm = kvm_create_vm(type, fd);
+	snprintf(fdname, sizeof(fdname), "%d", fd);
+
+	kvm = kvm_create_vm(type, fdname);
 	if (IS_ERR(kvm)) {
 		r = PTR_ERR(kvm);
 		goto put_fd;

base-commit: 3d7c3ff77a78f103c2cf1104157a4132f56fd6d1
--

Re: [PATCH 2/5] KVM: Shove vcpu stats_id init into kvm_vcpu_create_debugfs()

[Oliver Upton]

On Mon, May 16, 2022 at 2:01 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Apr 15, 2022, Oliver Upton wrote:
> > Again, the stats_id is only ever used by the stats code; put it where it
> > belongs with the rest of the stats initialization.
>
> Heh, again, no?  Ah, but here you've conflated debugfs and stats in the changelog.
> They are two different things.  And most critically, stats is not dependent on debugfs.

Agreed. Patch 1 and 2 arise from being bitter that struct kvm and
struct kvm_vcpu fields are initialized out of line with other fields.
I'll yank these into kvm_create_vm() and kvm_vcpu_init(),
respectively.

--
Thanks,
Oliver

Re: [PATCH 4/5] KVM: Actually create debugfs in kvm_create_vm()

[Oliver Upton]

On Mon, May 16, 2022 at 3:19 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Apr 15, 2022, Oliver Upton wrote:
> > @@ -1049,7 +1050,7 @@ int __weak kvm_arch_create_vm_debugfs(struct kvm *kvm)
> >       return 0;
> >  }
> >
> > -static struct kvm *kvm_create_vm(unsigned long type)
> > +static struct kvm *kvm_create_vm(unsigned long type, int fd)
>
> I don't love passing in @fd, because actually doing anything but printing the
> @fd in a string is doomed to fail.
>
> Rather than pass the raw fd, what about passing in just its name?

Urgh. Yeah, that's fine by me. I'll squash this and resend.

--
Thanks,
Oliver