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

[David Hildenbrand <david@xxxxxxxxxx>]

On 18.08.2017 20:43, 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.

Would the same also hold for nVMX guests? I think if our L1 CPU does not
have RSEED, then also the L2 CPU should not be allowed to use it.

@@ -10371,6 +10371,7 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu,
struct vmcs12 *vmcs12,
                                  SECONDARY_EXEC_RDTSCP |
                                  SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
                                  SECONDARY_EXEC_APIC_REGISTER_VIRT |
+                                 SECONDARY_EXEC_RDSEED_EXITING |
                                  SECONDARY_EXEC_ENABLE_VMFUNC);
                if (nested_cpu_has(vmcs12,
                                   CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)) {


and maybe also


+++ b/arch/x86/kvm/vmx.c
@@ -2811,6 +2811,7 @@ static void nested_vmx_setup_ctls_msrs(struct
vcpu_vmx *vmx)
                SECONDARY_EXEC_RDRAND | SECONDARY_EXEC_RDSEED |
                SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
                SECONDARY_EXEC_RDTSCP |
+               SECONDARY_EXEC_RDSEED_EXITING |
                SECONDARY_EXEC_DESC |
                SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
                SECONDARY_EXEC_APIC_REGISTER_VIRT |

(but I always get confused about the level of filtering)

> ---
>  arch/x86/kvm/vmx.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ed1074e98b8e..8b9015f081b7 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_EXITING |
>  			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_EXITING;
> +
>  	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;
> +}
> +

(unrelated to this patch)
just wondering if we should now replace most code fragments

kvm_queue_exception(vcpu, UD_VECTOR);
return 1;

by

return handle_invalid_op(vcpu);


>  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,
> 


-- 

Thanks,

David