[PATCH 0/3] KVM: nVMX: Add more statistics to KVM debugfs

[PATCH 0/3] KVM: nVMX: Add more statistics to KVM debugfs

[Krish Sadhukhan]

Patch# 1: Renames 'nested_run' statistic and increments it from a more
	  fine-grained location.
Patch# 2: Adds a new statistic to show if a VCPU is running a nested guest.
Patch# 3: Adds a new statistic to show number of VCPUs created in a given VM.


[PATCH 1/3] KVM: nVMX: Move 'nested_run' counter to
[PATCH 2/3] KVM: nVMX: Add a new VCPU statistic to show if VCPU is
[PATCH 3/3] KVM: x86: Add a new VM statistic to show number of VCPUs

 arch/x86/include/asm/kvm_host.h |  4 +++-
 arch/x86/kvm/debugfs.c          | 11 +++++++++++
 arch/x86/kvm/kvm_cache_regs.h   |  4 ++++
 arch/x86/kvm/svm/nested.c       |  2 --
 arch/x86/kvm/vmx/nested.c       |  2 --
 arch/x86/kvm/x86.c              |  4 +++-
 virt/kvm/kvm_main.c             |  2 ++
 7 files changed, 23 insertions(+), 6 deletions(-)

Krish Sadhukhan (3):
      KVM: nVMX: Move 'nested_run' counter to enter_guest_mode()
      KVM: nVMX: Add a new VCPU statistic to show if VCPU is running nested guest
      KVM: x86: Add a new VM statistic to show number of VCPUs created in a given VM

[PATCH 1/3] KVM: nVMX: Move 'nested_run' counter to enter_guest_mode()

[Krish Sadhukhan]

Move 'nested_run' counter to enter_guest_mode() because,
    i) This counter is common to both Intel and AMD and can be incremented
       from a common place,
    ii) guest mode is a more finer-grained state than the beginning of
	nested_svm_vmrun() and nested_vmx_run().

Also, rename it to 'nested_runs'.

Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 2 +-
 arch/x86/kvm/kvm_cache_regs.h   | 1 +
 arch/x86/kvm/svm/nested.c       | 2 --
 arch/x86/kvm/vmx/nested.c       | 2 --
 arch/x86/kvm/x86.c              | 2 +-
 5 files changed, 3 insertions(+), 6 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/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
index 3db5c42c9ecd..cf52cbff18d3 100644
--- a/arch/x86/kvm/kvm_cache_regs.h
+++ b/arch/x86/kvm/kvm_cache_regs.h
@@ -162,6 +162,7 @@ static inline u64 kvm_read_edx_eax(struct kvm_vcpu *vcpu)
 static inline void enter_guest_mode(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.hflags |= HF_GUEST_MASK;
+	++vcpu->stat.nested_runs;
 }
 
 static inline void leave_guest_mode(struct kvm_vcpu *vcpu)
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/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/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

Re: [PATCH 1/3] KVM: nVMX: Move 'nested_run' counter to enter_guest_mode()

[Sean Christopherson]

On Tue, May 11, 2021, Krish Sadhukhan wrote:
> Move 'nested_run' counter to enter_guest_mode() because,
>     i) This counter is common to both Intel and AMD and can be incremented
>        from a common place,
>     ii) guest mode is a more finer-grained state than the beginning of
> 	nested_svm_vmrun() and nested_vmx_run().

Hooking enter_guest_mode() makes the name a misnomer since it will count cases
such as setting nested state and resuming from SMI, neither of which is a nested
run in the sense of L1 deliberately choosing to run L2.

And while bumping nested_run at the very beginning of VMLAUNCH/VMRESUME/VMRUN is
arguably wrong in that it counts _attempts_ instead of successful VM-Enters, it's
at least consistent.  Moving this to enter_guest_mode() means it's arbitrarily
counting VM-Enter that fails late, but not those that fail early.

If we really want it to mean "successful VM-Enter", then we should wait until
after VM-Enter actual succeeds, and do it only for actual VM-Enter.

Re: [PATCH 1/3] KVM: nVMX: Move 'nested_run' counter to enter_guest_mode()

[Dongli Zhang]

On 5/12/21 11:30 AM, Sean Christopherson wrote:
> On Tue, May 11, 2021, Krish Sadhukhan wrote:
>> Move 'nested_run' counter to enter_guest_mode() because,
>>     i) This counter is common to both Intel and AMD and can be incremented
>>        from a common place,
>>     ii) guest mode is a more finer-grained state than the beginning of
>> 	nested_svm_vmrun() and nested_vmx_run().
> 
> Hooking enter_guest_mode() makes the name a misnomer since it will count cases
> such as setting nested state and resuming from SMI, neither of which is a nested
> run in the sense of L1 deliberately choosing to run L2.
> 
> And while bumping nested_run at the very beginning of VMLAUNCH/VMRESUME/VMRUN is
> arguably wrong in that it counts _attempts_ instead of successful VM-Enters, it's

