Re: [PATCH 02/17] x86emul: fetch all insn bytes during the decode phase

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

On 14/09/16 10:55, Jan Beulich wrote:
>>>> On 13.09.16 at 20:44, <andrew.cooper3@citrix.com> wrote:
>> On 08/09/16 14:07, Jan Beulich wrote:
>>> @@ -1602,6 +1602,45 @@ struct x86_emulate_state {
>>>   #define _regs (state->regs)
>>>   
>>>   static int
>>> +x86_decode_base(
>> What do you mean by decode_base here?
> The base instruction set (no 0f or alike prefixes). Suggestions for
> a better name welcome.

x86_decode_onebyte() to match the table of opcodes it is further decoding.

>
>>> @@ -2644,18 +2704,13 @@ x86_emulate(
>>>   
>>>       case 0x9a: /* call (far, absolute) */ {
>>>           struct segment_register reg;
>>> -        uint16_t sel;
>>> -        uint32_t eip;
>>>   
>>> -        generate_exception_if(mode_64bit(), EXC_UD, -1);
>>> +        ASSERT(!mode_64bit());
>> Are we going to strictly require that noone ever hand-crafts a
>> x86_emulate_state and hands it to x86_emulate()?
> Absolutely - that's why its definition does not live in a header.

Ok.

>
>> I would suggest leaving the generate_exception_if(mode_64bit(), EXC_UD,
>> -1); after the ASSERT() so even if we do end up in a wonky state, we
>> don't try to jump the guest to 0.
> That would look really strange to a reader, I think, and hence I'd
> rather not do this if I can get the patch accepted without.

It is conceptually no different to

default:
ASSERT_UNREACHABLE();
return;

~Andrew

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