From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 08/17] x86emul: generate and make use of canonical opcode representation
Date: Wed, 14 Sep 2016 18:30:17 +0100 [thread overview]
Message-ID: <cc5c7983-920a-c1f7-6a33-264a1b7a55ab@citrix.com> (raw)
In-Reply-To: <57D1803A020000780010D18B@prv-mh.provo.novell.com>
On 08/09/16 14:14, Jan Beulich wrote:
"of a canonical opcode representation".
You appear to be inventing your own here, but it isn't the only
canonical form you could represent x86 opcodes with.
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -415,12 +415,15 @@ struct x86_emulate_ctxt
> /* Stack pointer width in bits (16, 32 or 64). */
> unsigned int sp_size;
>
> - /* Set this if writes may have side effects. */
> - uint8_t force_writeback;
> + /* Canonical opcode (see below). */
> + unsigned int opcode;
>
> /* Software event injection support. */
> enum x86_swint_emulation swint_emulate;
>
> + /* Set this if writes may have side effects. */
> + uint8_t force_writeback;
> +
> /* Retirement state, set by the emulator (valid only on X86EMUL_OKAY). */
> union {
> struct {
> @@ -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
> + * 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?
> + */
> +#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.
> +/*
> + * 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 ?
> +
> +#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.
> +
> +#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?
> +# 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?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-09-14 17:30 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 [this message]
2016-09-15 6:43 ` Jan Beulich
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=cc5c7983-920a-c1f7-6a33-264a1b7a55ab@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.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).