Re: [Xen-devel] [PATCH v3 2/5] x86emul: don't assume a memory operand

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.12.16 at 14:05, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 07/12/16 08:22, Jan Beulich wrote:
>>>>> On 06.12.16 at 17:49, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 06/12/16 11:13, Jan Beulich wrote:
>>>> @@ -2359,7 +2360,7 @@ x86_decode(
>>>>          }
>>>>      }
>>>>  
>>>> -    if ( override_seg != -1 && ea.type == OP_MEM )
>>>> +    if ( override_seg != x86_seg_none )
>>>>          ea.mem.seg = override_seg;
>>> Could we get away with asserting ea.type == OP_MEM if override_seg is
>>> set, to help validate our assumptions about state?  (Possibly even
>>> passing #UD back in the non-debug case)
>> That would be wrong - we'd be asserting guest controlled state.
>> There's nothing preventing a segment override to be present on
>> instructions without memory operands.
> 
> True, but such overrides should have no effect on the instruction.

Correct. But we can't ASSERT() anything for that reason.

>> And for example string
>> insns don't have OP_MEM set despite having (implicit) memory
>> operands (after all that's the hole reason for the change here
>> [but not the patch as a hole], as the following PV priv-op patch
> 
> Do you mean whole instead of hole here?

Yes - shame on me, even twice the same mistake.

>> requires the segment override to take effect on OUTS). Nor
>> would such be correct for conditional branches, where some of
>> the segment overrides have a different meaning (necessarily
>> ignored by the emulator).
> 
> The point I am trying to address is that it feels wrong to be
> referencing ea.mem for non OP_MEM types.  I accept that, no longer being
> a union, this use should be safe.
> 
> The string instructions are certainly a spanner in the works, but the
> jump instructions need not care about likely/unlikely prefixes.  They
> were purely advisory to start with, and are ignored on all modern hardware.
> 
> Another spanner in the works is the upcoming meaning of %ds overrides
> for jump instructions as the no-track hint from the forthcoming
> Control-flow Enforcement extensions.
> 
> Given that there can only ever be one active segment override, does it
> need to be stored in ea.mem in the first place, or could it live
> somewhere else which is mem-neutral?

It could, but that wouldn't make ea.mem.seg go away, and it's a
convenient place to put it. Also if we put it elsewhere we'd have
to (a) change all existing uses (including all places structure-
assigning dst or src from ea), and be rather careful with future
changes.

Jan


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