Re: [Xen-devel] [PATCH v5 08/14] x86emul: add read-modify-write hook

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.18 at 15:19, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 15/03/18 13:08, Jan Beulich wrote:
>> @@ -8517,6 +8690,142 @@ x86_emulate(
>>  #undef vex
>>  #undef ea
>>  
>> +int x86_emul_rmw(
>> +    void *ptr,
>> +    unsigned int bytes,
>> +    uint32_t *eflags,
>> +    struct x86_emulate_state *state,
>> +    struct x86_emulate_ctxt *ctxt)
>> +{
>> +    unsigned long *dst = ptr;
>> +
>> +    ASSERT(bytes == state->op_bytes);
>> +
>> +#ifdef __x86_64__
>> +# define JCXZ "jrcxz"
>> +#else
>> +# define JCXZ "jecxz"
>> +#endif
>> +
>> +#define COND_LOCK(op) \
>> +    JCXZ " .L" #op "%=\n\t" \
>> +    "lock\n" \
>> +    ".L" #op "%=:\n\t" \
>> +    #op
> 
> I'd forgotten that these encoding of jmp existed, but various ORMs
> suggest that it is far slower to execute than other jumps.
> 
> Irrespective of the instruction latency argument, you'll get better code
> generation with cond_op() looking rather more like:
> 
> cmpb $0, %[lock]
> je 1f
> lock
> 1: #op
> 
> which will most likely cause the cmp to be encoded with a memory
> operand, rather than forcing it the value into %ecx.

You're missing the main point of having chosen J{E,R}CXZ (arguably
I should have added a comment, and I now will): This operation sits
inside the range where we have loaded guest EFLAGS (the status
ones) into the hardware EFLAGS register. For ADC, SBB, RCL, and
RCR to function we must not alter CF here, and for flags not
modified by an insn (e.g. everything other than CF for BT*) we
must not alter any of the other status flags.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel