diff mbox series

[SRU,Bionic,1/1] KVM: nVMX: Always reflect #NM VM-exits to L1

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

Commit Message

Thadeu Lima de Souza Cascardo Dec. 3, 2020, 8:33 p.m. UTC 
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(-)

Comments

Stefan Bader Dec. 4, 2020, 8:35 a.m. UTC | #1 
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))
>
Ian May Dec. 8, 2020, 4:11 p.m. UTC | #2 
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 mbox series

Patch

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))