Yes, the original purpose was to track the attempt. That's why the counter is
incremented at the beginning. It helps tell if there is any attempt to run L2 VM
(and also if VM is actively running L2 VM by monitoring the counter).

This helps as sometimes VM owner does not realize software like jailhouse will
involve the L2 VM.

Without the counter, we may need to temporarily use
"/sys/kernel/debug/kvm/mmu_unsync" assuming shadow page table is not used by
host if L2 VM is not involved.

Dongli Zhang

> at least consistent.  Moving this to enter_guest_mode() means it's arbitrarily
> counting VM-Enter that fails late, but not those that fail early.
> 
> If we really want it to mean "successful VM-Enter", then we should wait until
> after VM-Enter actual succeeds, and do it only for actual VM-Enter.
> 

[PATCH 2/3] KVM: nVMX: Add a new VCPU statistic to show if VCPU is running nested guest

[Krish Sadhukhan]

Add the following per-VCPU statistic to KVM debugfs to show if a given
VCPU is running a nested guest:

	nested_guest_running

Also add this as a per-VM statistic to KVM debugfs to show the total number
of VCPUs running a nested guest in a given VM.

Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/debugfs.c          | 11 +++++++++++
 arch/x86/kvm/kvm_cache_regs.h   |  3 +++
 arch/x86/kvm/x86.c              |  1 +
 4 files changed, 16 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index cf8557b2b90f..884f6e5ba669 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1173,6 +1173,7 @@ struct kvm_vcpu_stat {
 	u64 nested_runs;
 	u64 directed_yield_attempted;
 	u64 directed_yield_successful;
+	u64 nested_guest_running;
 };
 
 struct x86_instruction_info;
diff --git a/arch/x86/kvm/debugfs.c b/arch/x86/kvm/debugfs.c
index 7e818d64bb4d..465d243afaac 100644
--- a/arch/x86/kvm/debugfs.c
+++ b/arch/x86/kvm/debugfs.c
@@ -17,6 +17,15 @@ static int vcpu_get_timer_advance_ns(void *data, u64 *val)
 
 DEFINE_SIMPLE_ATTRIBUTE(vcpu_timer_advance_ns_fops, vcpu_get_timer_advance_ns, NULL, "%llu\n");
 
+static int vcpu_get_guest_mode(void *data, u64 *val)
+{
+	struct kvm_vcpu *vcpu = (struct kvm_vcpu *) data;
+	*val = vcpu->stat.nested_guest_running;
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(vcpu_guest_mode_fops, vcpu_get_guest_mode, NULL, "%lld\n");
+
 static int vcpu_get_tsc_offset(void *data, u64 *val)
 {
 	struct kvm_vcpu *vcpu = (struct kvm_vcpu *) data;
@@ -45,6 +54,8 @@ DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_scaling_frac_fops, vcpu_get_tsc_scaling_frac_bi
 
 void kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry)
 {
+	debugfs_create_file("nested_guest_running", 0444, debugfs_dentry, vcpu,
+			    &vcpu_guest_mode_fops);
 	debugfs_create_file("tsc-offset", 0444, debugfs_dentry, vcpu,
 			    &vcpu_tsc_offset_fops);
 
diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
index cf52cbff18d3..348ef98c6199 100644
--- a/arch/x86/kvm/kvm_cache_regs.h
+++ b/arch/x86/kvm/kvm_cache_regs.h
@@ -163,6 +163,7 @@ static inline void enter_guest_mode(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.hflags |= HF_GUEST_MASK;
 	++vcpu->stat.nested_runs;
+	vcpu->stat.nested_guest_running = 1;
 }
 
 static inline void leave_guest_mode(struct kvm_vcpu *vcpu)
@@ -173,6 +174,8 @@ static inline void leave_guest_mode(struct kvm_vcpu *vcpu)
 		vcpu->arch.load_eoi_exitmap_pending = false;
 		kvm_make_request(KVM_REQ_LOAD_EOI_EXITMAP, vcpu);
 	}
+
+	vcpu->stat.nested_guest_running = 0;
 }
 
 static inline bool is_guest_mode(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6d1f51f6c344..01805b68dc9b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -246,6 +246,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	VCPU_STAT("nested_runs", nested_runs),
 	VCPU_STAT("directed_yield_attempted", directed_yield_attempted),
 	VCPU_STAT("directed_yield_successful", directed_yield_successful),
+	VCPU_STAT("nested_guest_running", nested_guest_running),
 	VM_STAT("mmu_shadow_zapped", mmu_shadow_zapped),
 	VM_STAT("mmu_pte_write", mmu_pte_write),
 	VM_STAT("mmu_pde_zapped", mmu_pde_zapped),
-- 
2.27.0

Re: [PATCH 2/3] KVM: nVMX: Add a new VCPU statistic to show if VCPU is running nested guest

[Jim Mattson]

On Tue, May 11, 2021 at 7:37 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:
>
> Add the following per-VCPU statistic to KVM debugfs to show if a given
> VCPU is running a nested guest:
>
>         nested_guest_running
>
> Also add this as a per-VM statistic to KVM debugfs to show the total number
> of VCPUs running a nested guest in a given VM.
>
> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>

This is fine, but I don't really see its usefulness. OTOH, one
statistic I would really like to see is how many vCPUs have *ever* run
a nested guest.

Re: [PATCH 2/3] KVM: nVMX: Add a new VCPU statistic to show if VCPU is running nested guest

[Krish Sadhukhan]

On 5/12/21 9:01 AM, Jim Mattson wrote:
> On Tue, May 11, 2021 at 7:37 PM Krish Sadhukhan
> <krish.sadhukhan@oracle.com> wrote:
>> Add the following per-VCPU statistic to KVM debugfs to show if a given
>> VCPU is running a nested guest:
>>
>>          nested_guest_running
>>
>> Also add this as a per-VM statistic to KVM debugfs to show the total number
>> of VCPUs running a nested guest in a given VM.
>>
>> Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
> This is fine, but I don't really see its usefulness. OTOH, one

Two potential uses:

     1. If Live Migration of L2 guests is broken/buggy, this can be used 
to determine a safer time to trigger Live Migration of L1 guests.

     2. This can be used to create a time-graph of the load of L1 and L2 
in a given VM as well across the host.

> statistic I would really like to see is how many vCPUs have *ever* run
> a nested guest.

'nested_runs' statistic provides this data, though only till VCPUs are 
alive. We can convert 'nested_runs' to be persistent beyond VCPU 
destruction.

But I am curious about the usage you are thinking of with this.

Re: [PATCH 2/3] KVM: nVMX: Add a new VCPU statistic to show if VCPU is running nested guest

[Jim Mattson]

On Wed, May 12, 2021 at 10:56 AM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:

> But I am curious about the usage you are thinking of with this.

I'm interested in tracking usage of nested virtualization.

Re: [PATCH 2/3] KVM: nVMX: Add a new VCPU statistic to show if VCPU is running nested guest

[Sean Christopherson]

On Wed, May 12, 2021, Krish Sadhukhan wrote:
> 
> On 5/12/21 9:01 AM, Jim Mattson wrote:
> > On Tue, May 11, 2021 at 7:37 PM Krish Sadhukhan
> > <krish.sadhukhan@oracle.com> wrote:
> > > Add the following per-VCPU statistic to KVM debugfs to show if a given
> > > VCPU is running a nested guest:
> > > 
> > >          nested_guest_running
> > > 
> > > Also add this as a per-VM statistic to KVM debugfs to show the total number
> > > of VCPUs running a nested guest in a given VM.
> > > 
> > > Signed-off-by: Krish Sadhukhan <Krish.Sadhukhan@oracle.com>
> > This is fine, but I don't really see its usefulness. OTOH, one
> 
> Two potential uses:
> 
>     1. If Live Migration of L2 guests is broken/buggy, this can be used to
> determine a safer time to trigger Live Migration of L1 guests.

This seems tenuous.  The stats are inherently racy, so userspace would still need
to check for "guest mode" after retrieving state.  And wouldn't you want to wait
until L1 turns VMX/SVM _off_?  If migrating L2 is broken, simply waiting until L2
exits likely isn't going to help all that much.

>     2. This can be used to create a time-graph of the load of L1 and L2 in a
> given VM as well across the host.

Hrm, I like the idea of being able to observe how much time a vCPU is spending
in L1 vs. L2, but cross-referencing guest time with "in L2" seems difficult
and error prone.  I wonder if we can do better, i.e. explicitly track L1 vs. L2+
usage.  I think that would also grant Jim's wish of being able to more precisely
track nested virtualization utilization.

> > statistic I would really like to see is how many vCPUs have *ever* run
> > a nested guest.

[PATCH 3/3] 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/x86.c              | 1 +
 virt/kvm/kvm_main.c             | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 884f6e5ba669..4a3e3c04ef38 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;
+	u64 created_vcpus;
 };
 
 struct kvm_vcpu_stat {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 01805b68dc9b..0c2a57e1e096 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("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("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