* Re: [PATCH] KVM: Add extra information in kvm_page_fault trace point
2022-05-10 7:10 [PATCH] KVM: Add extra information in kvm_page_fault trace point Wonhyuk Yang
@ 2022-08-30 19:29 ` Sean Christopherson
2022-08-30 21:42 ` Sean Christopherson
1 sibling, 0 replies; 3+ messages in thread
From: Sean Christopherson @ 2022-08-30 19:29 UTC (permalink / raw)
To: Wonhyuk Yang
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Baik Song An, Hong Yeon Kim,
Taeung Song, linuxgeek, kvm, linux-kernel
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
--
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] KVM: Add extra information in kvm_page_fault trace point
2022-05-10 7:10 [PATCH] KVM: Add extra information in kvm_page_fault trace point Wonhyuk Yang
2022-08-30 19:29 ` Sean Christopherson
@ 2022-08-30 21:42 ` Sean Christopherson
1 sibling, 0 replies; 3+ messages in thread
From: Sean Christopherson @ 2022-08-30 21:42 UTC (permalink / raw)
To: Wonhyuk Yang
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Baik Song An, Hong Yeon Kim,
Taeung Song, linuxgeek, kvm, linux-kernel
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.
^ permalink raw reply [flat|nested] 3+ messages in thread