KVM: Add extra information in kvm_page_fault trace point

[PATCH] KVM: Add extra information in kvm_page_fault trace point

Posted by Wonhyuk Yang 3 years, 12 months ago

Currently, kvm_page_fault trace point provide fault_address and error
code. However it is not enough to find which cpu and instruction
cause kvm_page_faults. So add vcpu id and instruction pointer in
kvm_page_fault trace point.

Cc: Baik Song An <bsahn@etri.re.kr>
Cc: Hong Yeon Kim <kimhy@etri.re.kr>
Cc: Taeung Song <taeung@reallinux.co.kr>
Cc: linuxgeek@linuxgeek.io
Signed-off-by: Wonhyuk Yang <vvghjk1234@gmail.com>
---
 arch/x86/kvm/mmu/mmu.c |  2 +-
 arch/x86/kvm/svm/svm.c |  2 +-
 arch/x86/kvm/trace.h   | 12 +++++++++---
 arch/x86/kvm/vmx/vmx.c |  2 +-
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 311e4e1d7870..b9421060efa8 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4080,7 +4080,7 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 
 	vcpu->arch.l1tf_flush_l1d = true;
 	if (!flags) {
-		trace_kvm_page_fault(fault_address, error_code);
+		trace_kvm_page_fault(vcpu, fault_address, error_code);
 
 		if (kvm_event_needs_reinjection(vcpu))
 			kvm_mmu_unprotect_page_virt(vcpu, fault_address);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7e45d03cd018..9741cfbf47a4 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1784,7 +1784,7 @@ static int npf_interception(struct kvm_vcpu *vcpu)
 	u64 fault_address = svm->vmcb->control.exit_info_2;
 	u64 error_code = svm->vmcb->control.exit_info_1;
 
-	trace_kvm_page_fault(fault_address, error_code);
+	trace_kvm_page_fault(vcpu, fault_address, error_code);
 	return kvm_mmu_page_fault(vcpu, fault_address, error_code,
 			static_cpu_has(X86_FEATURE_DECODEASSISTS) ?
 			svm->vmcb->control.insn_bytes : NULL,
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index e3a24b8f04be..78d20d392904 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -383,20 +383,26 @@ TRACE_EVENT(kvm_inj_exception,
  * Tracepoint for page fault.
  */
 TRACE_EVENT(kvm_page_fault,
-	TP_PROTO(unsigned long fault_address, unsigned int error_code),
-	TP_ARGS(fault_address, error_code),
+	TP_PROTO(struct kvm_vcpu *vcpu, unsigned long fault_address,
+		 unsigned int error_code),
+	TP_ARGS(vcpu, fault_address, error_code),
 
 	TP_STRUCT__entry(
+		__field(	unsigned int,	vcpu_id		)
+		__field(	unsigned long,	guest_rip	)
 		__field(	unsigned long,	fault_address	)
 		__field(	unsigned int,	error_code	)
 	),
 
 	TP_fast_assign(
+		__entry->vcpu_id	= vcpu->vcpu_id;
+		__entry->guest_rip	= kvm_rip_read(vcpu);
 		__entry->fault_address	= fault_address;
 		__entry->error_code	= error_code;
 	),
 
-	TP_printk("address %lx error_code %x",
+	TP_printk("vcpu %u rip 0x%lx address 0x%lx error_code %x",
+		  __entry->vcpu_id, __entry->guest_rip,
 		  __entry->fault_address, __entry->error_code)
 );
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 610355b9ccce..0f1edd02b68b 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5398,7 +5398,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
 		vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO, GUEST_INTR_STATE_NMI);
 
 	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
-	trace_kvm_page_fault(gpa, exit_qualification);
+	trace_kvm_page_fault(vcpu, gpa, exit_qualification);
 
 	/* Is it a read fault? */
 	error_code = (exit_qualification & EPT_VIOLATION_ACC_READ)
-- 
2.30.2

Re: [PATCH] KVM: Add extra information in kvm_page_fault trace point

Posted by Sean Christopherson 3 years, 8 months ago

On Tue, May 10, 2022, Wonhyuk Yang wrote:
> Currently, kvm_page_fault trace point provide fault_address and error
> code. However it is not enough to find which cpu and instruction
> cause kvm_page_faults. So add vcpu id and instruction pointer in
> kvm_page_fault trace point.
> 
> Cc: Baik Song An <bsahn@etri.re.kr>
> Cc: Hong Yeon Kim <kimhy@etri.re.kr>
> Cc: Taeung Song <taeung@reallinux.co.kr>
> Cc: linuxgeek@linuxgeek.io
> Signed-off-by: Wonhyuk Yang <vvghjk1234@gmail.com>
> ---

Pushed to branch `for_paolo/6.1` at:

    https://github.com/sean-jc/linux.git

Unless you hear otherwise, it will make its way to kvm/queue "soon".

Note, the commit IDs are not guaranteed to be stable.

Re: [PATCH] KVM: Add extra information in kvm_page_fault trace point

Posted by Sean Christopherson 3 years, 8 months ago

On Tue, May 10, 2022, Wonhyuk Yang wrote:
> Currently, kvm_page_fault trace point provide fault_address and error
> code. However it is not enough to find which cpu and instruction
> cause kvm_page_faults. So add vcpu id and instruction pointer in
> kvm_page_fault trace point.
> 
> Cc: Baik Song An <bsahn@etri.re.kr>
> Cc: Hong Yeon Kim <kimhy@etri.re.kr>
> Cc: Taeung Song <taeung@reallinux.co.kr>
> Cc: linuxgeek@linuxgeek.io
> Signed-off-by: Wonhyuk Yang <vvghjk1234@gmail.com>
> ---

Patch is good, some tangentially related FYI comments below.

> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index e3a24b8f04be..78d20d392904 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -383,20 +383,26 @@ TRACE_EVENT(kvm_inj_exception,
>   * Tracepoint for page fault.
>   */
>  TRACE_EVENT(kvm_page_fault,
> -	TP_PROTO(unsigned long fault_address, unsigned int error_code),
> -	TP_ARGS(fault_address, error_code),
> +	TP_PROTO(struct kvm_vcpu *vcpu, unsigned long fault_address,
> +		 unsigned int error_code),
> +	TP_ARGS(vcpu, fault_address, error_code),
>  
>  	TP_STRUCT__entry(
> +		__field(	unsigned int,	vcpu_id		)
> +		__field(	unsigned long,	guest_rip	)
>  		__field(	unsigned long,	fault_address	)
>  		__field(	unsigned int,	error_code	)

This tracepoint is comically bad.  The address should be a u64 since GPAs can be
64 bits even on 32-bit hosts.  Ditto for error_code since #NPF has 64-bit error
codes.

>  	),
>  
>  	TP_fast_assign(
> +		__entry->vcpu_id	= vcpu->vcpu_id;
> +		__entry->guest_rip	= kvm_rip_read(vcpu);
>  		__entry->fault_address	= fault_address;
>  		__entry->error_code	= error_code;
>  	),
>  
> -	TP_printk("address %lx error_code %x",
> +	TP_printk("vcpu %u rip 0x%lx address 0x%lx error_code %x",

And here the error code needs a "0x" prefix, especially since the majority of error
codes end up being valid decimal values, e.g. 182, 184, 181.

I also think it makes sense to force "address" to pad to 16, but not the others.
Padding error_code is wasteful most of the time, and I actually like that user vs.
kernel addresses and up with different formatting as it makes it trivial to see
where the fault originated (when running "real" guests).

       CPU 5/KVM-4145    [002] .....    86.581928: kvm_page_fault: vcpu 5 rip 0x7f08a4602116 address 0x0000000113600002 error_code 0x181
       CPU 7/KVM-4150    [001] .....    86.581936: kvm_page_fault: vcpu 7 rip 0xffffffff81511f37 address 0x0000000113674000 error_code 0x182
       CPU 5/KVM-4145    [002] .....    86.582585: kvm_page_fault: vcpu 5 rip 0xffffffff81040f72 address 0x00000000fee000b0 error_code 0x182
       CPU 1/KVM-4136    [006] .....    86.588913: kvm_page_fault: vcpu 1 rip 0xffffffff81511ba7 address 0x0000000111400000 error_code 0x182
       CPU 6/KVM-4146    [001] .....    86.594913: kvm_page_fault: vcpu 6 rip 0xffffffff81040f72 address 0x00000000fee000b0 error_code 0x182
       CPU 5/KVM-4145    [002] .....    86.595872: kvm_page_fault: vcpu 5 rip 0x7f08a4602116 address 0x0000000113810002 error_code 0x181
       CPU 5/KVM-4145    [002] .....    86.603341: kvm_page_fault: vcpu 5 rip 0x7f08a4602116 address 0x0000000113a00002 error_code 0x181

All in all, what about me adding this on top?

---
From: Sean Christopherson <seanjc@google.com>
Date: Tue, 30 Aug 2022 12:26:24 -0700
Subject: [PATCH] KVM: x86: Use u64 for address and error code in page fault
 tracepoint

Track the address and error code as 64-bit values in the page fault
tracepoint.  When TDP is enabled, the address is a GPA and thus can be a
64-bit value even on 32-bit hosts.  And SVM's #NPF genereates 64-bit
error codes.

Opportunistically clean up the formatting.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/trace.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 331bdb0ae4b1..c369ebc7269c 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -394,15 +394,14 @@ TRACE_EVENT(kvm_inj_exception,
  * Tracepoint for page fault.
  */
 TRACE_EVENT(kvm_page_fault,
-	TP_PROTO(struct kvm_vcpu *vcpu, unsigned long fault_address,
-		 unsigned int error_code),
+	TP_PROTO(struct kvm_vcpu *vcpu, u64 fault_address, u64 error_code),
 	TP_ARGS(vcpu, fault_address, error_code),

 	TP_STRUCT__entry(
 		__field(	unsigned int,	vcpu_id		)
 		__field(	unsigned long,	guest_rip	)
-		__field(	unsigned long,	fault_address	)
-		__field(	unsigned int,	error_code	)
+		__field(	u64,		fault_address	)
+		__field(	u64,		error_code	)
 	),

 	TP_fast_assign(
@@ -412,7 +411,7 @@ TRACE_EVENT(kvm_page_fault,
 		__entry->error_code	= error_code;
 	),

-	TP_printk("vcpu %u rip 0x%lx address 0x%lx error_code %x",
+	TP_printk("vcpu %u rip 0x%lx address 0x%016llx error_code 0x%llx",
 		  __entry->vcpu_id, __entry->guest_rip,
 		  __entry->fault_address, __entry->error_code)
 );

base-commit: ca362851673d7c01c6624fff0f5a4ee192e6e56a
--