Re: [PATCH 08/17] x86emul: generate and make use of canonical opcode representation

["Jan Beulich" <JBeulich@suse.com>]

>>> On 27.09.16 at 16:03, <andrew.cooper3@citrix.com> wrote:
> On 15/09/16 07:43, Jan Beulich wrote:
>>>>> On 14.09.16 at 19:30, <andrew.cooper3@citrix.com> wrote:
>>>> +#define X86EMUL_OPC_PFX_MASK         0x00000300
>>>> +# define X86EMUL_OPC_66(ext, byte)   (X86EMUL_OPC(ext, byte) | 0x00000100)
>>>> +# define X86EMUL_OPC_F3(ext, byte)   (X86EMUL_OPC(ext, byte) | 0x00000200)
>>>> +# define X86EMUL_OPC_F2(ext, byte)   (X86EMUL_OPC(ext, byte) | 0x00000300)
>>> The PFX mask is moderately obvious from here, but a sentence describing
>>> what is legitimate to add in the future wouldn't go amiss.
>> I don't understand the "what is legitimate to add in the future"
>> part: Nothing should be added to this set.
> 
> It occurs to me that using only 2 bits rather than 8 bits for the prefix
> information would help the compiler make a smaller switch statements.

I don't think this would help - the compiler struggles with the
high 16 bits, and that wouldn't change. I'm surprised they're not
smart enough to split this into a few compares and a couple of
independent branch tables.

>>>> +#define X86EMUL_OPC_KIND_MASK        0x00003000
>>>> +#define X86EMUL_OPC_VEX_             0x00001000
>>> OTOH, I am rather more confused about what is eligible for inclusion
>>> into "kind".  Also, what does a kind of 0 indicate?
>> VEX, XOP, and EVEX are the valid non-zero kinds. Zero (I would
>> say obviously) means neither of those three.
> 
> It is not clear how "kind" is a suitable collective term for VEX/XOP/EVEX.
> 
> Or in other words, X86EMUL_OPC_KIND_MASK doesn't provide any hint that
> the operation is referring to a legacy or vex encoding of the instruction.
> 
> Would s/kind/encoding/ be ok?

Sure, changed.

> At that point, X86EMUL_OPC_LEGACY_ with a
> value of 0 might be useful.  (e.g. perhaps (opcode &
> X86EMUL_OPC_ENCODING_MASK) == X86EMUL_OPC_LEGACY_?)

Added for completeness (but it'll be unused for now).

>>>> +# define X86EMUL_OPC_VEX(ext, byte) \
>>>> +    (X86EMUL_OPC(ext, byte) | X86EMUL_OPC_VEX_)
>>>> +# define X86EMUL_OPC_VEX_66(ext, byte) \
>>>> +    (X86EMUL_OPC_66(ext, byte) | X86EMUL_OPC_VEX_)
>>>> +# define X86EMUL_OPC_VEX_F3(ext, byte) \
>>>> +    (X86EMUL_OPC_F3(ext, byte) | X86EMUL_OPC_VEX_)
>>>> +# define X86EMUL_OPC_VEX_F2(ext, byte) \
>>>> +    (X86EMUL_OPC_F2(ext, byte) | X86EMUL_OPC_VEX_)
>>>> +#define X86EMUL_OPC_EVEX_            0x00002000
>>>> +# define X86EMUL_OPC_EVEX(ext, byte) \
>>>> +    (X86EMUL_OPC(ext, byte) | X86EMUL_OPC_EVEX_)
>>>> +# define X86EMUL_OPC_EVEX_66(ext, byte) \
>>>> +    (X86EMUL_OPC_66(ext, byte) | X86EMUL_OPC_EVEX_)
>>>> +# define X86EMUL_OPC_EVEX_F3(ext, byte) \
>>>> +    (X86EMUL_OPC_F3(ext, byte) | X86EMUL_OPC_EVEX_)
>>>> +# define X86EMUL_OPC_EVEX_F2(ext, byte) \
>>>> +    (X86EMUL_OPC_F2(ext, byte) | X86EMUL_OPC_EVEX_)
>>> Why do we go to the effort of spelling out the individual VEX/EVEX
>>> possibilities, but not the XOP ones?
>> Because I need some of them right away, but we currently don't
>> emulate any XOP insns. If you feel strongly about it, I surely can
>> add XOP ones.
> 
> Thats ok - I presume we will be gaining some in due course.

Actually I was wrong with the earlier reply - the lack of XOP
counterparts is because they wouldn't get encoded this way.
Instead they'd use X86EMUL_OPC(0x8fXX, 0xYY). Whether a
"shorthand" to make this X86EMUL_OPC_XOP(0xXX, 0xYY) or
X86EMUL_OPC_XOP_XX(0xYY) would be worthwhile I'm not sure
at this point, so I'd rather leave it out until we actually get to
see what's most suitable.

Jan


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