[PATCH 0/4 v2] KVM: nVMX: nSVM: Add more statistics to KVM debugfs

[PATCH 0/4 v2] KVM: nVMX: nSVM: Add more statistics to KVM debugfs

[Krish Sadhukhan]

v1 -> v2:
	* Patch# 1 is new and is not directly related to this patchset.
	* Patch# 2 now tracks only those guest-entries that pass all
	  checks in hardware and make it to guest code.
	* Patch# 3 removes the 'nested_guest_running' statistic that was
	  added in v1 and instead adds a stat that tracks how many VCPUs
	  have run a nested guest.

[PATCH 1/4 v2] KVM: nVMX: Reset 'nested_run_pending' only in guest mode
[PATCH 2/4 v2] KVM: nVMX: nSVM: 'nested_run' should count guest-entry
[PATCH 3/4 v2] KVM: nVMX: nSVM: Add a new debugfs statistic to show how
[PATCH 4/4 v2] KVM: x86: Add a new VM statistic to show number of VCPUs

 arch/x86/include/asm/kvm_host.h |  4 +++-
 arch/x86/kvm/svm/nested.c       |  2 --
 arch/x86/kvm/svm/svm.c          |  9 +++++++++
 arch/x86/kvm/vmx/nested.c       |  2 --
 arch/x86/kvm/vmx/vmx.c          | 16 +++++++++++++++-
 arch/x86/kvm/x86.c              |  4 +++-
 virt/kvm/kvm_main.c             |  2 ++
 7 files changed, 32 insertions(+), 7 deletions(-)

Krish Sadhukhan (4):
      KVM: nVMX: Reset 'nested_run_pending' only in guest mode
      KVM: nVMX: nSVM: 'nested_run' should count guest-entry attempts that make it to guest code
      KVM: nVMX: nSVM: Add a new debugfs statistic to show how many VCPUs have run nested guests
      KVM: x86: Add a new VM statistic to show number of VCPUs created in a given VM

[PATCH 1/4 v2] KVM: nVMX: Reset 'nested_run_pending' only in guest mode

[Krish Sadhukhan]

Currently, vmx_vcpu_run() resets 'nested_run_pending' irrespective of whether
it is in guest mode. 'nested_run_pending' matters only to guest mode and
hence reset it only in guest mode.

Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
---
 arch/x86/kvm/vmx/vmx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f2fd447eed45..af2be5930ba4 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6839,7 +6839,9 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu)
 
 	kvm_load_host_xsave_state(vcpu);
 
