From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 03/17] x86emul: track only rIP in emulator state
Date: Wed, 14 Sep 2016 03:58:43 -0600 [thread overview]
Message-ID: <57D93B73020000780010EB7C@prv-mh.provo.novell.com> (raw)
In-Reply-To: <308f5ed1-e997-f478-8ed0-9a2c511d2eda@citrix.com>
>>> On 13.09.16 at 21:09, <andrew.cooper3@citrix.com> wrote:
> On 08/09/16 14:09, Jan Beulich wrote:
>> Now that all decoding happens in x86_decode() there's no need to keep
>> the local registers copy in struct x86_emulate_state. Only rIP gets
>> updated in the decode phase, so only that register needs tracking
>> there. All other (read-only) registers can be read from the original
>> structure (but sadly, due to it getting passed to decode_register(),
>> the pointer can't be made point to "const" to make the compiler help
>> ensure no modification happens).
>
> I was going to suggest making a second helper and casting away
> constness, but that also has problems with the mark_regs_dirty() call.
>
> However, on further consideration...
>
>> @@ -2061,6 +2064,8 @@ x86_emulate(
>> struct x86_emulate_ctxt *ctxt,
>> const struct x86_emulate_ops *ops)
>> {
>> + /* Shadow copy of register state. Committed on successful emulation. */
>> + struct cpu_user_regs _regs = *ctxt->regs;
>> struct x86_emulate_state state;
>> int rc;
>> uint8_t b, d;
>> @@ -2074,10 +2079,21 @@ x86_emulate(
>> if ( rc != X86EMUL_OKAY)
>> return rc;
>>
>> + /* Sync rIP to post decode value. */
>> + _regs.eip = state.eip;
>> +
>> b = state.opcode;
>> d = state.desc;
>> #define state (&state)
>>
>> + /* Re-vector ea's register pointer into our shadow registers. */
>> + if ( ea.type == OP_REG )
>> + {
>> + unsigned int offs = (void *)ea.reg - (void *)state->regs;
>> +
>> + ea.reg = (void *)&_regs + offs;
>> + }
>> +
>
> This is some very hairy pointer arithmetic.
>
> Why do we need to decode registers in x86_decode()?
>
> We don't need to decode the GPRs to calculate the length of the
> instruction. If the displacement is stashed in x86_emulate_state, the
> calculation of ea can be deferred until the start of x86_emulate(), and
> no arithmetic like this would be necessary.
That's a good idea; I didn't really like this pointer arithmetic myself,
but didn't come to think of this (seemingly obvious) alternative.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-09-14 10:01 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 [this message]
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
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=57D93B73020000780010EB7C@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).