Re: [Xen-devel] [PATCH] x86emul: support {RD,WR}{F,G}SBASE

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/12/16 14:49, Jan Beulich wrote:
>>>> On 14.12.16 at 14:47, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 14/12/16 13:39, Jan Beulich wrote:
>>>>>> On 14.12.16 at 14:28, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> On 14/12/16 13:18, Jan Beulich wrote:
>>>>>>>> On 14.12.16 at 13:36, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>> On 14/12/16 09:37, Jan Beulich wrote:
>>>>>>> @@ -5205,6 +5206,44 @@ x86_emulate(
>>>>>>>          }
>>>>>>>          break;
>>>>>>>  
>>>>>>> +    case X86EMUL_OPC_F3(0x0f, 0xae): /* Grp15 */
>>>>>>> +    {
>>>>>>> +        unsigned long cr4;
>>>>>>> +
>>>>>>> +        fail_if(modrm_mod != 3);
>>>>>> This should surely be an explicit #UD?  The only issue is that we don't
>>>>>> yet implement Grp15/F3 instructions with memory operands (as there are
>>>>>> none yet defined)?
>>>>> If there weren't any, I probably would have used #UD here. But
>>>>> there are - ptwrite is even in the normal SDM already (but it looks
>>>>> to be missing from the opcode map).
>>>> I find that the opcode maps are consistently out of date.
>>>>
>>>> However, I don't understand why you have chosen to avoid the #UD.  #UD
>>>> is the appropriate action for an opcode which isn't implemented.
>>> I don't think we should raise #UD knowingly for the wrong reason.
>>> Hence my plan was to go through all fail_if()-s once we are at a
>>> point where we consider the emulator complete enough, but keep
>>> using fail_if() vs #UD to distinguish cases where we know of gaps
>>> vs an encoding being undefined in up-to-date docs. While I guess
>>> we don't always match this model at present, that was at least
>>> what I have been trying to follow in all my recent work.
>> Hmm.
>>
>> I had considered at one point to have X86EMUL_NOT_IMPLEMENTED which was
>> separate from X86EMUL_UNHANDLEABLE, as they are subtly different. 
>> NOT_IMPLEMENTED means "everything else was fine, but I don't know how to
>> do that", whereas UNHANDLEABLE is "something went wrong trying to do
>> that", and is typically used for missing hooks or problems in lower levels.
> Fine with me, preferably when we're closer to covering most of the
> opcode space. Bottom line here though is that unless you insist I'd
> prefer to keep the fail_if() as being more in line with what we do
> elsewhere.

Fine for now.

~Andrew

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