-	vmx->nested.nested_run_pending = 0;
+	if (is_guest_mode(vcpu))
+		vmx->nested.nested_run_pending = 0;
+
 	vmx->idt_vectoring_info = 0;
 
 	if (unlikely(vmx->fail)) {
-- 
2.27.0

[PATCH 2/4 v2] KVM: nVMX: nSVM: 'nested_run' should count guest-entry attempts that make it to guest code

[Krish Sadhukhan]

Currently, the 'nested_run' statistic counts all guest-entry attempts,
including those that fail during vmentry checks on Intel and during
consistency checks on AMD. Convert this statistic to count only those
guest-entries that make it past these state checks and make it to guest
code. This will tell us the number of guest-entries that actually executed
or tried to execute guest code.

Also, rename this statistic to 'nested_runs' since it is a count.

Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
Suggested-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/svm/nested.c       |  2 --
 arch/x86/kvm/svm/svm.c          |  7 +++++++
 arch/x86/kvm/vmx/nested.c       |  2 --
 arch/x86/kvm/vmx/vmx.c          | 11 ++++++++++-
 arch/x86/kvm/x86.c              |  2 +-
 6 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 55efbacfc244..cf8557b2b90f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1170,7 +1170,7 @@ struct kvm_vcpu_stat {
 	u64 req_event;
 	u64 halt_poll_success_ns;
 	u64 halt_poll_fail_ns;
-	u64 nested_run;
+	u64 nested_runs;
 	u64 directed_yield_attempted;
 	u64 directed_yield_successful;
 };
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 5e8d8443154e..34fc74b0d58a 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -596,8 +596,6 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
 	struct kvm_host_map map;
 	u64 vmcb12_gpa;
 
-	++vcpu->stat.nested_run;
-
 	if (is_smm(vcpu)) {
 		kvm_queue_exception(vcpu, UD_VECTOR);
 		return 1;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 4dd9b7856e5b..57c351640355 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3872,6 +3872,13 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
 	svm->next_rip = 0;
 	if (is_guest_mode(vcpu)) {
 		nested_sync_control_from_vmcb02(svm);
+
+		/* Track VMRUNs that have made past consistency checking */
+		if (svm->nested.nested_run_pending &&
+		    svm->vmcb->control.exit_code != SVM_EXIT_ERR &&
+		    svm->vmcb->control.exit_code != SVM_EXIT_NPF)
+                        ++vcpu->stat.nested_runs;
+
 		svm->nested.nested_run_pending = 0;
 	}
 
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 6058a65a6ede..94f70c0af4a4 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3454,8 +3454,6 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
 	u32 interrupt_shadow = vmx_get_interrupt_shadow(vcpu);
 	enum nested_evmptrld_status evmptrld_status;
 
-	++vcpu->stat.nested_run;
-
 	if (!nested_vmx_check_permission(vcpu))
 		return 1;
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index af2be5930ba4..fa8df7ab2756 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6839,8 +6839,17 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu)
 
 	kvm_load_host_xsave_state(vcpu);
 
-	if (is_guest_mode(vcpu))
+	if (is_guest_mode(vcpu)) {
+		/*
+		 * Track VMLAUNCH/VMRESUME that have made past guest state
+		 * checking.
+		 */
+		if (vmx->nested.nested_run_pending &&
+		    !vmx->exit_reason.failed_vmentry)
+			++vcpu->stat.nested_runs;
+
 		vmx->nested.nested_run_pending = 0;
+	}
 
 	vmx->idt_vectoring_info = 0;
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5bd550eaf683..6d1f51f6c344 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -243,7 +243,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	VCPU_STAT("l1d_flush", l1d_flush),
 	VCPU_STAT("halt_poll_success_ns", halt_poll_success_ns),
 	VCPU_STAT("halt_poll_fail_ns", halt_poll_fail_ns),
-	VCPU_STAT("nested_run", nested_run),
+	VCPU_STAT("nested_runs", nested_runs),
 	VCPU_STAT("directed_yield_attempted", directed_yield_attempted),
 	VCPU_STAT("directed_yield_successful", directed_yield_successful),
 	VM_STAT("mmu_shadow_zapped", mmu_shadow_zapped),
-- 
2.27.0

[PATCH 3/4 v2] KVM: nVMX: nSVM: Add a new debugfs statistic to show how many VCPUs have run nested guests

[Krish Sadhukhan]

Add a new debugfs statistic to show how many VCPUs have run nested guests.
This statistic considers only the first time a given VCPU successfully runs
a nested guest.

Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
Suggested-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/svm/svm.c          | 5 ++++-
 arch/x86/kvm/vmx/vmx.c          | 5 ++++-
 arch/x86/kvm/x86.c              | 1 +
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index cf8557b2b90f..a19fe2cfaa93 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1138,6 +1138,7 @@ struct kvm_vm_stat {
 	ulong lpages;
 	ulong nx_lpage_splits;
 	ulong max_mmu_page_hash_collisions;
+	ulong vcpus_ran_nested;
 };
 
 struct kvm_vcpu_stat {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 57c351640355..d1871c51411f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3876,8 +3876,11 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
 		/* Track VMRUNs that have made past consistency checking */
 		if (svm->nested.nested_run_pending &&
 		    svm->vmcb->control.exit_code != SVM_EXIT_ERR &&
-		    svm->vmcb->control.exit_code != SVM_EXIT_NPF)
+		    svm->vmcb->control.exit_code != SVM_EXIT_NPF) {
+			if (!vcpu->stat.nested_runs)
+				++vcpu->kvm->stat.vcpus_ran_nested;
                         ++vcpu->stat.nested_runs;
+		}
 
 		svm->nested.nested_run_pending = 0;
 	}
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index fa8df7ab2756..dc29aa926be6 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6845,8 +6845,11 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu)
 		 * checking.
 		 */
 		if (vmx->nested.nested_run_pending &&
-		    !vmx->exit_reason.failed_vmentry)
+		    !vmx->exit_reason.failed_vmentry) {
+			if (!vcpu->stat.nested_runs)
+				++vcpu->kvm->stat.vcpus_ran_nested;
 			++vcpu->stat.nested_runs;
+		}
 
 		vmx->nested.nested_run_pending = 0;
 	}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6d1f51f6c344..cbca3609a152 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -257,6 +257,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	VM_STAT("largepages", lpages, .mode = 0444),
 	VM_STAT("nx_largepages_splitted", nx_lpage_splits, .mode = 0444),
 	VM_STAT("max_mmu_page_hash_collisions", max_mmu_page_hash_collisions),
+	VM_STAT("vcpus_ran_nested", vcpus_ran_nested),
 	{ NULL }
 };
 
-- 
2.27.0

[PATCH 4/4 v2] KVM: x86: Add a new VM statistic to show number of VCPUs created in a given VM

[Krish Sadhukhan]

'struct kvm' already has a member for counting the number of VCPUs created
for a given VM. Add this as a new VM statistic to KVM debugfs.

Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/svm/svm.c          | 3 +--
 arch/x86/kvm/x86.c              | 1 +
 virt/kvm/kvm_main.c             | 2 ++
 4 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index a19fe2cfaa93..69ca1d6f6557 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1139,6 +1139,7 @@ struct kvm_vm_stat {
 	ulong nx_lpage_splits;
 	ulong max_mmu_page_hash_collisions;
 	ulong vcpus_ran_nested;
+	u64 created_vcpus;
 };
 
 struct kvm_vcpu_stat {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d1871c51411f..fef0baba043b 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3875,8 +3875,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
 
 		/* Track VMRUNs that have made past consistency checking */
 		if (svm->nested.nested_run_pending &&
-		    svm->vmcb->control.exit_code != SVM_EXIT_ERR &&
-		    svm->vmcb->control.exit_code != SVM_EXIT_NPF) {
+		    svm->vmcb->control.exit_code != SVM_EXIT_ERR) {
 			if (!vcpu->stat.nested_runs)
 				++vcpu->kvm->stat.vcpus_ran_nested;
                         ++vcpu->stat.nested_runs;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cbca3609a152..a9d27ce4cc93 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -258,6 +258,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	VM_STAT("nx_largepages_splitted", nx_lpage_splits, .mode = 0444),
 	VM_STAT("max_mmu_page_hash_collisions", max_mmu_page_hash_collisions),
 	VM_STAT("vcpus_ran_nested", vcpus_ran_nested),
+	VM_STAT("created_vcpus", created_vcpus),
 	{ NULL }
 };
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6b4feb92dc79..ac8f02d8a051 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3318,6 +3318,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 	}
 
 	kvm->created_vcpus++;
+	kvm->stat.created_vcpus++;
 	mutex_unlock(&kvm->lock);
 
 	r = kvm_arch_vcpu_precreate(kvm, id);
@@ -3394,6 +3395,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 vcpu_decrement:
 	mutex_lock(&kvm->lock);
 	kvm->created_vcpus--;
+	kvm->stat.created_vcpus--;
 	mutex_unlock(&kvm->lock);
 	return r;
 }
-- 
2.27.0

Re: [PATCH 2/4 v2] KVM: nVMX: nSVM: 'nested_run' should count guest-entry attempts that make it to guest code

[Sean Christopherson]

