Re: [Xen-devel] [PATCH 03/17] x86emul: track only rIP in emulator state

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.09.16 at 21:09, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel