Re: [PATCH v2 3/3] x86/emul: Support CPUID faulting via a speculative MSR read

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 20/02/17 10:59, Jan Beulich wrote:
>>>> On 20.02.17 at 11:28, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -5424,10 +5424,25 @@ x86_emulate(
>>          break;
>>  
>>      case X86EMUL_OPC(0x0f, 0xa2): /* cpuid */
>> +        msr_val = 0;
>>          fail_if(ops->cpuid == NULL);
>> +
>> +        /* Speculatively read MSR_INTEL_MISC_FEATURES_ENABLES. */
>> +        if ( ops->read_msr &&
>> +             (rc = ops->read_msr(MSR_INTEL_MISC_FEATURES_ENABLES,
>> +                                 &msr_val, ctxt)) == X86EMUL_EXCEPTION )
>> +        {
>> +            /* Not implemented.  Squash the exception and proceed normally. */
>> +            x86_emul_reset_event(ctxt);
>> +            rc = X86EMUL_OKAY;
>> +        }
>> +        if ( rc != X86EMUL_OKAY )
>> +            goto done;
>> +
>> +        generate_exception_if((msr_val & MSR_MISC_FEATURES_CPUID_FAULTING) &&
>> +                              !mode_ring0(), EXC_GP, 0); /* Faulting active? */
> Could you please move the mode_ring0() check up to bypass the
> MSR read in the first place?

Ok.

>
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
>> @@ -413,12 +413,7 @@ struct x86_emulate_ops
>>      int (*wbinvd)(
>>          struct x86_emulate_ctxt *ctxt);
>>  
>> -    /*
>> -     * cpuid: Emulate CPUID via given set of EAX-EDX inputs/outputs.
>> -     *
>> -     * May return X86EMUL_EXCEPTION, which causes the emulator to inject
>> -     * #GP[0].  Used to implement CPUID faulting.
>> -     */
>> +    /* cpuid: Emulate CPUID via given set of EAX-EDX inputs/outputs. */
>>      int (*cpuid)(
>>          uint32_t leaf,
>>          uint32_t subleaf,
> Are there any ways left for the hook to fail? IOW, should its return
> type become void now?

In principle, it should return RETRY when putting a cpuid entry into the
vmevent ring.  At the moment, that logic is wedged into the vmx hook, so
emulated cpuid instructions don't generate events, but I am looking to
fix this (mis)behaviour in the long run.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel