Right. If L1 doesn't support RDSEED, then the corresponding
"allowed-1" bit in the IA32_VMX_PROCBASED_CTLS2 MSR should be cleared.
I think vmx_cpuid_update is the right place for this. Note, however,
that prepare_vmcs02() should still respect L0's setting of this bit.
On Mon, Aug 21, 2017 at 6:00 AM, David Hildenbrand <david@xxxxxxxxxx> wrote:
> 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
