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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
From: Jim Mattson <[email protected]>
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 <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
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
From: Jim Mattson <[email protected]>
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 <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
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
On Thu, Aug 24, 2017 at 6:37 AM, Paolo Bonzini <[email protected]> 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 <[email protected]>
> ---
> 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
>
>
On Thu, Aug 24, 2017 at 6:37 AM, Paolo Bonzini <[email protected]> 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 <[email protected]>
> ---
> 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
>
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
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 <[email protected]> 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
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 <[email protected]> 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
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 <[email protected]> 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 <[email protected]> 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
>
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 <[email protected]>
Date: Mon Jul 2 01:18:48 2012 +0000
KVM: VMX: Implement PCID/INVPCID for guests with EPT
Paolo
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 <[email protected]>
---
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
On 24.08.2017 15:37, Paolo Bonzini wrote:
> From: Jim Mattson <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> 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 <[email protected]>
--
Thanks,
David
On 24.08.2017 15:37, Paolo Bonzini wrote:
> From: Jim Mattson <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> 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 <[email protected]>
--
Thanks,
David
Reviewed-by: Jim Mattson <[email protected]>
On Thu, Aug 24, 2017 at 9:09 AM, Paolo Bonzini <[email protected]> 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 <[email protected]>
> ---
> 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
>