| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Yw5lCY5/SOmFGQrK@google.com>
Date: Tue, 30 Aug 2022 19:29:13 +0000
From: Sean Christopherson <seanjc@...gle.com>
To: Wonhyuk Yang <vvghjk1234@...il.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>, Baik Song An <bsahn@...i.re.kr>,
Hong Yeon Kim <kimhy@...i.re.kr>,
Taeung Song <taeung@...llinux.co.kr>, linuxgeek@...uxgeek.io,
kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: Add extra information in kvm_page_fault trace point
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@...i.re.kr>
> Cc: Hong Yeon Kim <kimhy@...i.re.kr>
> Cc: Taeung Song <taeung@...llinux.co.kr>
> Cc: linuxgeek@...uxgeek.io
> Signed-off-by: Wonhyuk Yang <vvghjk1234@...il.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@...gle.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@...gle.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
--
Powered by blists - more mailing lists