Message ID | 20220301135526.136554-2-mlevitsk@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | SVM fixes + apic fix | expand |
On Tue, Mar 01, 2022, Maxim Levitsky wrote: > Use a dummy unused vmexit reason to mark the 'VM exit' that is happening > when kvm exits to handle SMM, which is not a real VM exit. Why not use "62h VMEXIT_SMI"? > This makes it a bit easier to read the KVM trace, and avoids > other potential problems. What other potential problems? > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > arch/x86/kvm/svm/svm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 7038c76fa8410..c08fd7f4f3414 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -4218,7 +4218,7 @@ static int svm_enter_smm(struct kvm_vcpu *vcpu, char *smstate) > svm->vmcb->save.rsp = vcpu->arch.regs[VCPU_REGS_RSP]; > svm->vmcb->save.rip = vcpu->arch.regs[VCPU_REGS_RIP]; > > - ret = nested_svm_vmexit(svm); > + ret = nested_svm_simple_vmexit(svm, SVM_EXIT_SW); > if (ret) > return ret; > > -- > 2.26.3 >
On Tue, 2022-03-01 at 16:31 +0000, Sean Christopherson wrote: > On Tue, Mar 01, 2022, Maxim Levitsky wrote: > > Use a dummy unused vmexit reason to mark the 'VM exit' that is happening > > when kvm exits to handle SMM, which is not a real VM exit. > > Why not use "62h VMEXIT_SMI"? Because VMEXIT_SMI is real vmexit which happens when L1 intercepts #SMI And here nested_svm_vmexit is abused to just exit guest mode without vmexit. > > > This makes it a bit easier to read the KVM trace, and avoids > > other potential problems. > > What other potential problems? The fact that we have a stale VM exit reason in vmcb without this patch which can be in theory consumed somewhere down the road. This stale vm exit reason also appears in the tracs which is very misleading. Best regards, Maxim Levitsky > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > arch/x86/kvm/svm/svm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > > index 7038c76fa8410..c08fd7f4f3414 100644 > > --- a/arch/x86/kvm/svm/svm.c > > +++ b/arch/x86/kvm/svm/svm.c > > @@ -4218,7 +4218,7 @@ static int svm_enter_smm(struct kvm_vcpu *vcpu, char *smstate) > > svm->vmcb->save.rsp = vcpu->arch.regs[VCPU_REGS_RSP]; > > svm->vmcb->save.rip = vcpu->arch.regs[VCPU_REGS_RIP]; > > > > - ret = nested_svm_vmexit(svm); > > + ret = nested_svm_simple_vmexit(svm, SVM_EXIT_SW); > > if (ret) > > return ret; > > > > -- > > 2.26.3 > >
On 3/1/22 18:13, Maxim Levitsky wrote: > The fact that we have a stale VM exit reason in vmcb without this > patch which can be in theory consumed somewhere down the road. > > This stale vm exit reason also appears in the tracs which is > very misleading. Let's put it in the commit message: This makes it a bit easier to read the KVM trace, and avoids other potential problems due to a stale vmexit reason in the vmcb. If SVM_EXIT_SW somehow reaches svm_invoke_exit_handler(), instead, svm_check_exit_valid() will return false and a WARN will be logged. Paolo
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 7038c76fa8410..c08fd7f4f3414 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -4218,7 +4218,7 @@ static int svm_enter_smm(struct kvm_vcpu *vcpu, char *smstate) svm->vmcb->save.rsp = vcpu->arch.regs[VCPU_REGS_RSP]; svm->vmcb->save.rip = vcpu->arch.regs[VCPU_REGS_RIP]; - ret = nested_svm_vmexit(svm); + ret = nested_svm_simple_vmexit(svm, SVM_EXIT_SW); if (ret) return ret;
Use a dummy unused vmexit reason to mark the 'VM exit' that is happening when kvm exits to handle SMM, which is not a real VM exit. This makes it a bit easier to read the KVM trace, and avoids other potential problems. Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- arch/x86/kvm/svm/svm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)