diff mbox

[v5] kvm: vmx: Raise #UD on unsupported RDSEED

Message ID 20170821192640.30817-1-jmattson@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jim Mattson Aug. 21, 2017, 7:26 p.m. UTC
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>
---
 arch/x86/kvm/vmx.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

David Hildenbrand Aug. 22, 2017, 11:11 a.m. UTC | #1
On 21.08.2017 21:26, Jim Mattson wrote:
> 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>

I'd really vote to update

SECONDARY_EXEC_RDSEED -> SECONDARY_EXEC_RDSEED_EXITING
SECONDARY_EXEC_RDRAND -> SECONDARY_EXEC_RDRAND_EXITING

Otherwise this looks like it would enable RDSEED interpretation. And
this way it matches the SDM definition.
Paolo Bonzini Aug. 23, 2017, 9:34 p.m. UTC | #2
On 21/08/2017 21:26, Jim Mattson wrote:
> 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>

Usually we're not that particular about #UD-ing on disabled instructions
(because you cannot do it for all instructions), but in this case I
agree that we should do it for the sake of getting vmexits right.

I'm applying v5 and posting shortly a similar patch for RDRAND.

Paolo

> ---
>  arch/x86/kvm/vmx.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ed1074e98b8e..61174e1f7d0f 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3662,6 +3662,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_ENABLE_PML |
>  			SECONDARY_EXEC_TSC_SCALING |
>  			SECONDARY_EXEC_ENABLE_VMFUNC;
> @@ -5298,6 +5299,9 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
>  	if (!enable_pml)
>  		exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>  
> +	if (guest_cpuid_has(&vmx->vcpu, X86_FEATURE_RDSEED))
> +		exec_control &= ~SECONDARY_EXEC_RDSEED;
> +
>  	return exec_control;
>  }
>  
> @@ -6806,6 +6810,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;
> @@ -8050,6 +8060,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_RDSEED]                  = handle_invalid_op,
>  	[EXIT_REASON_XSAVES]                  = handle_xsaves,
>  	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
>  	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
> @@ -8979,6 +8990,12 @@ static bool vmx_mpx_supported(void)
>  		(vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS);
>  }
>  
> +static bool vmx_rdseed_supported(void)
> +{
> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
> +		SECONDARY_EXEC_RDSEED;
> +}
> +
>  static bool vmx_xsaves_supported(void)
>  {
>  	return vmcs_config.cpu_based_2nd_exec_ctrl &
> @@ -9665,6 +9682,24 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  		}
>  	}
>  
> +	if (vmx_rdseed_supported()) {
> +		bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
> +
> +		if (rdseed_enabled)
> +			secondary_exec_ctl &= ~SECONDARY_EXEC_RDSEED;
> +		else
> +			secondary_exec_ctl |= 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;
> +		}
> +	}
> +
>  	if (cpu_has_secondary_exec_ctrls())
>  		vmcs_set_secondary_exec_control(secondary_exec_ctl);
>  
>
Jim Mattson Aug. 23, 2017, 10:37 p.m. UTC | #3
Wait, wait! I have v6 almost ready!

On Wed, Aug 23, 2017 at 2:34 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 21/08/2017 21:26, Jim Mattson wrote:
>> 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>
>
> Usually we're not that particular about #UD-ing on disabled instructions
> (because you cannot do it for all instructions), but in this case I
> agree that we should do it for the sake of getting vmexits right.
>
> I'm applying v5 and posting shortly a similar patch for RDRAND.
>
> Paolo
>
>> ---
>>  arch/x86/kvm/vmx.c | 35 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index ed1074e98b8e..61174e1f7d0f 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -3662,6 +3662,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_ENABLE_PML |
>>                       SECONDARY_EXEC_TSC_SCALING |
>>                       SECONDARY_EXEC_ENABLE_VMFUNC;
>> @@ -5298,6 +5299,9 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
>>       if (!enable_pml)
>>               exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>>
>> +     if (guest_cpuid_has(&vmx->vcpu, X86_FEATURE_RDSEED))
>> +             exec_control &= ~SECONDARY_EXEC_RDSEED;
>> +
>>       return exec_control;
>>  }
>>
>> @@ -6806,6 +6810,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;
>> @@ -8050,6 +8060,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_RDSEED]                  = handle_invalid_op,
>>       [EXIT_REASON_XSAVES]                  = handle_xsaves,
>>       [EXIT_REASON_XRSTORS]                 = handle_xrstors,
>>       [EXIT_REASON_PML_FULL]                = handle_pml_full,
>> @@ -8979,6 +8990,12 @@ static bool vmx_mpx_supported(void)
>>               (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS);
>>  }
>>
>> +static bool vmx_rdseed_supported(void)
>> +{
>> +     return vmcs_config.cpu_based_2nd_exec_ctrl &
>> +             SECONDARY_EXEC_RDSEED;
>> +}
>> +
>>  static bool vmx_xsaves_supported(void)
>>  {
>>       return vmcs_config.cpu_based_2nd_exec_ctrl &
>> @@ -9665,6 +9682,24 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>>               }
>>       }
>>
>> +     if (vmx_rdseed_supported()) {
>> +             bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
>> +
>> +             if (rdseed_enabled)
>> +                     secondary_exec_ctl &= ~SECONDARY_EXEC_RDSEED;
>> +             else
>> +                     secondary_exec_ctl |= 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;
>> +             }
>> +     }
>> +
>>       if (cpu_has_secondary_exec_ctrls())
>>               vmcs_set_secondary_exec_control(secondary_exec_ctl);
>>
>>
>
diff mbox

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ed1074e98b8e..61174e1f7d0f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3662,6 +3662,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_ENABLE_PML |
 			SECONDARY_EXEC_TSC_SCALING |
 			SECONDARY_EXEC_ENABLE_VMFUNC;
@@ -5298,6 +5299,9 @@  static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
 	if (!enable_pml)
 		exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
 
+	if (guest_cpuid_has(&vmx->vcpu, X86_FEATURE_RDSEED))
+		exec_control &= ~SECONDARY_EXEC_RDSEED;
+
 	return exec_control;
 }
 
@@ -6806,6 +6810,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;
@@ -8050,6 +8060,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_RDSEED]                  = handle_invalid_op,
 	[EXIT_REASON_XSAVES]                  = handle_xsaves,
 	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
 	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
@@ -8979,6 +8990,12 @@  static bool vmx_mpx_supported(void)
 		(vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS);
 }
 
+static bool vmx_rdseed_supported(void)
+{
+	return vmcs_config.cpu_based_2nd_exec_ctrl &
+		SECONDARY_EXEC_RDSEED;
+}
+
 static bool vmx_xsaves_supported(void)
 {
 	return vmcs_config.cpu_based_2nd_exec_ctrl &
@@ -9665,6 +9682,24 @@  static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 		}
 	}
 
+	if (vmx_rdseed_supported()) {
+		bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
+
+		if (rdseed_enabled)
+			secondary_exec_ctl &= ~SECONDARY_EXEC_RDSEED;
+		else
+			secondary_exec_ctl |= 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;
+		}
+	}
+
 	if (cpu_has_secondary_exec_ctrls())
 		vmcs_set_secondary_exec_control(secondary_exec_ctl);