Re: [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS

[Jim Mattson]

On Thu, Aug 24, 2017 at 6:37 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> A guest may not be configured to support XSAVES/XRSTORS, even when the host
> does. If the guest does not support XSAVES/XRSTORS, clear the secondary
> execution control so that the processor will raise #UD.
>
> Also clear the "allowed-1" bit for "enable XSAVES/XRSTORS" in the
> IA32_VMX_PROCBASED_CTLS2 MSR, and pass through VMCS12's control in
> the VMCS02.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  arch/x86/kvm/vmx.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 954e26079cd6..2f1fd748d533 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1358,8 +1358,7 @@ static inline int nested_cpu_has_ept(struct vmcs12 *vmcs12)
>
>  static inline bool nested_cpu_has_xsaves(struct vmcs12 *vmcs12)
>  {
> -       return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES) &&
> -               vmx_xsaves_supported();
> +       return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES);
>  }
>
>  static inline bool nested_cpu_has_pml(struct vmcs12 *vmcs12)
> @@ -2823,8 +2822,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>                 SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
>                 SECONDARY_EXEC_APIC_REGISTER_VIRT |
>                 SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
> -               SECONDARY_EXEC_WBINVD_EXITING |
> -               SECONDARY_EXEC_XSAVES;
> +               SECONDARY_EXEC_WBINVD_EXITING;
>
>         if (enable_ept) {
>                 /* nested EPT: emulate EPT also to L1 */
> @@ -5319,6 +5317,21 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>         if (!enable_pml)
>                 exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>
> +       if (vmx_xsaves_supported()) {
> +               bool xsaves_enabled = guest_cpuid_has(vcpu, X86_FEATURE_XSAVES);

According to the XSAVES documentation in volume 2 of the SDM, XSAVES
will raise #UD if *either* CPUID.01H:ECX.XSAVE[bit 26] = 0 or
CPUID.(EAX=0DH,ECX=1):EAX.XSS[bit 3] = 0.


> +               if (!xsaves_enabled)
> +                       exec_control &= ~SECONDARY_EXEC_XSAVES;
> +
> +               if (nested) {
> +                       if (xsaves_enabled)
> +                               vmx->nested.nested_vmx_secondary_ctls_high |=
> +                                       SECONDARY_EXEC_XSAVES;
> +                       else
> +                               vmx->nested.nested_vmx_secondary_ctls_high &=
> +                                       ~SECONDARY_EXEC_XSAVES;
> +               }
> +       }
> +
>         if (vmx_rdtscp_supported()) {
>                 bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
>                 if (!rdtscp_enabled)
> @@ -10421,6 +10434,7 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>                 exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
>                                   SECONDARY_EXEC_ENABLE_INVPCID |
>                                   SECONDARY_EXEC_RDTSCP |
> +                                 SECONDARY_EXEC_XSAVES |
>                                   SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>                                   SECONDARY_EXEC_APIC_REGISTER_VIRT |
>                                   SECONDARY_EXEC_ENABLE_VMFUNC);
> --
> 1.8.3.1
>