* [PATCH 1/4] KVM: VMX: cache secondary exec controls
2017-08-24 13:37 [PATCH v7 0/4] Raise #UD and disable execution controls for more CPUID bits Paolo Bonzini
@ 2017-08-24 13:37 ` Paolo Bonzini
2017-08-24 14:47 ` Jim Mattson
2017-08-24 13:37 ` [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND Paolo Bonzini
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2017-08-24 13:37 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: jmattson
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls
2017-08-24 13:37 ` [PATCH 1/4] KVM: VMX: cache secondary exec controls Paolo Bonzini
@ 2017-08-24 14:47 ` Jim Mattson
2017-08-24 15:25 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Jim Mattson @ 2017-08-24 14:47 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: LKML, kvm list
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
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls
2017-08-24 14:47 ` Jim Mattson
@ 2017-08-24 15:25 ` Paolo Bonzini
2017-08-24 15:41 ` Jim Mattson
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2017-08-24 15:25 UTC (permalink / raw)
To: Jim Mattson; +Cc: LKML, kvm list
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls
2017-08-24 15:25 ` Paolo Bonzini
@ 2017-08-24 15:41 ` Jim Mattson
2017-08-24 15:46 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Jim Mattson @ 2017-08-24 15:41 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: LKML, kvm list
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls
2017-08-24 15:41 ` Jim Mattson
@ 2017-08-24 15:46 ` Paolo Bonzini
2017-08-24 16:02 ` Jim Mattson
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2017-08-24 15:46 UTC (permalink / raw)
To: Jim Mattson; +Cc: LKML, kvm list
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls
2017-08-24 15:46 ` Paolo Bonzini
@ 2017-08-24 16:02 ` Jim Mattson
2017-08-24 16:08 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Jim Mattson @ 2017-08-24 16:02 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: LKML, kvm list
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] KVM: VMX: cache secondary exec controls
2017-08-24 16:02 ` Jim Mattson
@ 2017-08-24 16:08 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2017-08-24 16:08 UTC (permalink / raw)
To: Jim Mattson; +Cc: LKML, kvm list
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND
2017-08-24 13:37 [PATCH v7 0/4] Raise #UD and disable execution controls for more CPUID bits Paolo Bonzini
2017-08-24 13:37 ` [PATCH 1/4] KVM: VMX: cache secondary exec controls Paolo Bonzini
@ 2017-08-24 13:37 ` Paolo Bonzini
2017-08-24 17:42 ` David Hildenbrand
2017-08-24 13:37 ` [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED Paolo Bonzini
2017-08-24 13:37 ` [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS Paolo Bonzini
3 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2017-08-24 13:37 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: jmattson
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND
2017-08-24 13:37 ` [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND Paolo Bonzini
@ 2017-08-24 17:42 ` David Hildenbrand
0 siblings, 0 replies; 16+ messages in thread
From: David Hildenbrand @ 2017-08-24 17:42 UTC (permalink / raw)
To: Paolo Bonzini, linux-kernel, kvm; +Cc: jmattson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED
2017-08-24 13:37 [PATCH v7 0/4] Raise #UD and disable execution controls for more CPUID bits Paolo Bonzini
2017-08-24 13:37 ` [PATCH 1/4] KVM: VMX: cache secondary exec controls Paolo Bonzini
2017-08-24 13:37 ` [PATCH 2/4] kvm: vmx: Raise #UD on unsupported RDRAND Paolo Bonzini
@ 2017-08-24 13:37 ` Paolo Bonzini
2017-08-24 17:42 ` David Hildenbrand
2017-08-24 13:37 ` [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS Paolo Bonzini
3 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2017-08-24 13:37 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: jmattson
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED
2017-08-24 13:37 ` [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED Paolo Bonzini
@ 2017-08-24 17:42 ` David Hildenbrand
0 siblings, 0 replies; 16+ messages in thread
From: David Hildenbrand @ 2017-08-24 17:42 UTC (permalink / raw)
To: Paolo Bonzini, linux-kernel, kvm; +Cc: jmattson
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS
2017-08-24 13:37 [PATCH v7 0/4] Raise #UD and disable execution controls for more CPUID bits Paolo Bonzini
` (2 preceding siblings ...)
2017-08-24 13:37 ` [PATCH 3/4] kvm: vmx: Raise #UD on unsupported RDSEED Paolo Bonzini
@ 2017-08-24 13:37 ` Paolo Bonzini
2017-08-24 14:54 ` Jim Mattson
3 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2017-08-24 13:37 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: jmattson
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS
2017-08-24 13:37 ` [PATCH 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS Paolo Bonzini
@ 2017-08-24 14:54 ` Jim Mattson
2017-08-24 16:09 ` [PATCH v8 " Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Jim Mattson @ 2017-08-24 14:54 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: LKML, kvm list
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v8 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS
2017-08-24 14:54 ` Jim Mattson
@ 2017-08-24 16:09 ` Paolo Bonzini
2017-08-24 20:41 ` Jim Mattson
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2017-08-24 16:09 UTC (permalink / raw)
To: linux-kernel, kvm
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v8 4/4] kvm: vmx: Raise #UD on unsupported XSAVES/XRSTORS
2017-08-24 16:09 ` [PATCH v8 " Paolo Bonzini
@ 2017-08-24 20:41 ` Jim Mattson
0 siblings, 0 replies; 16+ messages in thread
From: Jim Mattson @ 2017-08-24 20:41 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: LKML, kvm list
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread