Message ID | 20201203203337.2724594-2-cascardo@canonical.com |
---|---|
State | New |
Headers | show |
Series | [SRU,Bionic,1/1] KVM: nVMX: Always reflect #NM VM-exits to L1 | expand |
On 03.12.20 21:33, Thadeu Lima de Souza Cascardo wrote: > From: Jim Mattson <jmattson@google.com> > > BugLink: https://bugs.launchpad.net/bugs/1872401 > > When bit 3 (corresponding to CR0.TS) of the VMCS12 cr0_guest_host_mask > field is clear, the VMCS12 guest_cr0 field does not necessarily hold > the current value of the L2 CR0.TS bit, so the code that checked for > L2's CR0.TS bit being set was incorrect. Moreover, I'm not sure that > the CR0.TS check was adequate. (What if L2's CR0.EM was set, for > instance?) > > Fortunately, lazy FPU has gone away, so L0 has lost all interest in > intercepting #NM exceptions. See commit bd7e5b0899a4 ("KVM: x86: > remove code for lazy FPU handling"). Therefore, there is no longer any > question of which hypervisor gets first dibs. The #NM VM-exit should > always be reflected to L1. (Note that the corresponding bit must be > set in the VMCS12 exception_bitmap field for there to be an #NM > VM-exit at all.) > > Fixes: ccf9844e5d99c ("kvm, vmx: Really fix lazy FPU on nested guest") > Reported-by: Abhiroop Dabral <adabral@paloaltonetworks.com> > Signed-off-by: Jim Mattson <jmattson@google.com> > Reviewed-by: Peter Shier <pshier@google.com> > Tested-by: Abhiroop Dabral <adabral@paloaltonetworks.com> > Reviewed-by: Liran Alon <liran.alon@oracle.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (cherry picked from commit 3c6e099fa15fdb6fb1892199ed8709012e1294f2) > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > arch/x86/kvm/vmx.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index dd940ac9cf88..2773499f3578 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -1215,11 +1215,6 @@ static inline bool is_page_fault(u32 intr_info) > return is_exception_n(intr_info, PF_VECTOR); > } > > -static inline bool is_no_device(u32 intr_info) > -{ > - return is_exception_n(intr_info, NM_VECTOR); > -} > - > static inline bool is_invalid_opcode(u32 intr_info) > { > return is_exception_n(intr_info, UD_VECTOR); > @@ -8726,9 +8721,6 @@ static bool nested_vmx_exit_reflected(struct kvm_vcpu *vcpu, u32 exit_reason) > return false; > else if (is_page_fault(intr_info)) > return !vmx->vcpu.arch.apf.host_apf_reason && enable_ept; > - else if (is_no_device(intr_info) && > - !(vmcs12->guest_cr0 & X86_CR0_TS)) > - return false; > else if (is_debug(intr_info) && > vcpu->guest_debug & > (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) >
Applied to bionic/linux Thanks, Ian On 2020-12-03 17:33:37 , Thadeu Lima de Souza Cascardo wrote: > From: Jim Mattson <jmattson@google.com> > > BugLink: https://bugs.launchpad.net/bugs/1872401 > > When bit 3 (corresponding to CR0.TS) of the VMCS12 cr0_guest_host_mask > field is clear, the VMCS12 guest_cr0 field does not necessarily hold > the current value of the L2 CR0.TS bit, so the code that checked for > L2's CR0.TS bit being set was incorrect. Moreover, I'm not sure that > the CR0.TS check was adequate. (What if L2's CR0.EM was set, for > instance?) > > Fortunately, lazy FPU has gone away, so L0 has lost all interest in > intercepting #NM exceptions. See commit bd7e5b0899a4 ("KVM: x86: > remove code for lazy FPU handling"). Therefore, there is no longer any > question of which hypervisor gets first dibs. The #NM VM-exit should > always be reflected to L1. (Note that the corresponding bit must be > set in the VMCS12 exception_bitmap field for there to be an #NM > VM-exit at all.) > > Fixes: ccf9844e5d99c ("kvm, vmx: Really fix lazy FPU on nested guest") > Reported-by: Abhiroop Dabral <adabral@paloaltonetworks.com> > Signed-off-by: Jim Mattson <jmattson@google.com> > Reviewed-by: Peter Shier <pshier@google.com> > Tested-by: Abhiroop Dabral <adabral@paloaltonetworks.com> > Reviewed-by: Liran Alon <liran.alon@oracle.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (cherry picked from commit 3c6e099fa15fdb6fb1892199ed8709012e1294f2) > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > --- > arch/x86/kvm/vmx.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index dd940ac9cf88..2773499f3578 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -1215,11 +1215,6 @@ static inline bool is_page_fault(u32 intr_info) > return is_exception_n(intr_info, PF_VECTOR); > } > > -static inline bool is_no_device(u32 intr_info) > -{ > - return is_exception_n(intr_info, NM_VECTOR); > -} > - > static inline bool is_invalid_opcode(u32 intr_info) > { > return is_exception_n(intr_info, UD_VECTOR); > @@ -8726,9 +8721,6 @@ static bool nested_vmx_exit_reflected(struct kvm_vcpu *vcpu, u32 exit_reason) > return false; > else if (is_page_fault(intr_info)) > return !vmx->vcpu.arch.apf.host_apf_reason && enable_ept; > - else if (is_no_device(intr_info) && > - !(vmcs12->guest_cr0 & X86_CR0_TS)) > - return false; > else if (is_debug(intr_info) && > vcpu->guest_debug & > (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) > -- > 2.27.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index dd940ac9cf88..2773499f3578 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -1215,11 +1215,6 @@ static inline bool is_page_fault(u32 intr_info) return is_exception_n(intr_info, PF_VECTOR); } -static inline bool is_no_device(u32 intr_info) -{ - return is_exception_n(intr_info, NM_VECTOR); -} - static inline bool is_invalid_opcode(u32 intr_info) { return is_exception_n(intr_info, UD_VECTOR); @@ -8726,9 +8721,6 @@ static bool nested_vmx_exit_reflected(struct kvm_vcpu *vcpu, u32 exit_reason) return false; else if (is_page_fault(intr_info)) return !vmx->vcpu.arch.apf.host_apf_reason && enable_ept; - else if (is_no_device(intr_info) && - !(vmcs12->guest_cr0 & X86_CR0_TS)) - return false; else if (is_debug(intr_info) && vcpu->guest_debug & (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))