* [PATCH v2 0/2] Extend KVM trace_kvm_nested_vmrun() to support VMX @ 2022-07-18 17:13 Mingwei Zhang 2022-07-18 17:13 ` [PATCH v2 1/2] KVM: nested/x86: update trace_kvm_nested_vmrun() to suppot VMX Mingwei Zhang 2022-07-18 17:13 ` [PATCH v2 2/2] kvm: nVMX: add tracepoint for kvm:kvm_nested_vmrun Mingwei Zhang 0 siblings, 2 replies; 6+ messages in thread From: Mingwei Zhang @ 2022-07-18 17:13 UTC (permalink / raw) To: Paolo Bonzini Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, Mingwei Zhang, David Matlack This patch set update trace_kvm_nested_vmrun() to support VMX: - Change the print of EPT/NPT enabled boolean to print of nested EPT/NPT address in the trace; - Add a caller from vmx/nested.c. - Fix some minor format fixes from the callsites and Update the trace output naming according to the x86 vendor. v1 -> v2: - fix some format issue in trace_kvm_nested_vmrun() in vmx/nested. v1 link: - https://lore.kernel.org/lkml/20220708232304.1001099-2-mizhang@google.com/T/ David Matlack (1): kvm: nVMX: add tracepoint for kvm:kvm_nested_vmrun Mingwei Zhang (1): KVM: nested/x86: update trace_kvm_nested_vmrun() to suppot VMX arch/x86/kvm/svm/nested.c | 4 +++- arch/x86/kvm/trace.h | 19 +++++++++++++------ arch/x86/kvm/vmx/nested.c | 9 +++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) -- 2.37.0.144.g8ac04bfd2-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] KVM: nested/x86: update trace_kvm_nested_vmrun() to suppot VMX 2022-07-18 17:13 [PATCH v2 0/2] Extend KVM trace_kvm_nested_vmrun() to support VMX Mingwei Zhang @ 2022-07-18 17:13 ` Mingwei Zhang 2022-08-03 16:50 ` Sean Christopherson 2022-07-18 17:13 ` [PATCH v2 2/2] kvm: nVMX: add tracepoint for kvm:kvm_nested_vmrun Mingwei Zhang 1 sibling, 1 reply; 6+ messages in thread From: Mingwei Zhang @ 2022-07-18 17:13 UTC (permalink / raw) To: Paolo Bonzini Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, Mingwei Zhang, David Matlack Update trace_kvm_nested_vmrun() to support VMX by adding a new field 'isa'; update the output to print out VMX/SVM related naming respectively, eg., vmcb vs. vmcs; npt vs. ept. In addition, print nested EPT/NPT address instead of the 1bit of nested ept/npt on/off. This should convey more information in the trace. When nested ept/npt is not used, simply print "0x0" so that we don't lose any information. Opportunistically update the call site of trace_kvm_nested_vmrun() to make one line per parameter. Signed-off-by: Mingwei Zhang <mizhang@google.com> --- arch/x86/kvm/svm/nested.c | 7 +++++-- arch/x86/kvm/trace.h | 29 ++++++++++++++++++++--------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index ba7cd26f438f..8581164b6808 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -724,11 +724,14 @@ int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa, struct vcpu_svm *svm = to_svm(vcpu); int ret; - trace_kvm_nested_vmrun(svm->vmcb->save.rip, vmcb12_gpa, + trace_kvm_nested_vmrun(svm->vmcb->save.rip, + vmcb12_gpa, vmcb12->save.rip, vmcb12->control.int_ctl, vmcb12->control.event_inj, - vmcb12->control.nested_ctl); + vmcb12->control.nested_ctl, + vmcb12->control.nested_cr3, + KVM_ISA_SVM); trace_kvm_nested_intercepts(vmcb12->control.intercepts[INTERCEPT_CR] & 0xffff, vmcb12->control.intercepts[INTERCEPT_CR] >> 16, diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h index de4762517569..aac4c8bd2c3a 100644 --- a/arch/x86/kvm/trace.h +++ b/arch/x86/kvm/trace.h @@ -580,8 +580,10 @@ TRACE_EVENT(kvm_pv_eoi, */ TRACE_EVENT(kvm_nested_vmrun, TP_PROTO(__u64 rip, __u64 vmcb, __u64 nested_rip, __u32 int_ctl, - __u32 event_inj, bool npt), - TP_ARGS(rip, vmcb, nested_rip, int_ctl, event_inj, npt), + __u32 event_inj, bool npt_enabled, __u64 npt_addr, + __u32 isa), + TP_ARGS(rip, vmcb, nested_rip, int_ctl, event_inj, npt_enabled, + npt_addr, isa), TP_STRUCT__entry( __field( __u64, rip ) @@ -589,7 +591,9 @@ TRACE_EVENT(kvm_nested_vmrun, __field( __u64, nested_rip ) __field( __u32, int_ctl ) __field( __u32, event_inj ) - __field( bool, npt ) + __field( bool, npt_enabled ) + __field( __u64, npt_addr ) + __field( __u32, isa ) ), TP_fast_assign( @@ -598,14 +602,21 @@ TRACE_EVENT(kvm_nested_vmrun, __entry->nested_rip = nested_rip; __entry->int_ctl = int_ctl; __entry->event_inj = event_inj; - __entry->npt = npt; + __entry->npt_enabled = npt_enabled; + __entry->npt_addr = npt_addr; + __entry->isa = isa; ), - TP_printk("rip: 0x%016llx vmcb: 0x%016llx nrip: 0x%016llx int_ctl: 0x%08x " - "event_inj: 0x%08x npt: %s", - __entry->rip, __entry->vmcb, __entry->nested_rip, - __entry->int_ctl, __entry->event_inj, - __entry->npt ? "on" : "off") + TP_printk("rip: 0x%016llx %s: 0x%016llx nested rip: 0x%016llx " + "int_ctl: 0x%08x event_inj: 0x%08x nested %s: 0x%016llx", + __entry->rip, + __entry->isa == KVM_ISA_VMX ? "vmcs" : "vmcb", + __entry->vmcb, + __entry->nested_rip, + __entry->int_ctl, + __entry->event_inj, + __entry->isa == KVM_ISA_VMX ? "ept" : "npt", + __entry->npt_enabled ? __entry->npt_addr : 0x0) ); TRACE_EVENT(kvm_nested_intercepts, -- 2.37.0.170.g444d1eabd0-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] KVM: nested/x86: update trace_kvm_nested_vmrun() to suppot VMX 2022-07-18 17:13 ` [PATCH v2 1/2] KVM: nested/x86: update trace_kvm_nested_vmrun() to suppot VMX Mingwei Zhang @ 2022-08-03 16:50 ` Sean Christopherson 2022-08-03 19:27 ` Sean Christopherson 0 siblings, 1 reply; 6+ messages in thread From: Sean Christopherson @ 2022-08-03 16:50 UTC (permalink / raw) To: Mingwei Zhang Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, David Matlack If we want to add a unique identifier for nested x86, use "KVM: x86/nested:" to align with the MMU and to make `grep "KVM: x86"` viable. Ideally, we'd align with nVMX and nSVM, but nx86 is pretty gross and looks like a typo. I have a slight preference for using a plain "KVM: x86:", as I suspect we'll never have enough common nested to make it worth differentiating, but I've no objection if you want to go with "KVM: x86/nested:". On Mon, Jul 18, 2022, Mingwei Zhang wrote: > Update trace_kvm_nested_vmrun() to support VMX by adding a new field 'isa'; > update the output to print out VMX/SVM related naming respectively, > eg., vmcb vs. vmcs; npt vs. ept. > > In addition, print nested EPT/NPT address instead of the 1bit of nested > ept/npt on/off. This should convey more information in the trace. When > nested ept/npt is not used, simply print "0x0" so that we don't lose any > information. Adding a new field that's not related to the VMX vs. SVM change belongs in a separate patch. > Opportunistically update the call site of trace_kvm_nested_vmrun() to make > one line per parameter. > > Signed-off-by: Mingwei Zhang <mizhang@google.com> > --- > arch/x86/kvm/svm/nested.c | 7 +++++-- > arch/x86/kvm/trace.h | 29 ++++++++++++++++++++--------- > 2 files changed, 25 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c > index ba7cd26f438f..8581164b6808 100644 > --- a/arch/x86/kvm/svm/nested.c > +++ b/arch/x86/kvm/svm/nested.c > @@ -724,11 +724,14 @@ int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa, > struct vcpu_svm *svm = to_svm(vcpu); > int ret; > > - trace_kvm_nested_vmrun(svm->vmcb->save.rip, vmcb12_gpa, > + trace_kvm_nested_vmrun(svm->vmcb->save.rip, > + vmcb12_gpa, > vmcb12->save.rip, > vmcb12->control.int_ctl, > vmcb12->control.event_inj, > - vmcb12->control.nested_ctl); > + vmcb12->control.nested_ctl, > + vmcb12->control.nested_cr3, > + KVM_ISA_SVM); > > trace_kvm_nested_intercepts(vmcb12->control.intercepts[INTERCEPT_CR] & 0xffff, > vmcb12->control.intercepts[INTERCEPT_CR] >> 16, > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h > index de4762517569..aac4c8bd2c3a 100644 > --- a/arch/x86/kvm/trace.h > +++ b/arch/x86/kvm/trace.h > @@ -580,8 +580,10 @@ TRACE_EVENT(kvm_pv_eoi, > */ > TRACE_EVENT(kvm_nested_vmrun, I think it makes sense to rename this to kvm_nested_vmenter. VMRUN is SVM-only, and tracepoints aren't ABI. > TP_PROTO(__u64 rip, __u64 vmcb, __u64 nested_rip, __u32 int_ctl, > - __u32 event_inj, bool npt), > - TP_ARGS(rip, vmcb, nested_rip, int_ctl, event_inj, npt), > + __u32 event_inj, bool npt_enabled, __u64 npt_addr, > + __u32 isa), > + TP_ARGS(rip, vmcb, nested_rip, int_ctl, event_inj, npt_enabled, > + npt_addr, isa), > > TP_STRUCT__entry( > __field( __u64, rip ) > @@ -589,7 +591,9 @@ TRACE_EVENT(kvm_nested_vmrun, > __field( __u64, nested_rip ) > __field( __u32, int_ctl ) > __field( __u32, event_inj ) > - __field( bool, npt ) > + __field( bool, npt_enabled ) s/npt_enabled/tdp_enabled, or maybe ntdp_enabled? > + __field( __u64, npt_addr ) Hmm, either s/npt_addr/nested_pgd or s/npt_addr/guest_pgd "npt_addr" or "tdp_addr" is too ambiguous, e.g. it can be interpreted as the address of a TDP page fault. My vote would be for "guest_pgd" and then pass in the non-nested CR3 when L1 isn't using nTDP. > + __field( __u32, isa ) > ), > > TP_fast_assign( > @@ -598,14 +602,21 @@ TRACE_EVENT(kvm_nested_vmrun, > __entry->nested_rip = nested_rip; > __entry->int_ctl = int_ctl; > __entry->event_inj = event_inj; > - __entry->npt = npt; > + __entry->npt_enabled = npt_enabled; > + __entry->npt_addr = npt_addr; > + __entry->isa = isa; > ), > > - TP_printk("rip: 0x%016llx vmcb: 0x%016llx nrip: 0x%016llx int_ctl: 0x%08x " > - "event_inj: 0x%08x npt: %s", > - __entry->rip, __entry->vmcb, __entry->nested_rip, > - __entry->int_ctl, __entry->event_inj, > - __entry->npt ? "on" : "off") > + TP_printk("rip: 0x%016llx %s: 0x%016llx nested rip: 0x%016llx " > + "int_ctl: 0x%08x event_inj: 0x%08x nested %s: 0x%016llx", This needs to explicitly print "nTDP on/off". As proposed, "nested ept/npt: %addr" doesn't capture that. (a) addr=0 is perfectly legal, and (b) addr!=0 is also legal if nTDP isn't enabled, i.e. a non-zero nested_cr3/eptp is ignore by hardware > + __entry->rip, > + __entry->isa == KVM_ISA_VMX ? "vmcs" : "vmcb", > + __entry->vmcb, > + __entry->nested_rip, > + __entry->int_ctl, > + __entry->event_inj, > + __entry->isa == KVM_ISA_VMX ? "ept" : "npt", > + __entry->npt_enabled ? __entry->npt_addr : 0x0) > ); > > TRACE_EVENT(kvm_nested_intercepts, > -- > 2.37.0.170.g444d1eabd0-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] KVM: nested/x86: update trace_kvm_nested_vmrun() to suppot VMX 2022-08-03 16:50 ` Sean Christopherson @ 2022-08-03 19:27 ` Sean Christopherson 0 siblings, 0 replies; 6+ messages in thread From: Sean Christopherson @ 2022-08-03 19:27 UTC (permalink / raw) To: Mingwei Zhang Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, David Matlack Almost forgot, s/suppot/support in the shortlog. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] kvm: nVMX: add tracepoint for kvm:kvm_nested_vmrun 2022-07-18 17:13 [PATCH v2 0/2] Extend KVM trace_kvm_nested_vmrun() to support VMX Mingwei Zhang 2022-07-18 17:13 ` [PATCH v2 1/2] KVM: nested/x86: update trace_kvm_nested_vmrun() to suppot VMX Mingwei Zhang @ 2022-07-18 17:13 ` Mingwei Zhang 2022-08-03 16:53 ` Sean Christopherson 1 sibling, 1 reply; 6+ messages in thread From: Mingwei Zhang @ 2022-07-18 17:13 UTC (permalink / raw) To: Paolo Bonzini Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, Mingwei Zhang, David Matlack From: David Matlack <dmatlack@google.com> This tracepoint is called by nested SVM during emulated VMRUN. Call also during emulated VMLAUNCH and VMRESUME in nested VMX. Attempt to use analagous VMCS fields to the VMCB fields that are reported in the SVM case: "int_ctl": 32-bit field of the VMCB that the CPU uses to deliver virtual interrupts. The analagous VMCS field is the 16-bit "guest interrupt status". "event_inj": 32-bit field of VMCB that is used to inject events (exceptions and interrupts) into the guest. The analagous VMCS field is the "VM-entry interruption-information field". "npt_enabled": 1 when the VCPU has enabled nested paging. The analagous VMCS field is the enable-EPT execution control. "npt_addr": 64-bit field when the VCPU has enabled nested paging. The analagous VMCS field is the ept_pointer. Signed-off-by: David Matlack <dmatlack@google.com> [Add several parameters and move the code into the nested_vmx_enter_non_root_mode().] Signed-off-by: Mingwei Zhang <mizhang@google.com> --- arch/x86/kvm/vmx/nested.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index f5cb18e00e78..825f7102daee 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -3367,6 +3367,15 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, }; u32 failed_index; + trace_kvm_nested_vmrun(kvm_rip_read(vcpu), + vmx->nested.current_vmptr, + vmcs12->guest_rip, + vmcs12->guest_intr_status, + vmcs12->vm_entry_intr_info_field, + vmcs12->secondary_vm_exec_control & SECONDARY_EXEC_ENABLE_EPT, + vmcs12->ept_pointer, + KVM_ISA_VMX); + kvm_service_local_tlb_flush_requests(vcpu); evaluate_pending_interrupts = exec_controls_get(vmx) & -- 2.37.0.170.g444d1eabd0-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] kvm: nVMX: add tracepoint for kvm:kvm_nested_vmrun 2022-07-18 17:13 ` [PATCH v2 2/2] kvm: nVMX: add tracepoint for kvm:kvm_nested_vmrun Mingwei Zhang @ 2022-08-03 16:53 ` Sean Christopherson 0 siblings, 0 replies; 6+ messages in thread From: Sean Christopherson @ 2022-08-03 16:53 UTC (permalink / raw) To: Mingwei Zhang Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel, David Matlack Capitalize KVM in the shortlog, i.e. "KVM: nVMX:". On Mon, Jul 18, 2022, Mingwei Zhang wrote: > From: David Matlack <dmatlack@google.com> > > This tracepoint is called by nested SVM during emulated VMRUN. Call > also during emulated VMLAUNCH and VMRESUME in nested VMX. Please reword this so it's a more coherent statement of what the patch does, e.g. Call trace_kvm_nested_vmenter() during nested VMLAUNCH/VMRESUME to bring parity with nSVM's usage of the tracepoint during nested VMRUN. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-03 19:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-18 17:13 [PATCH v2 0/2] Extend KVM trace_kvm_nested_vmrun() to support VMX Mingwei Zhang 2022-07-18 17:13 ` [PATCH v2 1/2] KVM: nested/x86: update trace_kvm_nested_vmrun() to suppot VMX Mingwei Zhang 2022-08-03 16:50 ` Sean Christopherson 2022-08-03 19:27 ` Sean Christopherson 2022-07-18 17:13 ` [PATCH v2 2/2] kvm: nVMX: add tracepoint for kvm:kvm_nested_vmrun Mingwei Zhang 2022-08-03 16:53 ` Sean Christopherson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox