Re: [PATCH] kvm: vmx: Raise #UD on unsupported RDRAND

[David Hildenbrand]

On 23.08.2017 23:39, Paolo Bonzini wrote:
> 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: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  arch/x86/kvm/vmx.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 8346fd53a42f..b13370381988 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3661,6 +3661,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_RDSEED |
>  			SECONDARY_EXEC_ENABLE_PML |
>  			SECONDARY_EXEC_TSC_SCALING |
> @@ -5295,6 +5296,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_RDRAND))
> +		exec_control &= ~SECONDARY_EXEC_RDRAND;
> +
>  	if (guest_cpuid_has(&vmx->vcpu, X86_FEATURE_RDSEED))
>  		exec_control &= ~SECONDARY_EXEC_RDSEED;
>  
> @@ -8051,6 +8055,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_RDSEED]                  = handle_invalid_op,
>  	[EXIT_REASON_XSAVES]                  = handle_xsaves,
>  	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
> @@ -8980,6 +8985,12 @@ static bool vmx_mpx_supported(void)
>  		(vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS);
>  }
>  
> +static bool vmx_rdrand_supported(void)
> +{
> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
> +		SECONDARY_EXEC_RDRAND;
> +}
> +
>  static bool vmx_rdseed_supported(void)
>  {
>  	return vmcs_config.cpu_based_2nd_exec_ctrl &
> @@ -9735,6 +9746,24 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  		}
>  	}
>  
> +	if (vmx_rdrand_supported()) {
> +		bool rdrand_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDRAND);
> +
> +		if (rdrand_enabled)
> +			secondary_exec_ctl &= ~SECONDARY_EXEC_RDRAND;
> +		else
> +			secondary_exec_ctl |= 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;
> +		}
> +	}
> +
>  	if (vmx_rdseed_supported()) {
>  		bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
>  
> 

>From what I can tell, this looks good to me.

Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>

-- 

Thanks,

David