diff mbox series

[v4,3/3] KVM: x86: Print guest pgd in kvm_nested_vmenter()

Message ID 20220825225755.907001-4-mizhang@google.com (mailing list archive)
State New, archived
Headers show
Series Extend KVM trace_kvm_nested_vmrun() to support VMX | expand

Commit Message

Mingwei Zhang Aug. 25, 2022, 10:57 p.m. UTC
Print guest pgd in kvm_nested_vmenter() to enrich the information for
tracing. When tdp is enabled, print the value of tdp page table (EPT/NPT);
when tdp is disabled, print the value of non-nested CR3.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Mingwei Zhang <mizhang@google.com>
---
 arch/x86/kvm/svm/nested.c |  2 ++
 arch/x86/kvm/trace.h      | 13 +++++++++----
 arch/x86/kvm/vmx/nested.c |  2 ++
 3 files changed, 13 insertions(+), 4 deletions(-)

Comments

Sean Christopherson Aug. 30, 2022, 9:29 p.m. UTC | #1
On Thu, Aug 25, 2022, Mingwei Zhang wrote:
> ---
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index e7f0da9474f0..b2be0348bb14 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -591,9 +591,10 @@ TRACE_EVENT(kvm_pv_eoi,
>   */
>  TRACE_EVENT(kvm_nested_vmenter,
>  	    TP_PROTO(__u64 rip, __u64 vmcb, __u64 nested_rip, __u32 int_ctl,
> -		     __u32 event_inj, bool tdp_enabled, __u32 isa),
> +		     __u32 event_inj, bool tdp_enabled, __u64 guest_tdp,

s/guest_tdp_pgd to differentiate it from "tdp_enabled"


>         TP_printk("rip: 0x%016llx %s: 0x%016llx nested_rip: 0x%016llx "
> -                 "int_ctl: 0x%08x event_inj: 0x%08x nested_%s: %s",
> +                 "int_ctl: 0x%08x event_inj: 0x%08x nested_%s: %s, "
> +                 "guest_pgd: 0x%016llx",

It's a little gross, but this can spit out nested_eptp vs. nested_cr3 vs. guest_cr3.

>                 __entry->rip,
>                 __entry->isa == KVM_ISA_VMX ? "vmcs" : "vmcb",
>                 __entry->vmcb,

> @@ -624,7 +628,8 @@ TRACE_EVENT(kvm_nested_vmenter,
>  		__entry->int_ctl,
>  		__entry->event_inj,
>  		__entry->isa == KVM_ISA_VMX ? "ept" : "npt",
> -		__entry->tdp_enabled ? "on" : "off")
> +		__entry->tdp_enabled ? "on" : "off",

To keep things aligned, and because "on nested_eptp" reads as a combined
snippet

  event_inj: 0x00000000 nested_ept: off guest_cr3: 0x0000000001007000
  event_inj: 0x00000000 nested_ept: off guest_cr3: 0x0000000001007000
  event_inj: 0x00000000 nested_ept: on nested_eptp: 0x0000000007ec501e
  event_inj: 0x00000000 nested_ept: on nested_eptp: 0x0000000007ec501e

what about teaking the format so that the output looks like this?

  event_inj: 0x00000000 nested_ept=n guest_cr3: 0x0000000001007000
  event_inj: 0x00000000 nested_ept=n guest_cr3: 0x0000000001007000
  event_inj: 0x00000000 nested_ept=y nested_eptp: 0x0000000007ec501e
  event_inj: 0x00000000 nested_ept=y nested_eptp: 0x0000000007ec501e

Deviating from the ": " style bothers me, but I find this difficult to read.
Again, letters delimited by whitespace get visually clumped together.

  event_inj: 0x00000000 nested_ept: n guest_cr3: 0x0000000001007000
  event_inj: 0x00000000 nested_ept: n guest_cr3: 0x0000000001007000
  event_inj: 0x00000000 nested_ept: y nested_eptp: 0x0000000007ec501e
  event_inj: 0x00000000 nested_ept: y nested_eptp: 0x0000000007ec501e

And this looks like a typo

  event_inj: 0x00000000 nested_ept:n guest_cr3: 0x0000000001007000
  event_inj: 0x00000000 nested_ept:n guest_cr3: 0x0000000001007000
  event_inj: 0x00000000 nested_ept:y nested_eptp: 0x0000000007ec501e
  event_inj: 0x00000000 nested_ept:y nested_eptp: 0x0000000007ec501e

We could leave off the nested_{ept/npt} entirely and leave the differentation to
the nested_eptp vs. nested_cr3 vs. guest_cr3, but I don't think that's worth
shaving a few chars.
diff mbox series

Patch

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 835c508eed8e..05b7994244c5 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -787,6 +787,8 @@  int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa,
 			       vmcb12->control.int_ctl,
 			       vmcb12->control.event_inj,
 			       vmcb12->control.nested_ctl,
+			       vmcb12->control.nested_cr3,
+			       vmcb12->save.cr3,
 			       KVM_ISA_SVM);
 
 	trace_kvm_nested_intercepts(vmcb12->control.intercepts[INTERCEPT_CR] & 0xffff,
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index e7f0da9474f0..b2be0348bb14 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -591,9 +591,10 @@  TRACE_EVENT(kvm_pv_eoi,
  */
 TRACE_EVENT(kvm_nested_vmenter,
 	    TP_PROTO(__u64 rip, __u64 vmcb, __u64 nested_rip, __u32 int_ctl,
-		     __u32 event_inj, bool tdp_enabled, __u32 isa),
+		     __u32 event_inj, bool tdp_enabled, __u64 guest_tdp,
+		     __u64 guest_cr3, __u32 isa),
 	    TP_ARGS(rip, vmcb, nested_rip, int_ctl, event_inj, tdp_enabled,
-		    isa),
+		    guest_tdp, guest_cr3, isa),
 
 	TP_STRUCT__entry(
 		__field(	__u64,		rip		)
@@ -602,6 +603,7 @@  TRACE_EVENT(kvm_nested_vmenter,
 		__field(	__u32,		int_ctl		)
 		__field(	__u32,		event_inj	)
 		__field(	bool,		tdp_enabled	)
+		__field(	__u64,		guest_pgd	)
 		__field(	__u32,		isa		)
 	),
 
@@ -612,11 +614,13 @@  TRACE_EVENT(kvm_nested_vmenter,
 		__entry->int_ctl	= int_ctl;
 		__entry->event_inj	= event_inj;
 		__entry->tdp_enabled	= tdp_enabled;
+		__entry->guest_pgd	= tdp_enabled ? guest_tdp : guest_cr3;
 		__entry->isa		= isa;
 	),
 
 	TP_printk("rip: 0x%016llx %s: 0x%016llx nested_rip: 0x%016llx "
-		  "int_ctl: 0x%08x event_inj: 0x%08x nested_%s: %s",
+		  "int_ctl: 0x%08x event_inj: 0x%08x nested_%s: %s, "
+		  "guest_pgd: 0x%016llx",
 		__entry->rip,
 		__entry->isa == KVM_ISA_VMX ? "vmcs" : "vmcb",
 		__entry->vmcb,
@@ -624,7 +628,8 @@  TRACE_EVENT(kvm_nested_vmenter,
 		__entry->int_ctl,
 		__entry->event_inj,
 		__entry->isa == KVM_ISA_VMX ? "ept" : "npt",
-		__entry->tdp_enabled ? "on" : "off")
+		__entry->tdp_enabled ? "on" : "off",
+		__entry->guest_pgd)
 );
 
 TRACE_EVENT(kvm_nested_intercepts,
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f72fe9452391..f963e5ce0a28 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3370,6 +3370,8 @@  enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 				 vmcs12->guest_intr_status,
 				 vmcs12->vm_entry_intr_info_field,
 				 vmcs12->secondary_vm_exec_control & SECONDARY_EXEC_ENABLE_EPT,
+				 vmcs12->ept_pointer,
+				 vmcs12->guest_cr3,
 				 KVM_ISA_VMX);
 
 	kvm_service_local_tlb_flush_requests(vcpu);