From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 08/17] x86emul: generate and make use of canonical opcode representation
Date: Thu, 15 Sep 2016 00:43:03 -0600 [thread overview]
Message-ID: <57DA5F17020000780010F112@prv-mh.provo.novell.com> (raw)
In-Reply-To: <cc5c7983-920a-c1f7-6a33-264a1b7a55ab@citrix.com>
>>> On 14.09.16 at 19:30, <andrew.cooper3@citrix.com> wrote:
>> @@ -435,6 +438,51 @@ struct x86_emulate_ctxt
>> void *data;
>> };
>>
>> +/*
>> + * This encodes the opcode extension in a "natural" way:
>
> I am not sure what you mean by natural way here. All you seem to mean
> is that you are encoding instructions with the following method
Hence the quotes. Do you have a suggestion for a better word?
>> + * 0x0fxxxx for 0f-prefixed opcodes (or their VEX/EVEX equivalents)
>> + * 0x0f38xxxx for 0f38-prefixed opcodes (or their VEX/EVEX equivalents)
>> + * 0x0f3axxxx for 0f3a-prefixed opcodes (or their VEX/EVEX equivalents)
>> + * 0x8f08xxxx for 8f/8-prefixed XOP opcodes
>> + * 0x8f09xxxx for 8f/9-prefixed XOP opcodes
>> + * 0x8f0axxxx for 8f/a-prefixed XOP opcodes
>> + * Hence no separate #define-s get added.
>
> Please also describe what the xxxx fields mean. Looking below, I guess
> that the bottom byte is the opcode itself, and some bits of the 2nd byte
> are legacy prefixes?
Yes. Comment extended.
>> + */
>> +#define X86EMUL_OPC_EXT_MASK 0xffff0000
>> +#define X86EMUL_OPC(ext, byte) ((byte) | \
>> + MASK_INSR((ext),
> X86EMUL_OPC_EXT_MASK))
>
> I would highly suggest using ((byte) & 0xff). In the case that a change
> is slightly out of range, this should cause a compiler error (duplicate
> case statement) rather than a very subtle bug.
Well, okay.
>> +/*
>> + * This includes the 0x66, 0xF3, and 0xF2 prefixes when used to alter
>> + * functionality instead of just insn attributes, as well as VEX/EVEX:
>> + */
>> +#define X86EMUL_OPC_MASK (0x000000ff | X86EMUL_OPC_PFX_MASK | \
>> + X86EMUL_OPC_KIND_MASK)
>
> The definition should presumably live after introducing the PFX_MASK and
> KIND_MASK ?
I would prefer it to live closer to X86EMUL_OPC_EXT_MASK.
>> +#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.
>> +
>> +#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.
>> +# 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.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-09-15 6:43 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-08 12:58 [PATCH 00/17] x86: split insn emulator decode and execution Jan Beulich
2016-09-08 13:04 ` [PATCH 01/17] x86emul: split instruction decoding from execution Jan Beulich
2016-09-09 18:35 ` Andrew Cooper
2016-09-12 7:20 ` Jan Beulich
2016-09-08 13:07 ` [PATCH 02/17] x86emul: fetch all insn bytes during the decode phase Jan Beulich
2016-09-13 18:44 ` Andrew Cooper
2016-09-14 9:55 ` Jan Beulich
2016-09-23 14:48 ` Andrew Cooper
2016-09-23 15:04 ` Jan Beulich
2016-09-08 13:08 ` [PATCH 04/17] x86emul: track only rIP in emulator state Jan Beulich
2016-09-08 13:23 ` Jan Beulich
2016-09-08 13:09 ` [PATCH 03/17] " Jan Beulich
2016-09-13 19:09 ` Andrew Cooper
2016-09-14 9:58 ` Jan Beulich
2016-09-08 13:10 ` [PATCH 04/17] x86emul: complete decoding of two-byte instructions Jan Beulich
2016-09-14 14:22 ` Andrew Cooper
2016-09-14 15:05 ` Jan Beulich
2016-09-23 16:34 ` Andrew Cooper
2016-09-26 7:34 ` Jan Beulich
2016-09-27 13:28 ` Andrew Cooper
2016-09-27 13:51 ` Jan Beulich
2016-09-08 13:11 ` [PATCH 05/17] x86emul: add XOP decoding Jan Beulich
2016-09-14 16:11 ` Andrew Cooper
2016-09-14 16:21 ` Jan Beulich
2016-09-23 17:01 ` Andrew Cooper
2016-09-08 13:12 ` [PATCH 06/17] x86emul: add EVEX decoding Jan Beulich
2016-09-14 17:05 ` Andrew Cooper
2016-09-15 6:26 ` Jan Beulich
2016-09-08 13:13 ` [PATCH 07/17] x86emul: move x86_execute() common epilogue code Jan Beulich
2016-09-08 13:28 ` Jan Beulich
2016-09-14 17:13 ` Andrew Cooper
2016-09-08 13:14 ` [PATCH 08/17] x86emul: generate and make use of canonical opcode representation Jan Beulich
2016-09-14 17:30 ` Andrew Cooper
2016-09-15 6:43 ` Jan Beulich [this message]
2016-09-27 14:03 ` Andrew Cooper
2016-09-28 7:24 ` Jan Beulich
2016-09-08 13:14 ` [PATCH 09/17] SVM: use generic instruction decoding Jan Beulich
2016-09-14 17:56 ` Andrew Cooper
2016-09-15 6:55 ` Jan Beulich
2016-09-27 13:42 ` Andrew Cooper
2016-09-27 13:56 ` Jan Beulich
2016-09-27 15:53 ` Andrew Cooper
2016-09-08 13:16 ` [PATCH 10/17] x86/32on64: use generic instruction decoding for call gate emulation Jan Beulich
2016-09-08 13:17 ` [PATCH 11/17] x86/PV: split out dealing with CRn from privileged instruction handling Jan Beulich
2016-09-08 13:17 ` [PATCH 12/17] x86/PV: split out dealing with DRn " Jan Beulich
2016-09-08 13:18 ` [PATCH 13/17] x86/PV: split out dealing with MSRs " Jan Beulich
2016-09-08 13:18 ` [PATCH 14/17] x86emul: support XSETBV Jan Beulich
2016-09-08 13:19 ` [PATCH 15/17] x86emul: sort opcode 0f01 special case switch() statement Jan Beulich
2016-09-08 13:20 ` [PATCH 16/17] x86/PV: use generic emulator for privileged instruction handling Jan Beulich
2016-09-08 13:21 ` [PATCH 17/17] x86emul: don't assume a memory operand Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57DA5F17020000780010F112@prv-mh.provo.novell.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).