Re: [Xen-devel] [PATCH] x86emul: defer rIP-relative address calculation

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/12/16 15:47, Jan Beulich wrote:
>>>> On 07.12.16 at 16:43, <JBeulich@xxxxxxxx> wrote:
>>>>> On 07.12.16 at 16:38, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 07/12/16 14:07, Jan Beulich wrote:
>>>> By putting it after all instruction fetching has been done, we can both
>>>> simplify the existing handling of immediate operands and take care of
>>>> any future instructions allowing rIP-relative operands and getting
>>>> additional bytes fetched in x86_decode_*() (the current cases of extra
>>>> bytes getting fetched there are only for operands without ModR/M bytes,
>>>> or with them only allowing their register forms).
>>>>
>>>> Similarly the new placement of truncate_ea() will take care of any
>>>> future cases of non-standard memory operands (the one existing case -
>>>> opcodes A0...A3 - are fine with and without this, as they fetch an
>>>> ad_bytes sized unsigned address anyway).
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> This is rather clearer to follow.
>>>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, although...
>>>
>>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>>> @@ -1925,6 +1925,7 @@ x86_decode(
>>>>      uint8_t b, d, sib, sib_index, sib_base;
>>>>      unsigned int def_op_bytes, def_ad_bytes, opcode;
>>>>      enum x86_segment override_seg = x86_seg_none;
>>>> +    bool ip_rel = false;
>>> I would name this specifically rip_rel, as that is the term used in all
>>> the manuals.
>> And I specifically avoided it as being wrong in the context of the
>> 32-bit test harness. Would pc_rel suit you better than ip_rel?
> Actually the reference to the 32-bit test harness was wrong here
> (obviously). Instead, it is wrong in the context of 32-bit addressing
> in 64-bit mode.

Such a case would still have rip_rel = false.  This addressing mode is
unique to 64bit.

But yes, pc_rel is still slightly better if you insist for not using
rip_rel.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel