| Message ID | 20190823010709.24879-6-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Remove emulation_result enums | expand |
Sean Christopherson <sean.j.christopherson@intel.com> writes: > The VMware backdoor hooks #GP faults on IN{S}, OUT{S}, and RDPMC, none > of which generate a non-zero error code for their #GP. Re-injecting #GP > instead of attempting emulation on a non-zero error code will allow a > future patch to move #GP injection (for emulation failure) into > kvm_emulate_instruction() without having to plumb in the error code. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> (I just need to get this off my chest) There was a long-standing issue with #GP interception: in case the exception has nothing to do with VMware we were getting into infinite loop of #GPs (and not #GP -> #DF -> #TF), e.g. here is a trace of platform_info selftest: <...>-43752 [001] 3615.602298: kvm_exit: reason EXIT_MSR rip 0x4015c2 info 0 0 <...>-43752 [001] 3615.602299: kvm_msr: msr_read ce = 0x0 (#GP) <...>-43752 [001] 3615.602300: kvm_inj_exception: #GP (0x0) <...>-43752 [001] 3615.602301: kvm_entry: vcpu 0 <...>-43752 [001] 3615.602302: kvm_exit: reason EXIT_EXCP_GP rip 0x4015c2 info 6a 0 <...>-43752 [001] 3615.602308: kvm_emulate_insn: 0:4015c2: 0f 32 <...>-43752 [001] 3615.602308: kvm_inj_exception: #GP (0x6a) <...>-43752 [001] 3615.602309: kvm_entry: vcpu 0 <...>-43752 [001] 3615.602310: kvm_exit: reason EXIT_EXCP_GP rip 0x4015c2 info 6a 0 <...>-43752 [001] 3615.602312: kvm_emulate_insn: 0:4015c2: 0f 32 <...>-43752 [001] 3615.602312: kvm_inj_exception: #GP (0x6a) <...>-43752 [001] 3615.602313: kvm_entry: vcpu 0 and so on. This commit fixes the issue as the second #GP has error code: <...>-52213 [006] 3740.739495: kvm_entry: vcpu 0 <...>-52213 [006] 3740.739496: kvm_exit: reason EXIT_MSR rip 0x4015c2 info 0 0 <...>-52213 [006] 3740.739497: kvm_msr: msr_read ce = 0x0 (#GP) <...>-52213 [006] 3740.739502: kvm_inj_exception: #GP (0x0) <...>-52213 [006] 3740.739503: kvm_entry: vcpu 0 <...>-52213 [006] 3740.739504: kvm_exit: reason EXIT_EXCP_GP rip 0x4015c2 info 6a 0 <...>-52213 [006] 3740.739505: kvm_inj_exception: #DF (0x0) <...>-52213 [006] 3740.739506: kvm_entry: vcpu 0 <...>-52213 [006] 3740.739507: kvm_exit: reason EXIT_EXCP_GP rip 0x4015c2 info 42 0 <...>-52213 [006] 3740.739508: kvm_fpu: unload <...>-52213 [006] 3740.739510: kvm_userspace_exit: reason KVM_EXIT_SHUTDOWN (8) I'm not exactly sure this covers all possible cases as there might be other cases when error code is not set but this is definitely an improvement. Reviewed-and-tested-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/svm.c | 6 +++++- > arch/x86/kvm/vmx/vmx.c | 7 ++++++- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 5a42f9c70014..b96a119690f4 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -2772,11 +2772,15 @@ static int gp_interception(struct vcpu_svm *svm) > > WARN_ON_ONCE(!enable_vmware_backdoor); > In case you'll be respinning for whatever reason, could you please add a short comment here (and vmx) saying something like "#GP interception for VMware backdoor emulation only handles IN{S}, OUT{S}, and RDPMC and none of these have a non-zero error code set" (I don't like the fact that we'll need to have two copies of it but I can't think of a better place for it). > + if (error_code) { > + kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); > + return 1; > + } > er = kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE); > if (er == EMULATE_USER_EXIT) > return 0; > else if (er != EMULATE_DONE) > - kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); > + kvm_queue_exception_e(vcpu, GP_VECTOR, 0); > return 1; > } > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 6ecf773825e2..3ee0dd304bc7 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -4509,11 +4509,16 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu) > > if (!vmx->rmode.vm86_active && is_gp_fault(intr_info)) { > WARN_ON_ONCE(!enable_vmware_backdoor); > + > + if (error_code) { > + kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); > + return 1; > + } > er = kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE); > if (er == EMULATE_USER_EXIT) > return 0; > else if (er != EMULATE_DONE) > - kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); > + kvm_queue_exception_e(vcpu, GP_VECTOR, 0); > return 1; > }
> On 23 Aug 2019, at 4:07, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > The VMware backdoor hooks #GP faults on IN{S}, OUT{S}, and RDPMC, none > of which generate a non-zero error code for their #GP. Re-injecting #GP > instead of attempting emulation on a non-zero error code will allow a > future patch to move #GP injection (for emulation failure) into > kvm_emulate_instruction() without having to plumb in the error code. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Reviewed-by: Liran Alon <liran.alon@oracle.com> -Liran > --- > arch/x86/kvm/svm.c | 6 +++++- > arch/x86/kvm/vmx/vmx.c | 7 ++++++- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 5a42f9c70014..b96a119690f4 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -2772,11 +2772,15 @@ static int gp_interception(struct vcpu_svm *svm) > > WARN_ON_ONCE(!enable_vmware_backdoor); > > + if (error_code) { > + kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); > + return 1; > + } > er = kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE); > if (er == EMULATE_USER_EXIT) > return 0; > else if (er != EMULATE_DONE) > - kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); > + kvm_queue_exception_e(vcpu, GP_VECTOR, 0); > return 1; > } > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 6ecf773825e2..3ee0dd304bc7 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -4509,11 +4509,16 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu) > > if (!vmx->rmode.vm86_active && is_gp_fault(intr_info)) { > WARN_ON_ONCE(!enable_vmware_backdoor); > + > + if (error_code) { > + kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); > + return 1; > + } > er = kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE); > if (er == EMULATE_USER_EXIT) > return 0; > else if (er != EMULATE_DONE) > - kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); > + kvm_queue_exception_e(vcpu, GP_VECTOR, 0); > return 1; > } > > -- > 2.22.0 >
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 5a42f9c70014..b96a119690f4 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2772,11 +2772,15 @@ static int gp_interception(struct vcpu_svm *svm) WARN_ON_ONCE(!enable_vmware_backdoor); + if (error_code) { + kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); + return 1; + } er = kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE); if (er == EMULATE_USER_EXIT) return 0; else if (er != EMULATE_DONE) - kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); + kvm_queue_exception_e(vcpu, GP_VECTOR, 0); return 1; } diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 6ecf773825e2..3ee0dd304bc7 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -4509,11 +4509,16 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu) if (!vmx->rmode.vm86_active && is_gp_fault(intr_info)) { WARN_ON_ONCE(!enable_vmware_backdoor); + + if (error_code) { + kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); + return 1; + } er = kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE); if (er == EMULATE_USER_EXIT) return 0; else if (er != EMULATE_DONE) - kvm_queue_exception_e(vcpu, GP_VECTOR, error_code); + kvm_queue_exception_e(vcpu, GP_VECTOR, 0); return 1; }
The VMware backdoor hooks #GP faults on IN{S}, OUT{S}, and RDPMC, none of which generate a non-zero error code for their #GP. Re-injecting #GP instead of attempting emulation on a non-zero error code will allow a future patch to move #GP injection (for emulation failure) into kvm_emulate_instruction() without having to plumb in the error code. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/svm.c | 6 +++++- arch/x86/kvm/vmx/vmx.c | 7 ++++++- 2 files changed, 11 insertions(+), 2 deletions(-)