On Wed, May 19, 2021, Krish Sadhukhan wrote:
> Currently, the 'nested_run' statistic counts all guest-entry attempts,
> including those that fail during vmentry checks on Intel and during
> consistency checks on AMD. Convert this statistic to count only those
> guest-entries that make it past these state checks and make it to guest
> code. This will tell us the number of guest-entries that actually executed
> or tried to execute guest code.
> 
> Also, rename this statistic to 'nested_runs' since it is a count.
> 
> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
> Suggested-by: Sean Christopherson <seanjc@google.com>

I didn't suggest this, or at least I didn't intend to.  My point was that _if_
we want stat.nested_run to count successful VM-Enter then the counter should be
bumped only after VM-Enter succeeds.  I did not mean to say that we should
actually do this.  As Dongli pointed out[*], there is value in tracking attempts,
and I still don't understand _why_ we care about only counting successful
VM-Enters.

FWIW, this misses the case where L1 enters L2 in an inactive state.

[*] ed4a8dae-be99-0d88-a8dd-510afe7cb956@oracle.com

Re: [PATCH 3/4 v2] KVM: nVMX: nSVM: Add a new debugfs statistic to show how many VCPUs have run nested guests

[Sean Christopherson]

On Wed, May 19, 2021, Krish Sadhukhan wrote:
> Add a new debugfs statistic to show how many VCPUs have run nested guests.
> This statistic considers only the first time a given VCPU successfully runs
> a nested guest.
> 
> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
> Suggested-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 1 +
>  arch/x86/kvm/svm/svm.c          | 5 ++++-
>  arch/x86/kvm/vmx/vmx.c          | 5 ++++-
>  arch/x86/kvm/x86.c              | 1 +
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index cf8557b2b90f..a19fe2cfaa93 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1138,6 +1138,7 @@ struct kvm_vm_stat {
>  	ulong lpages;
>  	ulong nx_lpage_splits;
>  	ulong max_mmu_page_hash_collisions;
> +	ulong vcpus_ran_nested;
>  };
>  
>  struct kvm_vcpu_stat {
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 57c351640355..d1871c51411f 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3876,8 +3876,11 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
>  		/* Track VMRUNs that have made past consistency checking */
>  		if (svm->nested.nested_run_pending &&
>  		    svm->vmcb->control.exit_code != SVM_EXIT_ERR &&
> -		    svm->vmcb->control.exit_code != SVM_EXIT_NPF)
> +		    svm->vmcb->control.exit_code != SVM_EXIT_NPF) {
> +			if (!vcpu->stat.nested_runs)
> +				++vcpu->kvm->stat.vcpus_ran_nested;

Using a separate counter seems unnecessary, userspace can aggregate
vcpu->stat.nested_run itself to see how many vCPUs have done nested VM-Enter.

Jim, were you thinking of something else?  Am I missing something?

Re: [PATCH 1/4 v2] KVM: nVMX: Reset 'nested_run_pending' only in guest mode

[Sean Christopherson]

On Wed, May 19, 2021, Krish Sadhukhan wrote:
> Currently, vmx_vcpu_run() resets 'nested_run_pending' irrespective of whether
> it is in guest mode. 'nested_run_pending' matters only to guest mode and
> hence reset it only in guest mode.
> 
> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index f2fd447eed45..af2be5930ba4 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6839,7 +6839,9 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu)
>  
>  	kvm_load_host_xsave_state(vcpu);
>  
> -	vmx->nested.nested_run_pending = 0;
> +	if (is_guest_mode(vcpu))
> +		vmx->nested.nested_run_pending = 0;

This patch does not stand on its own, checking is_guest_mode() is likely more
expensive than unconditionally clearing the flag.  If we end up with conditional
stats code then I've no objection to clearing this conditionally, but that can
be done opportunstically.

Also, the check should be against vmx->nested.nested_run_pending itself, not
against is_guest_mode().  E.g. the stats patch adds a check on that, too.

