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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

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

As e820nr is already a 32bit value, I'd just move them straight to
incl/cmpl.

~Andrew

>          jae     .Lmem88
>  
> -        incb    bootsym(e820nr)
>          movw    %di,%ax
>          addw    $20,%ax
>          movw    %ax,%di
>
>
>


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