Re: [PATCH] x86: always supply .cpuid() handler to x86_emulate()

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

On 11/11/16 14:58, Jan Beulich wrote:
>>>> On 11.11.16 at 15:16, <andrew.cooper3@citrix.com> wrote:
>> On 10/11/16 12:30, Jan Beulich wrote:
>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -2755,6 +2755,24 @@ static int priv_op_write_msr(unsigned in
>>>      return X86EMUL_UNHANDLEABLE;
>>>  }
>>>  
>>> +int pv_emul_cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
>>> +                  unsigned int *edx, struct x86_emulate_ctxt *ctxt)
>>> +{
>>> +    struct cpu_user_regs regs = *ctxt->regs;
>> You need a CPUID faulting check here, matching the hvm side of things,
>> or you will leave a latent bug which gets exposed when switching to
>> using full x86_emulate() for PV guests.
> That addition is part of the patch actually switching to x86_emulate().
> I don't think it belongs here, as for now we don't mean to emulate
> CPUID insns.

Fine, so long as it doesn't get forgotten.

>
>> Would it be wise to add a fail_if(!ops->cpuid) to x86_emulate() to catch
>> other misuses?
> I don't think so - there may be cases where it's indeed unneeded
> (and I foresee such arising quickly once the ->validate() hooks is
> in place, addition of which is now too part of aforementioned patch).

Ok.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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