> +
>  	vmx->idt_vectoring_info = 0;
>  
>  	if (unlikely(vmx->fail)) {
> -- 
> 2.27.0
> 

Re: [PATCH 4/4 v2] KVM: x86: Add a new VM statistic to show number of VCPUs created in a given VM

[Sean Christopherson]

On Wed, May 19, 2021, Krish Sadhukhan wrote:
> 'struct kvm' already has a member for counting the number of VCPUs created
> for a given VM. Add this as a new VM statistic to KVM debugfs.

Huh!??  Why?  Userspace is the one creating the vCPUs, it darn well should know
how many it's created.
 
> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 1 +
>  arch/x86/kvm/svm/svm.c          | 3 +--
>  arch/x86/kvm/x86.c              | 1 +
>  virt/kvm/kvm_main.c             | 2 ++
>  4 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index a19fe2cfaa93..69ca1d6f6557 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1139,6 +1139,7 @@ struct kvm_vm_stat {
>  	ulong nx_lpage_splits;
>  	ulong max_mmu_page_hash_collisions;
>  	ulong vcpus_ran_nested;
> +	u64 created_vcpus;
>  };
>  
>  struct kvm_vcpu_stat {
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d1871c51411f..fef0baba043b 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3875,8 +3875,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
>  
>  		/* Track VMRUNs that have made past consistency checking */
>  		if (svm->nested.nested_run_pending &&
> -		    svm->vmcb->control.exit_code != SVM_EXIT_ERR &&
> -		    svm->vmcb->control.exit_code != SVM_EXIT_NPF) {
> +		    svm->vmcb->control.exit_code != SVM_EXIT_ERR) {

???

>  			if (!vcpu->stat.nested_runs)
>  				++vcpu->kvm->stat.vcpus_ran_nested;
>                          ++vcpu->stat.nested_runs;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index cbca3609a152..a9d27ce4cc93 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -258,6 +258,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>  	VM_STAT("nx_largepages_splitted", nx_lpage_splits, .mode = 0444),
>  	VM_STAT("max_mmu_page_hash_collisions", max_mmu_page_hash_collisions),
>  	VM_STAT("vcpus_ran_nested", vcpus_ran_nested),
> +	VM_STAT("created_vcpus", created_vcpus),
>  	{ NULL }
>  };
>  
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 6b4feb92dc79..ac8f02d8a051 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3318,6 +3318,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
>  	}
>  
>  	kvm->created_vcpus++;
> +	kvm->stat.created_vcpus++;
>  	mutex_unlock(&kvm->lock);
>  
>  	r = kvm_arch_vcpu_precreate(kvm, id);
> @@ -3394,6 +3395,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
>  vcpu_decrement:
>  	mutex_lock(&kvm->lock);
>  	kvm->created_vcpus--;
> +	kvm->stat.created_vcpus--;
>  	mutex_unlock(&kvm->lock);
>  	return r;
>  }
> -- 
> 2.27.0
> 

Re: [PATCH 3/4 v2] KVM: nVMX: nSVM: Add a new debugfs statistic to show how many VCPUs have run nested guests

[Jim Mattson]

