Re: [Xen-devel] [PATCH 1/2] x86/E820: don't overrun array

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.12.17 at 12:18, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 12/12/17 11:10, Jan Beulich wrote:
>> The bounds check needs to be done after the increment, not before, or
>> else it needs to use a one lower immediate. Also use word operations
>> rather than byte ones for both the increment and the compare (allowing
>> E820_BIOS_MAX to be more easily bumped, should the need ever arise).
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/arch/x86/boot/mem.S
>> +++ b/xen/arch/x86/boot/mem.S
>> @@ -22,11 +22,10 @@ get_memory_map:
>>          cmpl    $SMAP,%eax                      # check the return is `SMAP'
>>          jne     .Lmem88
>>  
>> -        movb    bootsym(e820nr),%al             # up to 128 entries
>> -        cmpb    $E820_BIOS_MAX,%al
>> +        incw    bootsym(e820nr)
>> +        cmpw    $E820_BIOS_MAX,bootsym(e820nr)  # up to this many entries
> 
> Space after the comma here please.

Granted the file isn't consistent, but I had intentionally not added
a comma here, to keep things uniform with the neighboring blocks.

> Given your subsequent instruction scheduling patch, why the word
> operations? 32bit operations are faster than 16bit.

Not in 16-bit mode. Along the lines of the other patch the primary
goal isn't insn scheduling, but insn size (to keep the trampoline
small), so I'd like to avoid the operand size overrides.

Jan


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