* [PATCH v2 0/3] VMX: more nested fixes @ 2021-01-14 20:54 Maxim Levitsky 2021-01-14 20:54 ` [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration Maxim Levitsky ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Maxim Levitsky @ 2021-01-14 20:54 UTC (permalink / raw) To: kvm Cc: Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, Sean Christopherson, linux-kernel, Jim Mattson, Maxim Levitsky This is hopefully the last fix for VMX nested migration that finally allows my stress test of migration with a nested guest to pass. In a nutshell after an optimization that was done in commit 7952d769c29ca, some of vmcs02 fields which can be modified by the L2 freely while it runs (like GSBASE and such) were not copied back to vmcs12 unless: 1. L1 tries to vmread them (update done on intercept) 2. vmclear or vmldptr on other vmcs are done. 3. nested state is read and nested guest is running. What wasn't done was to sync these 'rare' fields when L1 is running but still has a loaded vmcs12 which might have some stale fields, if that vmcs was used to enter a guest already due to that optimization. Plus I added two minor patches to improve VMX tracepoints a bit. There is still a large room for improvement. Best regards, Maxim Levitsky Maxim Levitsky (3): KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint KVM: VMX: read idt_vectoring_info a bit earlier arch/x86/kvm/trace.h | 30 ++++++++++++++++++++++++++++++ arch/x86/kvm/vmx/nested.c | 19 ++++++++++++++----- arch/x86/kvm/vmx/vmx.c | 3 ++- arch/x86/kvm/x86.c | 1 + 4 files changed, 47 insertions(+), 6 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration 2021-01-14 20:54 [PATCH v2 0/3] VMX: more nested fixes Maxim Levitsky @ 2021-01-14 20:54 ` Maxim Levitsky 2021-01-14 23:58 ` Sean Christopherson 2021-01-14 20:54 ` [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint Maxim Levitsky ` (2 subsequent siblings) 3 siblings, 1 reply; 15+ messages in thread From: Maxim Levitsky @ 2021-01-14 20:54 UTC (permalink / raw) To: kvm Cc: Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, Sean Christopherson, linux-kernel, Jim Mattson, Maxim Levitsky Even when we are outside the nested guest, some vmcs02 fields are not in sync vs vmcs12. However during the migration, the vmcs12 has to be up to date to be able to load it later after the migration. To fix that, call that function. Fixes: 7952d769c29ca ("KVM: nVMX: Sync rarely accessed guest fields only when needed") Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- arch/x86/kvm/vmx/nested.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 0fbb46990dfce..776688f9d1017 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -6077,11 +6077,14 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu, if (is_guest_mode(vcpu)) { sync_vmcs02_to_vmcs12(vcpu, vmcs12); sync_vmcs02_to_vmcs12_rare(vcpu, vmcs12); - } else if (!vmx->nested.need_vmcs12_to_shadow_sync) { - if (vmx->nested.hv_evmcs) - copy_enlightened_to_vmcs12(vmx); - else if (enable_shadow_vmcs) - copy_shadow_to_vmcs12(vmx); + } else { + copy_vmcs02_to_vmcs12_rare(vcpu, get_vmcs12(vcpu)); + if (!vmx->nested.need_vmcs12_to_shadow_sync) { + if (vmx->nested.hv_evmcs) + copy_enlightened_to_vmcs12(vmx); + else if (enable_shadow_vmcs) + copy_shadow_to_vmcs12(vmx); + } } BUILD_BUG_ON(sizeof(user_vmx_nested_state->vmcs12) < VMCS12_SIZE); -- 2.26.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration 2021-01-14 20:54 ` [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration Maxim Levitsky @ 2021-01-14 23:58 ` Sean Christopherson 2021-01-21 14:59 ` Paolo Bonzini 0 siblings, 1 reply; 15+ messages in thread From: Sean Christopherson @ 2021-01-14 23:58 UTC (permalink / raw) To: Maxim Levitsky Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson Subject is confusing, and technically wrong. Confusing because there is no call to sync_vmcs02_to_vmcs12_rare(). Technically wrong because sync_...() won't be called if need_sync_vmcs02_to_vmcs12_rare==false. Maybe something like? KVM: nVMX: Sync unsync'd vmcs02 state to vmcs12 on migration On Thu, Jan 14, 2021, Maxim Levitsky wrote: > Even when we are outside the nested guest, some vmcs02 fields > are not in sync vs vmcs12. s/are not/may not be It'd be helpful to provide more details in the changelog, e.g. for those not in the loop, it's not obvious that KVM intentionally keeps those fields unsync'd, even across nested VM-Exit. > However during the migration, the vmcs12 has to be up to date > to be able to load it later after the migration. > > To fix that, call that function. s/that function/copy_vmcs02_to_vmcs12_rare(). Characters are cheap, and it's jarring to have to jump back all the way back to the subject. And, as mentioned above, this doesn't actually call sync_ directly. For the code, Reviewed-by: Sean Christopherson <seanjc@google.com> > > Fixes: 7952d769c29ca ("KVM: nVMX: Sync rarely accessed guest fields only when needed") > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/vmx/nested.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 0fbb46990dfce..776688f9d1017 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -6077,11 +6077,14 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu, > if (is_guest_mode(vcpu)) { > sync_vmcs02_to_vmcs12(vcpu, vmcs12); > sync_vmcs02_to_vmcs12_rare(vcpu, vmcs12); > - } else if (!vmx->nested.need_vmcs12_to_shadow_sync) { > - if (vmx->nested.hv_evmcs) > - copy_enlightened_to_vmcs12(vmx); > - else if (enable_shadow_vmcs) > - copy_shadow_to_vmcs12(vmx); > + } else { > + copy_vmcs02_to_vmcs12_rare(vcpu, get_vmcs12(vcpu)); > + if (!vmx->nested.need_vmcs12_to_shadow_sync) { > + if (vmx->nested.hv_evmcs) > + copy_enlightened_to_vmcs12(vmx); > + else if (enable_shadow_vmcs) > + copy_shadow_to_vmcs12(vmx); > + } > } > > BUILD_BUG_ON(sizeof(user_vmx_nested_state->vmcs12) < VMCS12_SIZE); > -- > 2.26.2 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration 2021-01-14 23:58 ` Sean Christopherson @ 2021-01-21 14:59 ` Paolo Bonzini 0 siblings, 0 replies; 15+ messages in thread From: Paolo Bonzini @ 2021-01-21 14:59 UTC (permalink / raw) To: Sean Christopherson, Maxim Levitsky Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On 15/01/21 00:58, Sean Christopherson wrote: > Reviewed-by: Sean Christopherson<seanjc@google.com> New commit message: KVM: nVMX: Sync unsync'd vmcs02 state to vmcs12 on migration Even when we are outside the nested guest, some vmcs02 fields may not be in sync vs vmcs12. This is intentional, even across nested VM-exit, because the sync can be delayed until the nested hypervisor performs a VMCLEAR or a VMREAD/VMWRITE that affects those rarely accessed fields. However, during KVM_GET_NESTED_STATE, the vmcs12 has to be up to date to be able to restore it. To fix that, call copy_vmcs02_to_vmcs12_rare() before the vmcs12 contents are copied to userspace. Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint 2021-01-14 20:54 [PATCH v2 0/3] VMX: more nested fixes Maxim Levitsky 2021-01-14 20:54 ` [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration Maxim Levitsky @ 2021-01-14 20:54 ` Maxim Levitsky 2021-01-15 0:14 ` Sean Christopherson 2021-01-14 20:54 ` [PATCH v2 3/3] KVM: VMX: read idt_vectoring_info a bit earlier Maxim Levitsky 2021-01-21 15:00 ` [PATCH v2 0/3] VMX: more nested fixes Paolo Bonzini 3 siblings, 1 reply; 15+ messages in thread From: Maxim Levitsky @ 2021-01-14 20:54 UTC (permalink / raw) To: kvm Cc: Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, Sean Christopherson, linux-kernel, Jim Mattson, Maxim Levitsky This is very helpful for debugging nested VMX issues. Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- arch/x86/kvm/trace.h | 30 ++++++++++++++++++++++++++++++ arch/x86/kvm/vmx/nested.c | 6 ++++++ arch/x86/kvm/x86.c | 1 + 3 files changed, 37 insertions(+) diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h index 2de30c20bc264..663d1b1d8bf64 100644 --- a/arch/x86/kvm/trace.h +++ b/arch/x86/kvm/trace.h @@ -554,6 +554,36 @@ TRACE_EVENT(kvm_nested_vmrun, __entry->npt ? "on" : "off") ); + +/* + * Tracepoint for nested VMLAUNCH/VMRESUME + */ +TRACE_EVENT(kvm_nested_vmlaunch_resume, + TP_PROTO(__u64 rip, __u64 vmcs, __u64 nested_rip, + __u32 entry_intr_info), + TP_ARGS(rip, vmcs, nested_rip, entry_intr_info), + + TP_STRUCT__entry( + __field( __u64, rip ) + __field( __u64, vmcs ) + __field( __u64, nested_rip ) + __field( __u32, entry_intr_info ) + ), + + TP_fast_assign( + __entry->rip = rip; + __entry->vmcs = vmcs; + __entry->nested_rip = nested_rip; + __entry->entry_intr_info = entry_intr_info; + ), + + TP_printk("rip: 0x%016llx vmcs: 0x%016llx nrip: 0x%016llx " + "entry_intr_info: 0x%08x", + __entry->rip, __entry->vmcs, __entry->nested_rip, + __entry->entry_intr_info) +); + + TRACE_EVENT(kvm_nested_intercepts, TP_PROTO(__u16 cr_read, __u16 cr_write, __u32 exceptions, __u32 intercept1, __u32 intercept2, __u32 intercept3), diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 776688f9d1017..cd51b66480d52 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -3327,6 +3327,12 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)) vmx->nested.vmcs01_guest_bndcfgs = vmcs_read64(GUEST_BNDCFGS); + trace_kvm_nested_vmlaunch_resume(kvm_rip_read(vcpu), + vmx->nested.current_vmptr, + vmcs12->guest_rip, + vmcs12->vm_entry_intr_info_field); + + /* * Overwrite vmcs01.GUEST_CR3 with L1's CR3 if EPT is disabled *and* * nested early checks are disabled. In the event of a "late" VM-Fail, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index a480804ae27a3..7c6e94e32100e 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -11562,6 +11562,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_inj_virq); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_page_fault); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_msr); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_cr); +EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmlaunch_resume); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmrun); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit); EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit_inject); -- 2.26.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint 2021-01-14 20:54 ` [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint Maxim Levitsky @ 2021-01-15 0:14 ` Sean Christopherson 2021-01-15 13:48 ` Paolo Bonzini 2021-01-21 17:02 ` Maxim Levitsky 0 siblings, 2 replies; 15+ messages in thread From: Sean Christopherson @ 2021-01-15 0:14 UTC (permalink / raw) To: Maxim Levitsky Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On Thu, Jan 14, 2021, Maxim Levitsky wrote: > This is very helpful for debugging nested VMX issues. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/trace.h | 30 ++++++++++++++++++++++++++++++ > arch/x86/kvm/vmx/nested.c | 6 ++++++ > arch/x86/kvm/x86.c | 1 + > 3 files changed, 37 insertions(+) > > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h > index 2de30c20bc264..663d1b1d8bf64 100644 > --- a/arch/x86/kvm/trace.h > +++ b/arch/x86/kvm/trace.h > @@ -554,6 +554,36 @@ TRACE_EVENT(kvm_nested_vmrun, > __entry->npt ? "on" : "off") > ); > > + > +/* > + * Tracepoint for nested VMLAUNCH/VMRESUME VM-Enter, as below. > + */ > +TRACE_EVENT(kvm_nested_vmlaunch_resume, s/vmlaunc_resume/vmenter, both for consistency with other code and so that it can sanely be reused by SVM. IMO, trace_kvm_entry is wrong :-). > + TP_PROTO(__u64 rip, __u64 vmcs, __u64 nested_rip, > + __u32 entry_intr_info), > + TP_ARGS(rip, vmcs, nested_rip, entry_intr_info), > + > + TP_STRUCT__entry( > + __field( __u64, rip ) > + __field( __u64, vmcs ) > + __field( __u64, nested_rip ) > + __field( __u32, entry_intr_info ) > + ), > + > + TP_fast_assign( > + __entry->rip = rip; > + __entry->vmcs = vmcs; > + __entry->nested_rip = nested_rip; > + __entry->entry_intr_info = entry_intr_info; > + ), > + > + TP_printk("rip: 0x%016llx vmcs: 0x%016llx nrip: 0x%016llx " > + "entry_intr_info: 0x%08x", > + __entry->rip, __entry->vmcs, __entry->nested_rip, > + __entry->entry_intr_info) > +); > + > + > TRACE_EVENT(kvm_nested_intercepts, > TP_PROTO(__u16 cr_read, __u16 cr_write, __u32 exceptions, > __u32 intercept1, __u32 intercept2, __u32 intercept3), > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 776688f9d1017..cd51b66480d52 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -3327,6 +3327,12 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, > !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)) > vmx->nested.vmcs01_guest_bndcfgs = vmcs_read64(GUEST_BNDCFGS); > > + trace_kvm_nested_vmlaunch_resume(kvm_rip_read(vcpu), Hmm, won't this RIP be wrong for the migration case? I.e. it'll be L2, not L1 as is the case for the "true" nested VM-Enter path. > + vmx->nested.current_vmptr, > + vmcs12->guest_rip, > + vmcs12->vm_entry_intr_info_field); The placement is a bit funky. I assume you put it here so that calls from vmx_set_nested_state() also get traced. But, that also means vmx_pre_leave_smm() will get traced, and it also creates some weirdness where some nested VM-Enters that VM-Fail will get traced, but others will not. Tracing vmx_pre_leave_smm() isn't necessarily bad, but it could be confusing, especially if the debugger looks up the RIP and sees RSM. Ditto for the migration case. Not sure what would be a good answer. > + > + > /* > * Overwrite vmcs01.GUEST_CR3 with L1's CR3 if EPT is disabled *and* > * nested early checks are disabled. In the event of a "late" VM-Fail, > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index a480804ae27a3..7c6e94e32100e 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -11562,6 +11562,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_inj_virq); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_page_fault); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_msr); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_cr); > +EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmlaunch_resume); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmrun); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit); > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit_inject); > -- > 2.26.2 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint 2021-01-15 0:14 ` Sean Christopherson @ 2021-01-15 13:48 ` Paolo Bonzini 2021-01-15 16:30 ` Sean Christopherson 2021-01-21 16:58 ` Maxim Levitsky 2021-01-21 17:02 ` Maxim Levitsky 1 sibling, 2 replies; 15+ messages in thread From: Paolo Bonzini @ 2021-01-15 13:48 UTC (permalink / raw) To: Sean Christopherson, Maxim Levitsky Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On 15/01/21 01:14, Sean Christopherson wrote: >> + trace_kvm_nested_vmlaunch_resume(kvm_rip_read(vcpu), > Hmm, won't this RIP be wrong for the migration case? I.e. it'll be L2, not L1 > as is the case for the "true" nested VM-Enter path. It will be the previous RIP---might as well be 0xfffffff0 depending on what userspace does. I don't think you can do much better than that, using vmcs12->host_rip would be confusing in the SMM case. >> + vmx->nested.current_vmptr, >> + vmcs12->guest_rip, >> + vmcs12->vm_entry_intr_info_field); > The placement is a bit funky. I assume you put it here so that calls from > vmx_set_nested_state() also get traced. But, that also means > vmx_pre_leave_smm() will get traced, and it also creates some weirdness where > some nested VM-Enters that VM-Fail will get traced, but others will not. > > Tracing vmx_pre_leave_smm() isn't necessarily bad, but it could be confusing, > especially if the debugger looks up the RIP and sees RSM. Ditto for the > migration case. Actually tracing vmx_pre_leave_smm() is good, and pointing to RSM makes sense so I'm not worried about that. Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint 2021-01-15 13:48 ` Paolo Bonzini @ 2021-01-15 16:30 ` Sean Christopherson 2021-01-21 17:00 ` Maxim Levitsky 2021-01-21 16:58 ` Maxim Levitsky 1 sibling, 1 reply; 15+ messages in thread From: Sean Christopherson @ 2021-01-15 16:30 UTC (permalink / raw) To: Paolo Bonzini Cc: Maxim Levitsky, kvm, Thomas Gleixner, x86, Borislav Petkov, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On Fri, Jan 15, 2021, Paolo Bonzini wrote: > On 15/01/21 01:14, Sean Christopherson wrote: > > > + trace_kvm_nested_vmlaunch_resume(kvm_rip_read(vcpu), > > Hmm, won't this RIP be wrong for the migration case? I.e. it'll be L2, not L1 > > as is the case for the "true" nested VM-Enter path. > > It will be the previous RIP---might as well be 0xfffffff0 depending on what > userspace does. I don't think you can do much better than that, using > vmcs12->host_rip would be confusing in the SMM case. > > > > + vmx->nested.current_vmptr, > > > + vmcs12->guest_rip, > > > + vmcs12->vm_entry_intr_info_field); > > The placement is a bit funky. I assume you put it here so that calls from > > vmx_set_nested_state() also get traced. But, that also means > > vmx_pre_leave_smm() will get traced, and it also creates some weirdness where > > some nested VM-Enters that VM-Fail will get traced, but others will not. > > > > Tracing vmx_pre_leave_smm() isn't necessarily bad, but it could be confusing, > > especially if the debugger looks up the RIP and sees RSM. Ditto for the > > migration case. > > Actually tracing vmx_pre_leave_smm() is good, and pointing to RSM makes > sense so I'm not worried about that. Ideally there would something in the tracepoint to differentiate the various cases. Not that the RSM/migration cases will pop up often, but I think it's an easily solved problem that could avoid confusion. What if we captured vmx->nested.smm.guest_mode and from_vmentry, and explicitly record what triggered the entry? TP_printk("from: %s rip: 0x%016llx vmcs: 0x%016llx nrip: 0x%016llx intr_info: 0x%08x", __entry->vmenter ? "VM-Enter" : __entry->smm ? "RSM" : "SET_STATE", __entry->rip, __entry->vmcs, __entry->nested_rip, __entry->entry_intr_info Side topic, can we have an "official" ruling on whether KVM tracepoints should use colons and/or commas? And probably same question for whether or not to prepend zeros. E.g. kvm_entry has "vcpu %u, rip 0x%lx" versus "rip: 0x%016llx vmcs: 0x%016llx". It bugs me that we're so inconsistent. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint 2021-01-15 16:30 ` Sean Christopherson @ 2021-01-21 17:00 ` Maxim Levitsky 0 siblings, 0 replies; 15+ messages in thread From: Maxim Levitsky @ 2021-01-21 17:00 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On Fri, 2021-01-15 at 08:30 -0800, Sean Christopherson wrote: > On Fri, Jan 15, 2021, Paolo Bonzini wrote: > > On 15/01/21 01:14, Sean Christopherson wrote: > > > > + trace_kvm_nested_vmlaunch_resume(kvm_rip_read(vcpu), > > > Hmm, won't this RIP be wrong for the migration case? I.e. it'll be L2, not L1 > > > as is the case for the "true" nested VM-Enter path. > > > > It will be the previous RIP---might as well be 0xfffffff0 depending on what > > userspace does. I don't think you can do much better than that, using > > vmcs12->host_rip would be confusing in the SMM case. > > > > > > + vmx->nested.current_vmptr, > > > > + vmcs12->guest_rip, > > > > + vmcs12->vm_entry_intr_info_field); > > > The placement is a bit funky. I assume you put it here so that calls from > > > vmx_set_nested_state() also get traced. But, that also means > > > vmx_pre_leave_smm() will get traced, and it also creates some weirdness where > > > some nested VM-Enters that VM-Fail will get traced, but others will not. > > > > > > Tracing vmx_pre_leave_smm() isn't necessarily bad, but it could be confusing, > > > especially if the debugger looks up the RIP and sees RSM. Ditto for the > > > migration case. > > > > Actually tracing vmx_pre_leave_smm() is good, and pointing to RSM makes > > sense so I'm not worried about that. > > Ideally there would something in the tracepoint to differentiate the various > cases. Not that the RSM/migration cases will pop up often, but I think it's an > easily solved problem that could avoid confusion. > > What if we captured vmx->nested.smm.guest_mode and from_vmentry, and explicitly > record what triggered the entry? > > TP_printk("from: %s rip: 0x%016llx vmcs: 0x%016llx nrip: 0x%016llx intr_info: 0x%08x", > __entry->vmenter ? "VM-Enter" : __entry->smm ? "RSM" : "SET_STATE", > __entry->rip, __entry->vmcs, __entry->nested_rip, > __entry->entry_intr_info I think that this is a good idea, but should be done in a separate patch. > > Side topic, can we have an "official" ruling on whether KVM tracepoints should > use colons and/or commas? And probably same question for whether or not to > prepend zeros. E.g. kvm_entry has "vcpu %u, rip 0x%lx" versus "rip: 0x%016llx > vmcs: 0x%016llx". It bugs me that we're so inconsistent. > As I said the kvm tracing has a lot of things that can be imporoved, and as it is often the only way to figure out complex bugs as these I had to deal with recently, I will do more improvements in this area as time permits. Best regards, Maxim Levitsky ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint 2021-01-15 13:48 ` Paolo Bonzini 2021-01-15 16:30 ` Sean Christopherson @ 2021-01-21 16:58 ` Maxim Levitsky 1 sibling, 0 replies; 15+ messages in thread From: Maxim Levitsky @ 2021-01-21 16:58 UTC (permalink / raw) To: Paolo Bonzini, Sean Christopherson Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On Fri, 2021-01-15 at 14:48 +0100, Paolo Bonzini wrote: > On 15/01/21 01:14, Sean Christopherson wrote: > > > + trace_kvm_nested_vmlaunch_resume(kvm_rip_read(vcpu), > > Hmm, won't this RIP be wrong for the migration case? I.e. it'll be L2, not L1 > > as is the case for the "true" nested VM-Enter path. Actually in this case, the initial RIP of 0x000000000000fff0 will be printed which isn't that bad. A tracepoint in nested state load function would be very nice to add to mark this explicitly. I'll do this later. > > It will be the previous RIP---might as well be 0xfffffff0 depending on > what userspace does. I don't think you can do much better than that, > using vmcs12->host_rip would be confusing in the SMM case. > > > > + vmx->nested.current_vmptr, > > > + vmcs12->guest_rip, > > > + vmcs12->vm_entry_intr_info_field); > > The placement is a bit funky. I assume you put it here so that calls from > > vmx_set_nested_state() also get traced. But, that also means > > vmx_pre_leave_smm() will get traced, and it also creates some weirdness where > > some nested VM-Enters that VM-Fail will get traced, but others will not. > > > > Tracing vmx_pre_leave_smm() isn't necessarily bad, but it could be confusing, > > especially if the debugger looks up the RIP and sees RSM. Ditto for the > > migration case. > > Actually tracing vmx_pre_leave_smm() is good, and pointing to RSM makes > sense so I'm not worried about that. > > Paolo > I agree with that and indeed this was my intention. In fact I will change the svm's tracepoint to behave the same way in the next patch series (I'll move it to enter_svm_guest_mode). (When I wrote this patch I somehow thought that this is what SVM already does). Best regards, Maxim Levitsky ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint 2021-01-15 0:14 ` Sean Christopherson 2021-01-15 13:48 ` Paolo Bonzini @ 2021-01-21 17:02 ` Maxim Levitsky 1 sibling, 0 replies; 15+ messages in thread From: Maxim Levitsky @ 2021-01-21 17:02 UTC (permalink / raw) To: Sean Christopherson Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On Thu, 2021-01-14 at 16:14 -0800, Sean Christopherson wrote: > On Thu, Jan 14, 2021, Maxim Levitsky wrote: > > This is very helpful for debugging nested VMX issues. > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > arch/x86/kvm/trace.h | 30 ++++++++++++++++++++++++++++++ > > arch/x86/kvm/vmx/nested.c | 6 ++++++ > > arch/x86/kvm/x86.c | 1 + > > 3 files changed, 37 insertions(+) > > > > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h > > index 2de30c20bc264..663d1b1d8bf64 100644 > > --- a/arch/x86/kvm/trace.h > > +++ b/arch/x86/kvm/trace.h > > @@ -554,6 +554,36 @@ TRACE_EVENT(kvm_nested_vmrun, > > __entry->npt ? "on" : "off") > > ); > > > > + > > +/* > > + * Tracepoint for nested VMLAUNCH/VMRESUME > > VM-Enter, as below. Will do > > > + */ > > +TRACE_EVENT(kvm_nested_vmlaunch_resume, > > s/vmlaunc_resume/vmenter, both for consistency with other code and so that it > can sanely be reused by SVM. IMO, trace_kvm_entry is wrong :-). SVM already has trace_kvm_nested_vmrun and it contains some SVM specific stuff that doesn't exist on VMX and vise versa. So I do want to keep these trace points separate. > > > + TP_PROTO(__u64 rip, __u64 vmcs, __u64 nested_rip, > > + __u32 entry_intr_info), > > + TP_ARGS(rip, vmcs, nested_rip, entry_intr_info), > > + > > + TP_STRUCT__entry( > > + __field( __u64, rip ) > > + __field( __u64, vmcs ) > > + __field( __u64, nested_rip ) > > + __field( __u32, entry_intr_info ) > > + ), > > + > > + TP_fast_assign( > > + __entry->rip = rip; > > + __entry->vmcs = vmcs; > > + __entry->nested_rip = nested_rip; > > + __entry->entry_intr_info = entry_intr_info; > > + ), > > + > > + TP_printk("rip: 0x%016llx vmcs: 0x%016llx nrip: 0x%016llx " > > + "entry_intr_info: 0x%08x", > > + __entry->rip, __entry->vmcs, __entry->nested_rip, > > + __entry->entry_intr_info) > > +); > > + > > + > > TRACE_EVENT(kvm_nested_intercepts, > > TP_PROTO(__u16 cr_read, __u16 cr_write, __u32 exceptions, > > __u32 intercept1, __u32 intercept2, __u32 intercept3), > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > index 776688f9d1017..cd51b66480d52 100644 > > --- a/arch/x86/kvm/vmx/nested.c > > +++ b/arch/x86/kvm/vmx/nested.c > > @@ -3327,6 +3327,12 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, > > !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)) > > vmx->nested.vmcs01_guest_bndcfgs = vmcs_read64(GUEST_BNDCFGS); > > > > + trace_kvm_nested_vmlaunch_resume(kvm_rip_read(vcpu), > > Hmm, won't this RIP be wrong for the migration case? I.e. it'll be L2, not L1 > as is the case for the "true" nested VM-Enter path. > > > + vmx->nested.current_vmptr, > > + vmcs12->guest_rip, > > + vmcs12->vm_entry_intr_info_field); > > The placement is a bit funky. I assume you put it here so that calls from > vmx_set_nested_state() also get traced. But, that also means > vmx_pre_leave_smm() will get traced, and it also creates some weirdness where > some nested VM-Enters that VM-Fail will get traced, but others will not. > > Tracing vmx_pre_leave_smm() isn't necessarily bad, but it could be confusing, > especially if the debugger looks up the RIP and sees RSM. Ditto for the > migration case. > > Not sure what would be a good answer. > > > + > > + > > /* > > * Overwrite vmcs01.GUEST_CR3 with L1's CR3 if EPT is disabled *and* > > * nested early checks are disabled. In the event of a "late" VM-Fail, > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index a480804ae27a3..7c6e94e32100e 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -11562,6 +11562,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_inj_virq); > > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_page_fault); > > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_msr); > > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_cr); > > +EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmlaunch_resume); > > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmrun); > > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit); > > EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_nested_vmexit_inject); > > -- > > 2.26.2 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] KVM: VMX: read idt_vectoring_info a bit earlier 2021-01-14 20:54 [PATCH v2 0/3] VMX: more nested fixes Maxim Levitsky 2021-01-14 20:54 ` [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration Maxim Levitsky 2021-01-14 20:54 ` [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint Maxim Levitsky @ 2021-01-14 20:54 ` Maxim Levitsky 2021-01-15 0:29 ` Sean Christopherson 2021-01-21 15:00 ` [PATCH v2 0/3] VMX: more nested fixes Paolo Bonzini 3 siblings, 1 reply; 15+ messages in thread From: Maxim Levitsky @ 2021-01-14 20:54 UTC (permalink / raw) To: kvm Cc: Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, Sean Christopherson, linux-kernel, Jim Mattson, Maxim Levitsky This allows it to be printed correctly by the trace print that follows. Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- arch/x86/kvm/vmx/vmx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 2af05d3b05909..9b6e7dbf5e2bd 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -6771,6 +6771,8 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) } vmx->exit_reason = vmcs_read32(VM_EXIT_REASON); + vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); + if (unlikely((u16)vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY)) kvm_machine_check(); @@ -6780,7 +6782,6 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) return EXIT_FASTPATH_NONE; vmx->loaded_vmcs->launched = 1; - vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); vmx_recover_nmi_blocking(vmx); vmx_complete_interrupts(vmx); -- 2.26.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] KVM: VMX: read idt_vectoring_info a bit earlier 2021-01-14 20:54 ` [PATCH v2 3/3] KVM: VMX: read idt_vectoring_info a bit earlier Maxim Levitsky @ 2021-01-15 0:29 ` Sean Christopherson 2021-01-21 17:04 ` Maxim Levitsky 0 siblings, 1 reply; 15+ messages in thread From: Sean Christopherson @ 2021-01-15 0:29 UTC (permalink / raw) To: Maxim Levitsky Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On Thu, Jan 14, 2021, Maxim Levitsky wrote: > This allows it to be printed correctly by the trace print It'd be helpful to explicitly say which tracepoint, and explain that the value is read by vmx_get_exit_info(). It's far from obvious how this gets consumed. > that follows. > Fixes: dcf068da7eb2 ("KVM: VMX: Introduce generic fastpath handler") > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/vmx/vmx.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 2af05d3b05909..9b6e7dbf5e2bd 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -6771,6 +6771,8 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) > } > > vmx->exit_reason = vmcs_read32(VM_EXIT_REASON); > + vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); Hrm, it probably makes sense to either do the VMREAD conditionally, or to zero idt_vectoring_info in the vmx->fail path. I don't care about the cycles on VM-Exit consistency checks, just that this would hide that the field is valid if and only if VM-Enter fully succeeded. A third option would be to add a comment saying that it's unnecessary if VM-Enter failed, but faster in the common case to just do the VMREAD. diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 2af05d3b0590..3c172c05570a 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -6774,13 +6774,15 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) if (unlikely((u16)vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY)) kvm_machine_check(); + if (likely(!(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY))) + vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); + trace_kvm_exit(vmx->exit_reason, vcpu, KVM_ISA_VMX); if (unlikely(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY)) return EXIT_FASTPATH_NONE; vmx->loaded_vmcs->launched = 1; - vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); vmx_recover_nmi_blocking(vmx); vmx_complete_interrupts(vmx); > + > if (unlikely((u16)vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY)) > kvm_machine_check(); > > @@ -6780,7 +6782,6 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) > return EXIT_FASTPATH_NONE; > > vmx->loaded_vmcs->launched = 1; > - vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); > > vmx_recover_nmi_blocking(vmx); > vmx_complete_interrupts(vmx); > -- > 2.26.2 > ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] KVM: VMX: read idt_vectoring_info a bit earlier 2021-01-15 0:29 ` Sean Christopherson @ 2021-01-21 17:04 ` Maxim Levitsky 0 siblings, 0 replies; 15+ messages in thread From: Maxim Levitsky @ 2021-01-21 17:04 UTC (permalink / raw) To: Sean Christopherson Cc: kvm, Thomas Gleixner, x86, Borislav Petkov, Paolo Bonzini, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, linux-kernel, Jim Mattson On Thu, 2021-01-14 at 16:29 -0800, Sean Christopherson wrote: > On Thu, Jan 14, 2021, Maxim Levitsky wrote: > > This allows it to be printed correctly by the trace print > > It'd be helpful to explicitly say which tracepoint, and explain that the value > is read by vmx_get_exit_info(). It's far from obvious how this gets consumed. > > > that follows. > > > > Fixes: dcf068da7eb2 ("KVM: VMX: Introduce generic fastpath handler") > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > arch/x86/kvm/vmx/vmx.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > > index 2af05d3b05909..9b6e7dbf5e2bd 100644 > > --- a/arch/x86/kvm/vmx/vmx.c > > +++ b/arch/x86/kvm/vmx/vmx.c > > @@ -6771,6 +6771,8 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) > > } > > > > vmx->exit_reason = vmcs_read32(VM_EXIT_REASON); > > + vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); > > Hrm, it probably makes sense to either do the VMREAD conditionally, or to > zero idt_vectoring_info in the vmx->fail path. I don't care about the cycles > on VM-Exit consistency checks, just that this would hide that the field is valid > if and only if VM-Enter fully succeeded. A third option would be to add a > comment saying that it's unnecessary if VM-Enter failed, but faster in the > common case to just do the VMREAD. Allright, I will add this. Best regards, Maxim Levitsky > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 2af05d3b0590..3c172c05570a 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -6774,13 +6774,15 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) > if (unlikely((u16)vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY)) > kvm_machine_check(); > > + if (likely(!(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY))) > + vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); > + > trace_kvm_exit(vmx->exit_reason, vcpu, KVM_ISA_VMX); > > if (unlikely(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY)) > return EXIT_FASTPATH_NONE; > > vmx->loaded_vmcs->launched = 1; > - vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); > > vmx_recover_nmi_blocking(vmx); > vmx_complete_interrupts(vmx); > > > > + > > if (unlikely((u16)vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY)) > > kvm_machine_check(); > > > > @@ -6780,7 +6782,6 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu) > > return EXIT_FASTPATH_NONE; > > > > vmx->loaded_vmcs->launched = 1; > > - vmx->idt_vectoring_info = vmcs_read32(IDT_VECTORING_INFO_FIELD); > > > > vmx_recover_nmi_blocking(vmx); > > vmx_complete_interrupts(vmx); > > -- > > 2.26.2 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/3] VMX: more nested fixes 2021-01-14 20:54 [PATCH v2 0/3] VMX: more nested fixes Maxim Levitsky ` (2 preceding siblings ...) 2021-01-14 20:54 ` [PATCH v2 3/3] KVM: VMX: read idt_vectoring_info a bit earlier Maxim Levitsky @ 2021-01-21 15:00 ` Paolo Bonzini 3 siblings, 0 replies; 15+ messages in thread From: Paolo Bonzini @ 2021-01-21 15:00 UTC (permalink / raw) To: Maxim Levitsky, kvm Cc: Thomas Gleixner, x86, Borislav Petkov, Ingo Molnar, Vitaly Kuznetsov, Joerg Roedel, Wanpeng Li, H. Peter Anvin, Sean Christopherson, linux-kernel, Jim Mattson On 14/01/21 21:54, Maxim Levitsky wrote: > This is hopefully the last fix for VMX nested migration > that finally allows my stress test of migration with a nested guest to pass. > > In a nutshell after an optimization that was done in commit 7952d769c29ca, > some of vmcs02 fields which can be modified by the L2 freely while it runs > (like GSBASE and such) were not copied back to vmcs12 unless: > > 1. L1 tries to vmread them (update done on intercept) > 2. vmclear or vmldptr on other vmcs are done. > 3. nested state is read and nested guest is running. > > What wasn't done was to sync these 'rare' fields when L1 is running > but still has a loaded vmcs12 which might have some stale fields, > if that vmcs was used to enter a guest already due to that optimization. > > Plus I added two minor patches to improve VMX tracepoints > a bit. There is still a large room for improvement. > > Best regards, > Maxim Levitsky > > Maxim Levitsky (3): > KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration > KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint > KVM: VMX: read idt_vectoring_info a bit earlier > > arch/x86/kvm/trace.h | 30 ++++++++++++++++++++++++++++++ > arch/x86/kvm/vmx/nested.c | 19 ++++++++++++++----- > arch/x86/kvm/vmx/vmx.c | 3 ++- > arch/x86/kvm/x86.c | 1 + > 4 files changed, 47 insertions(+), 6 deletions(-) > I committed patch 1 since I need to send it out to Linus quite soonish, but please adjust and resend the others based on Sean's review. Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-01-21 17:13 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-14 20:54 [PATCH v2 0/3] VMX: more nested fixes Maxim Levitsky 2021-01-14 20:54 ` [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration Maxim Levitsky 2021-01-14 23:58 ` Sean Christopherson 2021-01-21 14:59 ` Paolo Bonzini 2021-01-14 20:54 ` [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint Maxim Levitsky 2021-01-15 0:14 ` Sean Christopherson 2021-01-15 13:48 ` Paolo Bonzini 2021-01-15 16:30 ` Sean Christopherson 2021-01-21 17:00 ` Maxim Levitsky 2021-01-21 16:58 ` Maxim Levitsky 2021-01-21 17:02 ` Maxim Levitsky 2021-01-14 20:54 ` [PATCH v2 3/3] KVM: VMX: read idt_vectoring_info a bit earlier Maxim Levitsky 2021-01-15 0:29 ` Sean Christopherson 2021-01-21 17:04 ` Maxim Levitsky 2021-01-21 15:00 ` [PATCH v2 0/3] VMX: more nested fixes Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).