On Thu, May 20, 2021 at 7:56 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, May 19, 2021, Krish Sadhukhan wrote:
> > Add a new debugfs statistic to show how many VCPUs have run nested guests.
> > This statistic considers only the first time a given VCPU successfully runs
> > a nested guest.
> >
> > Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
> > Suggested-by: Jim Mattson <jmattson@google.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h | 1 +
> >  arch/x86/kvm/svm/svm.c          | 5 ++++-
> >  arch/x86/kvm/vmx/vmx.c          | 5 ++++-
> >  arch/x86/kvm/x86.c              | 1 +
> >  4 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index cf8557b2b90f..a19fe2cfaa93 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1138,6 +1138,7 @@ struct kvm_vm_stat {
> >       ulong lpages;
> >       ulong nx_lpage_splits;
> >       ulong max_mmu_page_hash_collisions;
> > +     ulong vcpus_ran_nested;
> >  };
> >
> >  struct kvm_vcpu_stat {
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 57c351640355..d1871c51411f 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -3876,8 +3876,11 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
> >               /* Track VMRUNs that have made past consistency checking */
> >               if (svm->nested.nested_run_pending &&
> >                   svm->vmcb->control.exit_code != SVM_EXIT_ERR &&
> > -                 svm->vmcb->control.exit_code != SVM_EXIT_NPF)
> > +                 svm->vmcb->control.exit_code != SVM_EXIT_NPF) {
> > +                     if (!vcpu->stat.nested_runs)
> > +                             ++vcpu->kvm->stat.vcpus_ran_nested;
>
> Using a separate counter seems unnecessary, userspace can aggregate
> vcpu->stat.nested_run itself to see how many vCPUs have done nested VM-Enter.
>
> Jim, were you thinking of something else?  Am I missing something?

It was in the context of a proposed stat to indicate how many vCPUs
are *currently* running nested guests that I said I'd rather just know
how many vCPUs had *ever* run nested guests. I don't need a separate
stat. Checking vcpu->stat.nested_run for non-zero values works fine
for me.

Re: [PATCH 2/4 v2] KVM: nVMX: nSVM: 'nested_run' should count guest-entry attempts that make it to guest code

[Krish Sadhukhan]

On 5/20/21 7:53 AM, Sean Christopherson wrote:
> On Wed, May 19, 2021, Krish Sadhukhan wrote:
>> Currently, the 'nested_run' statistic counts all guest-entry attempts,
>> including those that fail during vmentry checks on Intel and during
>> consistency checks on AMD. Convert this statistic to count only those
>> guest-entries that make it past these state checks and make it to guest
>> code. This will tell us the number of guest-entries that actually executed
>> or tried to execute guest code.
>>
>> Also, rename this statistic to 'nested_runs' since it is a count.
>>
>> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
> I didn't suggest this, or at least I didn't intend to.


The idea behind my v1 patch was actually to eliminate failed attempts of 
vmentry but of course I didn't place the counter in the right location. 
In v2, I placed it based on your suggestion.

So, the burden of the intention to count only successful vmentries still 
falls on me 😁.

>   My point was that _if_
> we want stat.nested_run to count successful VM-Enter then the counter should be
> bumped only after VM-Enter succeeds.  I did not mean to say that we should
> actually do this.  As Dongli pointed out[*], there is value in tracking attempts,
> and I still don't understand _why_ we care about only counting successful
> VM-Enters.


If we count all attempts, failed and successful, the counter won't tell 
us if any failed and how many. On the other hand, if we count only 
successful attempts, we won't know if there were any failed attempts. 
Both ways we miss potentially valuable data, but unless the count of 
failures is greatly useful we just count attempts that actually ended up 
executing a nested guest.

The other alternative is to provide two counters, 'nested_run_attempts' 
to count all attempts and 'nested_runs' to count only successful ones.

>
> FWIW, this misses the case where L1 enters L2 in an inactive state.
>
> [*] ed4a8dae-be99-0d88-a8dd-510afe7cb956@oracle.com

Re: [PATCH 3/4 v2] KVM: nVMX: nSVM: Add a new debugfs statistic to show how many VCPUs have run nested guests

[Krish Sadhukhan]

On 5/20/21 9:57 AM, Jim Mattson wrote:
> On Thu, May 20, 2021 at 7:56 AM Sean Christopherson <seanjc@google.com> wrote:
>> On Wed, May 19, 2021, Krish Sadhukhan wrote:
>>> Add a new debugfs statistic to show how many VCPUs have run nested guests.
>>> This statistic considers only the first time a given VCPU successfully runs
>>> a nested guest.
>>>
>>> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
>>> Suggested-by: Jim Mattson <jmattson@google.com>
>>> ---
>>>   arch/x86/include/asm/kvm_host.h | 1 +
>>>   arch/x86/kvm/svm/svm.c          | 5 ++++-
>>>   arch/x86/kvm/vmx/vmx.c          | 5 ++++-
>>>   arch/x86/kvm/x86.c              | 1 +
>>>   4 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>>> index cf8557b2b90f..a19fe2cfaa93 100644
>>> --- a/arch/x86/include/asm/kvm_host.h
>>> +++ b/arch/x86/include/asm/kvm_host.h
>>> @@ -1138,6 +1138,7 @@ struct kvm_vm_stat {
>>>        ulong lpages;
>>>        ulong nx_lpage_splits;
>>>        ulong max_mmu_page_hash_collisions;
>>> +     ulong vcpus_ran_nested;
>>>   };
>>>
>>>   struct kvm_vcpu_stat {
>>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>>> index 57c351640355..d1871c51411f 100644
>>> --- a/arch/x86/kvm/svm/svm.c
>>> +++ b/arch/x86/kvm/svm/svm.c
>>> @@ -3876,8 +3876,11 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
>>>                /* Track VMRUNs that have made past consistency checking */
>>>                if (svm->nested.nested_run_pending &&
>>>                    svm->vmcb->control.exit_code != SVM_EXIT_ERR &&
>>> -                 svm->vmcb->control.exit_code != SVM_EXIT_NPF)
>>> +                 svm->vmcb->control.exit_code != SVM_EXIT_NPF) {
>>> +                     if (!vcpu->stat.nested_runs)
>>> +                             ++vcpu->kvm->stat.vcpus_ran_nested;
>> Using a separate counter seems unnecessary, userspace can aggregate
>> vcpu->stat.nested_run itself to see how many vCPUs have done nested VM-Enter.
>>
>> Jim, were you thinking of something else?  Am I missing something?
> It was in the context of a proposed stat to indicate how many vCPUs
> are *currently* running nested guests that I said I'd rather just know
> how many vCPUs had *ever* run nested guests. I don't need a separate
> stat. Checking vcpu->stat.nested_run for non-zero values works fine
> for me.
I will fall back to my v1 idea then. That's at least useful if we want 
to create a time graph of VCPUs running nested guests.

Re: [PATCH 4/4 v2] KVM: x86: Add a new VM statistic to show number of VCPUs created in a given VM

[Krish Sadhukhan]

On 5/20/21 8:04 AM, Sean Christopherson wrote:
> On Wed, May 19, 2021, Krish Sadhukhan wrote:
>> 'struct kvm' already has a member for counting the number of VCPUs created
>> for a given VM. Add this as a new VM statistic to KVM debugfs.
> Huh!??  Why?  Userspace is the one creating the vCPUs, it darn well should know
> how many it's created.
>   


If I am providing a host for users to create VMs, how do I know who 
creates how many VCPUs ? This statistic is intended show usage of VCPU 
resources on a host used by customers.

>> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
>> ---
>>   arch/x86/include/asm/kvm_host.h | 1 +
>>   arch/x86/kvm/svm/svm.c          | 3 +--
>>   arch/x86/kvm/x86.c              | 1 +
>>   virt/kvm/kvm_main.c             | 2 ++
>>   4 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index a19fe2cfaa93..69ca1d6f6557 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -1139,6 +1139,7 @@ struct kvm_vm_stat {
>>   	ulong nx_lpage_splits;
>>   	ulong max_mmu_page_hash_collisions;
>>   	ulong vcpus_ran_nested;
>> +	u64 created_vcpus;
>>   };
>>   
>>   struct kvm_vcpu_stat {
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index d1871c51411f..fef0baba043b 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -3875,8 +3875,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
>>   
>>   		/* Track VMRUNs that have made past consistency checking */
>>   		if (svm->nested.nested_run_pending &&
>> -		    svm->vmcb->control.exit_code != SVM_EXIT_ERR &&
>> -		    svm->vmcb->control.exit_code != SVM_EXIT_NPF) {
>> +		    svm->vmcb->control.exit_code != SVM_EXIT_ERR) {
> ???


Sorry, this one sneaked in here from the other patch!  Will remove it.

>
>>   			if (!vcpu->stat.nested_runs)
>>   				++vcpu->kvm->stat.vcpus_ran_nested;
>>                           ++vcpu->stat.nested_runs;
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index cbca3609a152..a9d27ce4cc93 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -258,6 +258,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>>   	VM_STAT("nx_largepages_splitted", nx_lpage_splits, .mode = 0444),
>>   	VM_STAT("max_mmu_page_hash_collisions", max_mmu_page_hash_collisions),
>>   	VM_STAT("vcpus_ran_nested", vcpus_ran_nested),
>> +	VM_STAT("created_vcpus", created_vcpus),
>>   	{ NULL }
>>   };
>>   
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 6b4feb92dc79..ac8f02d8a051 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -3318,6 +3318,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
>>   	}
>>   
>>   	kvm->created_vcpus++;
>> +	kvm->stat.created_vcpus++;
>>   	mutex_unlock(&kvm->lock);
>>   
>>   	r = kvm_arch_vcpu_precreate(kvm, id);
>> @@ -3394,6 +3395,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
>>   vcpu_decrement:
>>   	mutex_lock(&kvm->lock);
>>   	kvm->created_vcpus--;
>> +	kvm->stat.created_vcpus--;
>>   	mutex_unlock(&kvm->lock);
>>   	return r;
>>   }
>> -- 
>> 2.27.0
>>

Re: [PATCH 4/4 v2] KVM: x86: Add a new VM statistic to show number of VCPUs created in a given VM

[Sean Christopherson]

On Fri, May 21, 2021, Krish Sadhukhan wrote:
> 
> On 5/20/21 8:04 AM, Sean Christopherson wrote:
> > On Wed, May 19, 2021, Krish Sadhukhan wrote:
> > > 'struct kvm' already has a member for counting the number of VCPUs created
> > > for a given VM. Add this as a new VM statistic to KVM debugfs.
> > Huh!??  Why?  Userspace is the one creating the vCPUs, it darn well should know
> > how many it's created.
> 
> If I am providing a host for users to create VMs, how do I know who creates
> how many VCPUs ? This statistic is intended show usage of VCPU resources on
> a host used by customers.

How are reviewers supposed to know that that's the use case?  Use the changelog
to state _why_ a patch is needed/justified.

> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index cbca3609a152..a9d27ce4cc93 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -258,6 +258,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
> > >   	VM_STAT("nx_largepages_splitted", nx_lpage_splits, .mode = 0444),
> > >   	VM_STAT("max_mmu_page_hash_collisions", max_mmu_page_hash_collisions),
> > >   	VM_STAT("vcpus_ran_nested", vcpus_ran_nested),
> > > +	VM_STAT("created_vcpus", created_vcpus),

IMO, the "created" part is unnecessary for the stats, i.e. just call it "vcpus",
or maybe "nr_vcpus".

> > >   	{ NULL }
> > >   };
> > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > > index 6b4feb92dc79..ac8f02d8a051 100644
> > > --- a/virt/kvm/kvm_main.c
> > > +++ b/virt/kvm/kvm_main.c
> > > @@ -3318,6 +3318,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
> > >   	}
> > >   	kvm->created_vcpus++;
> > > +	kvm->stat.created_vcpus++;
> > >   	mutex_unlock(&kvm->lock);
> > >   	r = kvm_arch_vcpu_precreate(kvm, id);
> > > @@ -3394,6 +3395,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
> > >   vcpu_decrement:
> > >   	mutex_lock(&kvm->lock);
> > >   	kvm->created_vcpus--;
> > > +	kvm->stat.created_vcpus--;
> > >   	mutex_unlock(&kvm->lock);
> > >   	return r;
> > >   }
> > > -- 
> > > 2.27.0
> > >