Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1719239 > unrolled thread

[PATCH v7 0/4] Raise #UD and disable execution controls for more CPUID bits

Started byPaolo Bonzini <pbonzini@redhat.com>
First post2017-08-24 15:40 +0200
Last post2017-08-24 22:50 +0200
Articles 16 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v7 0/4] Raise #UD and disable execution controls for more CPUID bits Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 15:40 +0200
    [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 15:40 +0200
      Re: [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED David Hildenbrand <david@redhat.com> - 2017-08-24 19:50 +0200
    [PATCH 1/4] KVM: VMX: cache secondary exec controls Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 15:40 +0200
      Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls Jim Mattson <jmattson@google.com> - 2017-08-24 16:50 +0200
        Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 17:30 +0200
          Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls Jim Mattson <jmattson@google.com> - 2017-08-24 17:50 +0200
            Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 17:50 +0200
              Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 18:10 +0200
              Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls Jim Mattson <jmattson@google.com> - 2017-08-24 18:10 +0200
    [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 15:40 +0200
      Re: [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND David Hildenbrand <david@redhat.com> - 2017-08-24 19:50 +0200
    [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 15:40 +0200
      Re: [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS Jim Mattson <jmattson@google.com> - 2017-08-24 17:00 +0200
        [PATCH v8 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS Paolo Bonzini <pbonzini@redhat.com> - 2017-08-24 18:20 +0200
          Re: [PATCH v8 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS Jim Mattson <jmattson@google.com> - 2017-08-24 22:50 +0200

#1719239 — [PATCH v7 0/4] Raise #UD and disable execution controls for more CPUID bits

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 15:40 +0200
Subject[PATCH v7 0/4] Raise #UD and disable execution controls for more CPUID bits
Message-ID<ui0wy-7c2-3@gated-at.bofh.it>
Usually we're not that particular about #UD-ing on disabled instructions,
because you cannot do it for all instructions.  However, whenever an
instruction has a corresponding execution control it is important to do
it for the sake of getting vmexits right.

Paolo

Jim Mattson (2):
  kvm: vmx: Raise #UD on unsupported RDRAND
  kvm: vmx: Raise #UD on unsupported RDSEED

Paolo Bonzini (2):
  KVM: VMX: cache secondary exec controls
  kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS

 arch/x86/kvm/vmx.c | 175 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 124 insertions(+), 51 deletions(-)

-- 
1.8.3.1

[toc] | [next] | [standalone]

#1719241 — [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 15:40 +0200
Subject[PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED
Message-ID<ui0wy-7c2-19@gated-at.bofh.it>
In reply to#1719239
From: Jim Mattson <jmattson@google.com>

A guest may not be configured to support RDSEED, even when the host
does. If the guest does not support RDSEED, intercept the instruction
and synthesize #UD. Also clear the "allowed-1" bit for RDSEED exiting
in the IA32_VMX_PROCBASED_CTLS2 MSR.

Signed-off-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1f2c69de7872..954e26079cd6 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2818,7 +2818,6 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 		vmx->nested.nested_vmx_secondary_ctls_high);
 	vmx->nested.nested_vmx_secondary_ctls_low = 0;
 	vmx->nested.nested_vmx_secondary_ctls_high &=
-		SECONDARY_EXEC_RDSEED |
 		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
 		SECONDARY_EXEC_DESC |
 		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
@@ -3671,6 +3670,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 			SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
 			SECONDARY_EXEC_SHADOW_VMCS |
 			SECONDARY_EXEC_XSAVES |
+			SECONDARY_EXEC_RDSEED |
 			SECONDARY_EXEC_RDRAND |
 			SECONDARY_EXEC_ENABLE_PML |
 			SECONDARY_EXEC_TSC_SCALING |
@@ -5280,6 +5280,12 @@ static bool vmx_rdrand_supported(void)
 		SECONDARY_EXEC_RDRAND;
 }
 
+static bool vmx_rdseed_supported(void)
+{
+	return vmcs_config.cpu_based_2nd_exec_ctrl &
+		SECONDARY_EXEC_RDSEED;
+}
+
 static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 {
 	struct kvm_vcpu *vcpu = &vmx->vcpu;
@@ -5364,6 +5370,21 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 		}
 	}
 
+	if (vmx_rdseed_supported()) {
+		bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
+		if (rdseed_enabled)
+			exec_control &= ~SECONDARY_EXEC_RDSEED;
+
+		if (nested) {
+			if (rdseed_enabled)
+				vmx->nested.nested_vmx_secondary_ctls_high |=
+					SECONDARY_EXEC_RDSEED;
+			else
+				vmx->nested.nested_vmx_secondary_ctls_high &=
+					~SECONDARY_EXEC_RDSEED;
+		}
+	}
+
 	vmx->secondary_exec_control = exec_control;
 }
 
@@ -8119,6 +8140,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 	[EXIT_REASON_INVEPT]                  = handle_invept,
 	[EXIT_REASON_INVVPID]                 = handle_invvpid,
 	[EXIT_REASON_RDRAND]                  = handle_invalid_op,
+	[EXIT_REASON_RDSEED]                  = handle_invalid_op,
 	[EXIT_REASON_XSAVES]                  = handle_xsaves,
 	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
 	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1719476 — Re: [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED

FromDavid Hildenbrand <david@redhat.com>
Date2017-08-24 19:50 +0200
SubjectRe: [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED
Message-ID<ui4qu-1bx-17@gated-at.bofh.it>
In reply to#1719241
On 24.08.2017 15:37, Paolo Bonzini wrote:
> From: Jim Mattson <jmattson@google.com>
> 
> A guest may not be configured to support RDSEED, even when the host
> does. If the guest does not support RDSEED, intercept the instruction
> and synthesize #UD. Also clear the "allowed-1" bit for RDSEED exiting
> in the IA32_VMX_PROCBASED_CTLS2 MSR.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 1f2c69de7872..954e26079cd6 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2818,7 +2818,6 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  		vmx->nested.nested_vmx_secondary_ctls_high);
>  	vmx->nested.nested_vmx_secondary_ctls_low = 0;
>  	vmx->nested.nested_vmx_secondary_ctls_high &=
> -		SECONDARY_EXEC_RDSEED |
>  		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
>  		SECONDARY_EXEC_DESC |
>  		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
> @@ -3671,6 +3670,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  			SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>  			SECONDARY_EXEC_SHADOW_VMCS |
>  			SECONDARY_EXEC_XSAVES |
> +			SECONDARY_EXEC_RDSEED |
>  			SECONDARY_EXEC_RDRAND |
>  			SECONDARY_EXEC_ENABLE_PML |
>  			SECONDARY_EXEC_TSC_SCALING |
> @@ -5280,6 +5280,12 @@ static bool vmx_rdrand_supported(void)
>  		SECONDARY_EXEC_RDRAND;
>  }
>  
> +static bool vmx_rdseed_supported(void)
> +{
> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
> +		SECONDARY_EXEC_RDSEED;
> +}
> +
>  static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  {
>  	struct kvm_vcpu *vcpu = &vmx->vcpu;
> @@ -5364,6 +5370,21 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  		}
>  	}
>  
> +	if (vmx_rdseed_supported()) {
> +		bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
> +		if (rdseed_enabled)
> +			exec_control &= ~SECONDARY_EXEC_RDSEED;
> +
> +		if (nested) {
> +			if (rdseed_enabled)
> +				vmx->nested.nested_vmx_secondary_ctls_high |=
> +					SECONDARY_EXEC_RDSEED;
> +			else
> +				vmx->nested.nested_vmx_secondary_ctls_high &=
> +					~SECONDARY_EXEC_RDSEED;
> +		}
> +	}
> +
>  	vmx->secondary_exec_control = exec_control;
>  }
>  
> @@ -8119,6 +8140,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>  	[EXIT_REASON_INVEPT]                  = handle_invept,
>  	[EXIT_REASON_INVVPID]                 = handle_invvpid,
>  	[EXIT_REASON_RDRAND]                  = handle_invalid_op,
> +	[EXIT_REASON_RDSEED]                  = handle_invalid_op,
>  	[EXIT_REASON_XSAVES]                  = handle_xsaves,
>  	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
>  	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

[toc] | [prev] | [next] | [standalone]

#1719243 — [PATCH 1/4] KVM: VMX: cache secondary exec controls

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 15:40 +0200
Subject[PATCH 1/4] KVM: VMX: cache secondary exec controls
Message-ID<ui0wz-7c2-29@gated-at.bofh.it>
In reply to#1719239
Currently, secondary execution controls are divided in three groups:

- static, depending mostly on the module arguments or the processor
  (vmx_secondary_exec_control)

- static, depending on CPUID (vmx_cpuid_update)

- dynamic, depending on nested VMX or local APIC state

Because walking CPUID is expensive, prepare_vmcs02 is using only
the first group.  This however is unnecessarily complicated.  Just
cache the static secondary execution controls, and then prepare_vmcs02
does not need to compute them every time.  Computation of all static
secondary execution controls is now kept in a single function,
vmx_compute_secondary_exec_control.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx.c | 100 +++++++++++++++++++++++++++++------------------------
 1 file changed, 54 insertions(+), 46 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 19aa69af7c2d..08381a5d8879 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -576,6 +576,8 @@ struct vcpu_vmx {
 #endif
 	u32 vm_entry_controls_shadow;
 	u32 vm_exit_controls_shadow;
+	u32 secondary_exec_control;
+
 	/*
 	 * loaded_vmcs points to the VMCS currently used in this vcpu. For a
 	 * non-nested (L1) guest, it always points to vmcs01. For a nested
@@ -2807,7 +2809,10 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 	vmx->nested.nested_vmx_procbased_ctls_low &=
 		~(CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING);
 
-	/* secondary cpu-based controls */
+	/*
+	 * secondary cpu-based controls.  Do not include those that
+	 * depend on CPUID bits, they are added later by vmx_cpuid_update.
+	 */
 	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
 		vmx->nested.nested_vmx_secondary_ctls_low,
 		vmx->nested.nested_vmx_secondary_ctls_high);
@@ -2815,7 +2820,6 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 	vmx->nested.nested_vmx_secondary_ctls_high &=
 		SECONDARY_EXEC_RDRAND | SECONDARY_EXEC_RDSEED |
 		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
-		SECONDARY_EXEC_RDTSCP |
 		SECONDARY_EXEC_DESC |
 		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
 		SECONDARY_EXEC_APIC_REGISTER_VIRT |
@@ -5269,10 +5273,12 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
 	return exec_control;
 }
 
-static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
+static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 {
+	struct kvm_vcpu *vcpu = &vmx->vcpu;
+
 	u32 exec_control = vmcs_config.cpu_based_2nd_exec_ctrl;
-	if (!cpu_need_virtualize_apic_accesses(&vmx->vcpu))
+	if (!cpu_need_virtualize_apic_accesses(vcpu))
 		exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
 	if (vmx->vpid == 0)
 		exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;
@@ -5286,7 +5292,7 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
 		exec_control &= ~SECONDARY_EXEC_UNRESTRICTED_GUEST;
 	if (!ple_gap)
 		exec_control &= ~SECONDARY_EXEC_PAUSE_LOOP_EXITING;
-	if (!kvm_vcpu_apicv_active(&vmx->vcpu))
+	if (!kvm_vcpu_apicv_active(vcpu))
 		exec_control &= ~(SECONDARY_EXEC_APIC_REGISTER_VIRT |
 				  SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
 	exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
@@ -5300,7 +5306,43 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
 	if (!enable_pml)
 		exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
 
-	return exec_control;
+	if (vmx_rdtscp_supported()) {
+		bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
+		if (!rdtscp_enabled)
+			exec_control &= ~SECONDARY_EXEC_RDTSCP;
+
+		if (nested) {
+			if (rdtscp_enabled)
+				vmx->nested.nested_vmx_secondary_ctls_high |=
+					SECONDARY_EXEC_RDTSCP;
+			else
+				vmx->nested.nested_vmx_secondary_ctls_high &=
+					~SECONDARY_EXEC_RDTSCP;
+		}
+	}
+
+	if (vmx_invpcid_supported()) {
+		/* Exposing INVPCID only when PCID is exposed */
+		bool invpcid_enabled =
+			guest_cpuid_has(vcpu, X86_FEATURE_INVPCID) &&
+			guest_cpuid_has(vcpu, X86_FEATURE_PCID);
+
+		if (!invpcid_enabled) {
+			exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
+			guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
+		}
+
+		if (nested) {
+			if (invpcid_enabled)
+				vmx->nested.nested_vmx_secondary_ctls_high |=
+					SECONDARY_EXEC_ENABLE_INVPCID;
+			else
+				vmx->nested.nested_vmx_secondary_ctls_high &=
+					~SECONDARY_EXEC_ENABLE_INVPCID;
+		}
+	}
+
+	vmx->secondary_exec_control = exec_control;
 }
 
 static void ept_set_mmio_spte_mask(void)
@@ -5344,8 +5386,9 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, vmx_exec_control(vmx));
 
 	if (cpu_has_secondary_exec_ctrls()) {
+		vmx_compute_secondary_exec_control(vmx);
 		vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
-				vmx_secondary_exec_control(vmx));
+			     vmx->secondary_exec_control);
 	}
 
 	if (kvm_vcpu_apicv_active(&vmx->vcpu)) {
@@ -9623,47 +9666,12 @@ static void nested_vmx_cr_fixed1_bits_update(struct kvm_vcpu *vcpu)
 static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	u32 secondary_exec_ctl = vmx_secondary_exec_control(vmx);
-
-	if (vmx_rdtscp_supported()) {
-		bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
-		if (!rdtscp_enabled)
-			secondary_exec_ctl &= ~SECONDARY_EXEC_RDTSCP;
-
-		if (nested) {
-			if (rdtscp_enabled)
-				vmx->nested.nested_vmx_secondary_ctls_high |=
-					SECONDARY_EXEC_RDTSCP;
-			else
-				vmx->nested.nested_vmx_secondary_ctls_high &=
-					~SECONDARY_EXEC_RDTSCP;
-		}
-	}
-
-	if (vmx_invpcid_supported()) {
-		/* Exposing INVPCID only when PCID is exposed */
-		bool invpcid_enabled =
-			guest_cpuid_has(vcpu, X86_FEATURE_INVPCID) &&
-			guest_cpuid_has(vcpu, X86_FEATURE_PCID);
-
-		if (!invpcid_enabled) {
-			secondary_exec_ctl &= ~SECONDARY_EXEC_ENABLE_INVPCID;
-			guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
-		}
 
-		if (nested) {
-			if (invpcid_enabled)
-				vmx->nested.nested_vmx_secondary_ctls_high |=
-					SECONDARY_EXEC_ENABLE_INVPCID;
-			else
-				vmx->nested.nested_vmx_secondary_ctls_high &=
-					~SECONDARY_EXEC_ENABLE_INVPCID;
-		}
+	if (cpu_has_secondary_exec_ctrls()) {
+		vmx_compute_secondary_exec_control(vmx);
+		vmcs_set_secondary_exec_control(vmx->secondary_exec_control);
 	}
 
-	if (cpu_has_secondary_exec_ctrls())
-		vmcs_set_secondary_exec_control(secondary_exec_ctl);
-
 	if (nested_vmx_allowed(vcpu))
 		to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |=
 			FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
@@ -10356,7 +10364,7 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 		enable_ept ? vmcs12->page_fault_error_code_match : 0);
 
 	if (cpu_has_secondary_exec_ctrls()) {
-		exec_control = vmx_secondary_exec_control(vmx);
+		exec_control = vmx->secondary_exec_control;
 
 		/* Take the following fields only from vmcs12 */
 		exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1719321 — Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls

FromJim Mattson <jmattson@google.com>
Date2017-08-24 16:50 +0200
SubjectRe: [PATCH 1/4] KVM: VMX: cache secondary exec controls
Message-ID<ui1Ch-7QZ-5@gated-at.bofh.it>
In reply to#1719243
On Thu, Aug 24, 2017 at 6:37 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Currently, secondary execution controls are divided in three groups:
>
> - static, depending mostly on the module arguments or the processor
>   (vmx_secondary_exec_control)
>
> - static, depending on CPUID (vmx_cpuid_update)

There should also be:

- static, depending on guest VMX capability MSRs (vmx_set_vmx_msr)

>
> - dynamic, depending on nested VMX or local APIC state
>
> Because walking CPUID is expensive, prepare_vmcs02 is using only
> the first group.  This however is unnecessarily complicated.  Just
> cache the static secondary execution controls, and then prepare_vmcs02
> does not need to compute them every time.  Computation of all static
> secondary execution controls is now kept in a single function,
> vmx_compute_secondary_exec_control.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 100 +++++++++++++++++++++++++++++------------------------
>  1 file changed, 54 insertions(+), 46 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 19aa69af7c2d..08381a5d8879 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -576,6 +576,8 @@ struct vcpu_vmx {
>  #endif
>         u32 vm_entry_controls_shadow;
>         u32 vm_exit_controls_shadow;
> +       u32 secondary_exec_control;
> +
>         /*
>          * loaded_vmcs points to the VMCS currently used in this vcpu. For a
>          * non-nested (L1) guest, it always points to vmcs01. For a nested
> @@ -2807,7 +2809,10 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>         vmx->nested.nested_vmx_procbased_ctls_low &=
>                 ~(CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING);
>
> -       /* secondary cpu-based controls */
> +       /*
> +        * secondary cpu-based controls.  Do not include those that
> +        * depend on CPUID bits, they are added later by vmx_cpuid_update.

I think vmx_cpuid_update should only clear VMX capability bits, and
not set them. See below.

> +        */
>         rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
>                 vmx->nested.nested_vmx_secondary_ctls_low,
>                 vmx->nested.nested_vmx_secondary_ctls_high);
> @@ -2815,7 +2820,6 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>         vmx->nested.nested_vmx_secondary_ctls_high &=
>                 SECONDARY_EXEC_RDRAND | SECONDARY_EXEC_RDSEED |
>                 SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
> -               SECONDARY_EXEC_RDTSCP |
>                 SECONDARY_EXEC_DESC |
>                 SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
>                 SECONDARY_EXEC_APIC_REGISTER_VIRT |
> @@ -5269,10 +5273,12 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
>         return exec_control;
>  }
>
> -static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
> +static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  {
> +       struct kvm_vcpu *vcpu = &vmx->vcpu;
> +
>         u32 exec_control = vmcs_config.cpu_based_2nd_exec_ctrl;
> -       if (!cpu_need_virtualize_apic_accesses(&vmx->vcpu))
> +       if (!cpu_need_virtualize_apic_accesses(vcpu))
>                 exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
>         if (vmx->vpid == 0)
>                 exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;
> @@ -5286,7 +5292,7 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
>                 exec_control &= ~SECONDARY_EXEC_UNRESTRICTED_GUEST;
>         if (!ple_gap)
>                 exec_control &= ~SECONDARY_EXEC_PAUSE_LOOP_EXITING;
> -       if (!kvm_vcpu_apicv_active(&vmx->vcpu))
> +       if (!kvm_vcpu_apicv_active(vcpu))
>                 exec_control &= ~(SECONDARY_EXEC_APIC_REGISTER_VIRT |
>                                   SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
>         exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
> @@ -5300,7 +5306,43 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
>         if (!enable_pml)
>                 exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>
> -       return exec_control;
> +       if (vmx_rdtscp_supported()) {
> +               bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
> +               if (!rdtscp_enabled)
> +                       exec_control &= ~SECONDARY_EXEC_RDTSCP;
> +
> +               if (nested) {
> +                       if (rdtscp_enabled)
> +                               vmx->nested.nested_vmx_secondary_ctls_high |=
> +                                       SECONDARY_EXEC_RDTSCP;

CPUID.80000001H:EDX.RDTSCP[bit 27] does not imply
IA32_VMX_PROCBASED_CTLS2.[bit 35].

> +                       else
> +                               vmx->nested.nested_vmx_secondary_ctls_high &=
> +                                       ~SECONDARY_EXEC_RDTSCP;
> +               }
> +       }
> +
> +       if (vmx_invpcid_supported()) {
> +               /* Exposing INVPCID only when PCID is exposed */
> +               bool invpcid_enabled =
> +                       guest_cpuid_has(vcpu, X86_FEATURE_INVPCID) &&
> +                       guest_cpuid_has(vcpu, X86_FEATURE_PCID);
> +
> +               if (!invpcid_enabled) {
> +                       exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
> +                       guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
> +               }
> +
> +               if (nested) {
> +                       if (invpcid_enabled)
> +                               vmx->nested.nested_vmx_secondary_ctls_high |=
> +                                       SECONDARY_EXEC_ENABLE_INVPCID;

CPUID.(EAX=07H,ECX=0):EBX.INVPCID[bit 10] does not imply
IA32_VMX_PROCBASED_CTLS2.[bit 44].

> +                       else
> +                               vmx->nested.nested_vmx_secondary_ctls_high &=
> +                                       ~SECONDARY_EXEC_ENABLE_INVPCID;
> +               }
> +       }
> +
> +       vmx->secondary_exec_control = exec_control;
>  }
>
>  static void ept_set_mmio_spte_mask(void)
> @@ -5344,8 +5386,9 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
>         vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, vmx_exec_control(vmx));
>
>         if (cpu_has_secondary_exec_ctrls()) {
> +               vmx_compute_secondary_exec_control(vmx);
>                 vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
> -                               vmx_secondary_exec_control(vmx));
> +                            vmx->secondary_exec_control);
>         }
>
>         if (kvm_vcpu_apicv_active(&vmx->vcpu)) {
> @@ -9623,47 +9666,12 @@ static void nested_vmx_cr_fixed1_bits_update(struct kvm_vcpu *vcpu)
>  static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  {
>         struct vcpu_vmx *vmx = to_vmx(vcpu);
> -       u32 secondary_exec_ctl = vmx_secondary_exec_control(vmx);
> -
> -       if (vmx_rdtscp_supported()) {
> -               bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
> -               if (!rdtscp_enabled)
> -                       secondary_exec_ctl &= ~SECONDARY_EXEC_RDTSCP;
> -
> -               if (nested) {
> -                       if (rdtscp_enabled)
> -                               vmx->nested.nested_vmx_secondary_ctls_high |=
> -                                       SECONDARY_EXEC_RDTSCP;
> -                       else
> -                               vmx->nested.nested_vmx_secondary_ctls_high &=
> -                                       ~SECONDARY_EXEC_RDTSCP;
> -               }
> -       }
> -
> -       if (vmx_invpcid_supported()) {
> -               /* Exposing INVPCID only when PCID is exposed */
> -               bool invpcid_enabled =
> -                       guest_cpuid_has(vcpu, X86_FEATURE_INVPCID) &&
> -                       guest_cpuid_has(vcpu, X86_FEATURE_PCID);
> -
> -               if (!invpcid_enabled) {
> -                       secondary_exec_ctl &= ~SECONDARY_EXEC_ENABLE_INVPCID;
> -                       guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
> -               }
>
> -               if (nested) {
> -                       if (invpcid_enabled)
> -                               vmx->nested.nested_vmx_secondary_ctls_high |=
> -                                       SECONDARY_EXEC_ENABLE_INVPCID;
> -                       else
> -                               vmx->nested.nested_vmx_secondary_ctls_high &=
> -                                       ~SECONDARY_EXEC_ENABLE_INVPCID;
> -               }
> +       if (cpu_has_secondary_exec_ctrls()) {
> +               vmx_compute_secondary_exec_control(vmx);
> +               vmcs_set_secondary_exec_control(vmx->secondary_exec_control);
>         }
>
> -       if (cpu_has_secondary_exec_ctrls())
> -               vmcs_set_secondary_exec_control(secondary_exec_ctl);
> -
>         if (nested_vmx_allowed(vcpu))
>                 to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |=
>                         FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
> @@ -10356,7 +10364,7 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
>                 enable_ept ? vmcs12->page_fault_error_code_match : 0);
>
>         if (cpu_has_secondary_exec_ctrls()) {
> -               exec_control = vmx_secondary_exec_control(vmx);
> +               exec_control = vmx->secondary_exec_control;
>
>                 /* Take the following fields only from vmcs12 */
>                 exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
> --
> 1.8.3.1
>
>

[toc] | [prev] | [next] | [standalone]

#1719335 — Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 17:30 +0200
SubjectRe: [PATCH 1/4] KVM: VMX: cache secondary exec controls
Message-ID<ui2f0-8jI-5@gated-at.bofh.it>
In reply to#1719321
On 24/08/2017 16:47, Jim Mattson wrote:
>> Currently, secondary execution controls are divided in three groups:
>>
>> - static, depending mostly on the module arguments or the processor
>>   (vmx_secondary_exec_control)
>>
>> - static, depending on CPUID (vmx_cpuid_update)
> There should also be:
> 
> - static, depending on guest VMX capability MSRs (vmx_set_vmx_msr)

Can you explain what you mean?

Paolo

[toc] | [prev] | [next] | [standalone]

#1719346 — Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls

FromJim Mattson <jmattson@google.com>
Date2017-08-24 17:50 +0200
SubjectRe: [PATCH 1/4] KVM: VMX: cache secondary exec controls
Message-ID<ui2yl-8q8-9@gated-at.bofh.it>
In reply to#1719335
Userspace can establish the value of the virtualized
IA32_VMX_PROCBASED_CTLS2 MSR via the KVM_SET_MSRS ioctl, which goes
through vms_set_vmx_msr. But maybe that's not important, since
features can only be disabled on that path.

On Thu, Aug 24, 2017 at 8:25 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 24/08/2017 16:47, Jim Mattson wrote:
>>> Currently, secondary execution controls are divided in three groups:
>>>
>>> - static, depending mostly on the module arguments or the processor
>>>   (vmx_secondary_exec_control)
>>>
>>> - static, depending on CPUID (vmx_cpuid_update)
>> There should also be:
>>
>> - static, depending on guest VMX capability MSRs (vmx_set_vmx_msr)
>
> Can you explain what you mean?
>
> Paolo

[toc] | [prev] | [next] | [standalone]

#1719349 — Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 17:50 +0200
SubjectRe: [PATCH 1/4] KVM: VMX: cache secondary exec controls
Message-ID<ui2ym-8q8-25@gated-at.bofh.it>
In reply to#1719346
On 24/08/2017 17:41, Jim Mattson wrote:
> Userspace can establish the value of the virtualized
> IA32_VMX_PROCBASED_CTLS2 MSR via the KVM_SET_MSRS ioctl, which goes
> through vms_set_vmx_msr. But maybe that's not important, since
> features can only be disabled on that path.

Yeah, I was only thinking of non-nested in the commit message.  It's
complicated enough. :)

Paolo

> On Thu, Aug 24, 2017 at 8:25 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 24/08/2017 16:47, Jim Mattson wrote:
>>>> Currently, secondary execution controls are divided in three groups:
>>>>
>>>> - static, depending mostly on the module arguments or the processor
>>>>   (vmx_secondary_exec_control)
>>>>
>>>> - static, depending on CPUID (vmx_cpuid_update)
>>> There should also be:
>>>
>>> - static, depending on guest VMX capability MSRs (vmx_set_vmx_msr)
>> Can you explain what you mean?
>>
>> Paolo

[toc] | [prev] | [next] | [standalone]

#1719368 — Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 18:10 +0200
SubjectRe: [PATCH 1/4] KVM: VMX: cache secondary exec controls
Message-ID<ui2RI-kG-25@gated-at.bofh.it>
In reply to#1719349
On 24/08/2017 18:02, Jim Mattson wrote:
> On the subject of complexity, why do we clear
> CPUID.(EAX=07H,ECX=0):EBX.INVPCID[bit 10] when CPUID.01H:ECX.PCID[bit
> 17] is clear? Sure, it would be odd to support the INVPCID instruction
> without also supporting PCIDs, but why single out this one check?
> Isn't it equally bizarre to support SSE2 without SSE, or XSAVES
> without XSAVE, or RDTSCP without TSC, or DS-CPL without DS, or ...?

I actually agree with you.  It's just been like this forever:

    commit ad756a1603c5fac207758faaac7f01c34c9d0b7b
    Author: Mao, Junjie <junjie.mao@intel.com>
    Date:   Mon Jul 2 01:18:48 2012 +0000

    KVM: VMX: Implement PCID/INVPCID for guests with EPT

Paolo

[toc] | [prev] | [next] | [standalone]

#1719370 — Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls

FromJim Mattson <jmattson@google.com>
Date2017-08-24 18:10 +0200
SubjectRe: [PATCH 1/4] KVM: VMX: cache secondary exec controls
Message-ID<ui2RI-kG-27@gated-at.bofh.it>
In reply to#1719349
On the subject of complexity, why do we clear
CPUID.(EAX=07H,ECX=0):EBX.INVPCID[bit 10] when CPUID.01H:ECX.PCID[bit
17] is clear? Sure, it would be odd to support the INVPCID instruction
without also supporting PCIDs, but why single out this one check?
Isn't it equally bizarre to support SSE2 without SSE, or XSAVES
without XSAVE, or RDTSCP without TSC, or DS-CPL without DS, or ...?

On Thu, Aug 24, 2017 at 8:46 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 24/08/2017 17:41, Jim Mattson wrote:
>> Userspace can establish the value of the virtualized
>> IA32_VMX_PROCBASED_CTLS2 MSR via the KVM_SET_MSRS ioctl, which goes
>> through vms_set_vmx_msr. But maybe that's not important, since
>> features can only be disabled on that path.
>
> Yeah, I was only thinking of non-nested in the commit message.  It's
> complicated enough. :)
>
> Paolo
>
>> On Thu, Aug 24, 2017 at 8:25 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> On 24/08/2017 16:47, Jim Mattson wrote:
>>>>> Currently, secondary execution controls are divided in three groups:
>>>>>
>>>>> - static, depending mostly on the module arguments or the processor
>>>>>   (vmx_secondary_exec_control)
>>>>>
>>>>> - static, depending on CPUID (vmx_cpuid_update)
>>>> There should also be:
>>>>
>>>> - static, depending on guest VMX capability MSRs (vmx_set_vmx_msr)
>>> Can you explain what you mean?
>>>
>>> Paolo
>

[toc] | [prev] | [next] | [standalone]

#1719244 — [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 15:40 +0200
Subject[PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND
Message-ID<ui0wz-7c2-35@gated-at.bofh.it>
In reply to#1719239
From: Jim Mattson <jmattson@google.com>

A guest may not be configured to support RDRAND, even when the host
does. If the guest does not support RDRAND, intercept the instruction
and synthesize #UD. Also clear the "allowed-1" bit for RDRAND exiting
in the IA32_VMX_PROCBASED_CTLS2 MSR.

Signed-off-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 08381a5d8879..1f2c69de7872 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2818,7 +2818,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 		vmx->nested.nested_vmx_secondary_ctls_high);
 	vmx->nested.nested_vmx_secondary_ctls_low = 0;
 	vmx->nested.nested_vmx_secondary_ctls_high &=
-		SECONDARY_EXEC_RDRAND | SECONDARY_EXEC_RDSEED |
+		SECONDARY_EXEC_RDSEED |
 		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
 		SECONDARY_EXEC_DESC |
 		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
@@ -3671,6 +3671,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 			SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
 			SECONDARY_EXEC_SHADOW_VMCS |
 			SECONDARY_EXEC_XSAVES |
+			SECONDARY_EXEC_RDRAND |
 			SECONDARY_EXEC_ENABLE_PML |
 			SECONDARY_EXEC_TSC_SCALING |
 			SECONDARY_EXEC_ENABLE_VMFUNC;
@@ -5273,6 +5274,12 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
 	return exec_control;
 }
 
