Re: [Xen-devel] [PATCH v2 01/13] x86emul/fuzz: add rudimentary limit checking

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.10.17 at 19:06, <george.dunlap@xxxxxxxxxx> wrote:
> On 10/06/2017 04:54 PM, Jan Beulich wrote:
>>>>> On 06.10.17 at 17:21, <george.dunlap@xxxxxxxxxx> wrote:
>>> On Mon, Sep 25, 2017 at 3:26 PM, George Dunlap <george.dunlap@xxxxxxxxxx> 
>>> wrote:
> One more thing:
> 
>> @@ -1249,10 +1249,10 @@ static void __put_rep_prefix(
>>
>>  /* Clip maximum repetitions so that the index register at most just wraps. 
>> */
>>  #define truncate_ea_and_reps(ea, reps, bytes_per_rep) ({
>> -    unsigned long todo__, ea__ = truncate_word(ea, ad_bytes);
>> +    unsigned long todo__, ea__ = truncate_ea(ea);
>>      if ( !(_regs.eflags & X86_EFLAGS_DF) )
>> -        todo__ = truncate_word(-(ea), ad_bytes) / (bytes_per_rep);
>> -    else if ( truncate_word((ea) + (bytes_per_rep) - 1, ad_bytes) < ea__ )\
>> +        todo__ = truncate_ea(-ea__) / (bytes_per_rep);
>> +    else if ( truncate_ea(ea__ + (bytes_per_rep) - 1) < ea__ )
> 
> This changes truncate_ea(-ea) to truncate_ea(-truncate_ea(ea)), and
> truncate_ea(ea + bpr-1) to truncate_ea(truncate_ea(ea) + bpr-1).  That
> sounds like a plausible change, but it's worth checking to see that it
> was intentional.

The main goal here was to eliminate the multiple evaluation of
the macro argument plus the open-coding of truncate_ea(). With
truncate_ea(truncate_ea(x)) == truncate_ea(x) and
truncate_ea(-truncate_ea(x)) == truncate_ea(-x) there's no
change in net result. So yes, the change was intentional.

> I'm not at the moment able to evaluate the assertion that "the
> specification allows for either original or new behavior for two-part
> accesses", nor that there is a 1:1 mapping between what this patch
> changes and what needs to be changed.

Section "Limit Checking" in volume 3 has

"When the effective limit is FFFFFFFFH (4 GBytes), these accesses
 may or may not cause the indicated exceptions. Behavior is
 implementation-specific and may vary from one execution to another."

Additionally section "Segment Wraparound" in volume 3 has

"The behavior when executing near the limit of a 4-GByte selector
 (limit = FFFFFFFFH) is different between the Pentium Pro and the
 Pentium 4 family of processors. On the Pentium Pro, instructions
 which cross the limit -- for example, a two byte instruction such as
 INC EAX that is encoded as FFH C0H starting exactly at the limit
 faults for a segment violation (a one byte instruction at FFFFFFFFH
 does not cause an exception). Using the Pentium 4 microprocessor
 family, neither of these situations causes a fault."

> But assuming the above are true (and the change I asked about was
> intentional):
> 
> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

Thanks.

> If that's not good enough to check it in, feel free to give me the
> appropriate references to the Intel manual that will allow me to
> independently verify those facts, and get rid of my caveat.

The patch still needs Andrew's ack before it can go in.

Jan


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