> On 13 Sep 2018, at 21:54, Jim Mattson <jmattson@xxxxxxxxxx> wrote:
>
> 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
Surprised this wasn’t noticed when
commit ccf9844e5d99 ("kvm, vmx: Really fix lazy FPU on nested guest”)
was reviewed. Funny it didn’t even “Really” fixed lazy FPU on nested guest :P.
Should always be careful with commit titles...
> 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@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> Reviewed-by: Peter Shier <pshier@xxxxxxxxxx>
> Tested-by: Abhiroop Dabral <adabral@xxxxxxxxxxxxxxxxxxxx>
> ---
> 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 533a327372c8..49364b791eb9 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1610,11 +1610,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);
> @@ -9639,9 +9634,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.19.0.rc2.392.g5ba43deb5a-goog
>
Reviewed-by: Liran Alon <liran.alon@xxxxxxxxxx>
I think it will also be a good idea to add a regression kvm-unit-tests to vmx_tests.c
as this should be fairly simple.
-Liran