+static bool vmx_rdrand_supported(void)
+{
+	return vmcs_config.cpu_based_2nd_exec_ctrl &
+		SECONDARY_EXEC_RDRAND;
+}
+
 static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 {
 	struct kvm_vcpu *vcpu = &vmx->vcpu;
@@ -5342,6 +5349,21 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 		}
 	}
 
+	if (vmx_rdrand_supported()) {
+		bool rdrand_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDRAND);
+		if (rdrand_enabled)
+			exec_control &= ~SECONDARY_EXEC_RDRAND;
+
+		if (nested) {
+			if (rdrand_enabled)
+				vmx->nested.nested_vmx_secondary_ctls_high |=
+					SECONDARY_EXEC_RDRAND;
+			else
+				vmx->nested.nested_vmx_secondary_ctls_high &=
+					~SECONDARY_EXEC_RDRAND;
+		}
+	}
+
 	vmx->secondary_exec_control = exec_control;
 }
 
@@ -6847,6 +6869,12 @@ static int handle_mwait(struct kvm_vcpu *vcpu)
 	return handle_nop(vcpu);
 }
 
+static int handle_invalid_op(struct kvm_vcpu *vcpu)
+{
+	kvm_queue_exception(vcpu, UD_VECTOR);
+	return 1;
+}
+
 static int handle_monitor_trap(struct kvm_vcpu *vcpu)
 {
 	return 1;
@@ -8090,6 +8118,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 	[EXIT_REASON_MONITOR_INSTRUCTION]     = handle_monitor,
 	[EXIT_REASON_INVEPT]                  = handle_invept,
 	[EXIT_REASON_INVVPID]                 = handle_invvpid,
+	[EXIT_REASON_RDRAND]                  = handle_invalid_op,
 	[EXIT_REASON_XSAVES]                  = handle_xsaves,
 	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
 	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1719470 — Re: [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND

FromDavid Hildenbrand <david@redhat.com>
Date2017-08-24 19:50 +0200
SubjectRe: [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND
Message-ID<ui4qt-1bx-1@gated-at.bofh.it>
In reply to#1719244
On 24.08.2017 15:37, Paolo Bonzini wrote:
> From: Jim Mattson <jmattson@google.com>
> 
> A guest may not be configured to support RDRAND, even when the host
> does. If the guest does not support RDRAND, intercept the instruction
> and synthesize #UD. Also clear the "allowed-1" bit for RDRAND exiting
> in the IA32_VMX_PROCBASED_CTLS2 MSR.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 08381a5d8879..1f2c69de7872 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2818,7 +2818,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  		vmx->nested.nested_vmx_secondary_ctls_high);
>  	vmx->nested.nested_vmx_secondary_ctls_low = 0;
>  	vmx->nested.nested_vmx_secondary_ctls_high &=
> -		SECONDARY_EXEC_RDRAND | SECONDARY_EXEC_RDSEED |
> +		SECONDARY_EXEC_RDSEED |
>  		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
>  		SECONDARY_EXEC_DESC |
>  		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
> @@ -3671,6 +3671,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  			SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>  			SECONDARY_EXEC_SHADOW_VMCS |
>  			SECONDARY_EXEC_XSAVES |
> +			SECONDARY_EXEC_RDRAND |
>  			SECONDARY_EXEC_ENABLE_PML |
>  			SECONDARY_EXEC_TSC_SCALING |
>  			SECONDARY_EXEC_ENABLE_VMFUNC;
> @@ -5273,6 +5274,12 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
>  	return exec_control;
>  }
>  
> +static bool vmx_rdrand_supported(void)
> +{
> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
> +		SECONDARY_EXEC_RDRAND;
> +}
> +
>  static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  {
>  	struct kvm_vcpu *vcpu = &vmx->vcpu;
> @@ -5342,6 +5349,21 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  		}
>  	}
>  
> +	if (vmx_rdrand_supported()) {
> +		bool rdrand_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDRAND);
> +		if (rdrand_enabled)
> +			exec_control &= ~SECONDARY_EXEC_RDRAND;
> +
> +		if (nested) {
> +			if (rdrand_enabled)
> +				vmx->nested.nested_vmx_secondary_ctls_high |=
> +					SECONDARY_EXEC_RDRAND;
> +			else
> +				vmx->nested.nested_vmx_secondary_ctls_high &=
> +					~SECONDARY_EXEC_RDRAND;
> +		}
> +	}
> +
>  	vmx->secondary_exec_control = exec_control;
>  }
>  
> @@ -6847,6 +6869,12 @@ static int handle_mwait(struct kvm_vcpu *vcpu)
>  	return handle_nop(vcpu);
>  }
>  
> +static int handle_invalid_op(struct kvm_vcpu *vcpu)
> +{
> +	kvm_queue_exception(vcpu, UD_VECTOR);
> +	return 1;
> +}
> +
>  static int handle_monitor_trap(struct kvm_vcpu *vcpu)
>  {
>  	return 1;
> @@ -8090,6 +8118,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>  	[EXIT_REASON_MONITOR_INSTRUCTION]     = handle_monitor,
>  	[EXIT_REASON_INVEPT]                  = handle_invept,
>  	[EXIT_REASON_INVVPID]                 = handle_invvpid,
> +	[EXIT_REASON_RDRAND]                  = handle_invalid_op,
>  	[EXIT_REASON_XSAVES]                  = handle_xsaves,
>  	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
>  	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

[toc] | [prev] | [next] | [standalone]

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

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 15:40 +0200
Subject[PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS
Message-ID<ui0wA-7c2-45@gated-at.bofh.it>
In reply to#1719239
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@redhat.com>
---
 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);
+		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

[toc] | [prev] | [next] | [standalone]

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

FromJim Mattson <jmattson@google.com>
Date2017-08-24 17:00 +0200
SubjectRe: [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS
Message-ID<ui1LY-7UE-17@gated-at.bofh.it>
In reply to#1719245
On Thu, Aug 24, 2017 at 6:37 AM, Paolo Bonzini <pbonzini@redhat.com> 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@redhat.com>
> ---
>  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
>

[toc] | [prev] | [next] | [standalone]

#1719382 — [PATCH v8 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-08-24 18:20 +0200
Subject[PATCH v8 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS
Message-ID<ui31o-pa-21@gated-at.bofh.it>
In reply to#1719329
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 XSAVES/XRSTORS exiting in the
IA32_VMX_PROCBASED_CTLS2 MSR, and pass through VMCS12's control in
the VMCS02.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 954e26079cd6..08bfeb8c32ea 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,25 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 	if (!enable_pml)
 		exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
 
+	if (vmx_xsaves_supported()) {
+		/* Exposing XSAVES only when XSAVE is exposed */
+		bool xsaves_enabled =
+			guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
+			guest_cpuid_has(vcpu, X86_FEATURE_XSAVES);
+
+		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 +10438,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

[toc] | [prev] | [next] | [standalone]

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

FromJim Mattson <jmattson@google.com>
Date2017-08-24 22:50 +0200
SubjectRe: [PATCH v8 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS
Message-ID<ui7eG-2Ut-27@gated-at.bofh.it>
In reply to#1719382
Reviewed-by: Jim Mattson <jmattson@google.com>

On Thu, Aug 24, 2017 at 9:09 AM, Paolo Bonzini <pbonzini@redhat.com> 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 XSAVES/XRSTORS exiting in the
> IA32_VMX_PROCBASED_CTLS2 MSR, and pass through VMCS12's control in
> the VMCS02.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 954e26079cd6..08bfeb8c32ea 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,25 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>         if (!enable_pml)
>                 exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>
> +       if (vmx_xsaves_supported()) {
> +               /* Exposing XSAVES only when XSAVE is exposed */
> +               bool xsaves_enabled =
> +                       guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
> +                       guest_cpuid_has(vcpu, X86_FEATURE_XSAVES);
> +
> +               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 +10438,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
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web