Re: [PATCH for-xen-4.5 v3 15/16] x86/boot: Use %ecx instead of %eax

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 08/10/14 18:52, Daniel Kiper wrote:
> Use %ecx instead of %eax and do not wipe multiboot protocol identifier.
> We need that info in __reloc() to differentiate between multiboot(1)
> and multiboot2 protocol.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>

Much easier to review!

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> given 1 tiny
comment...

> ---
>  xen/arch/x86/boot/head.S |   23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index f1b872a..18df0b7 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -86,14 +86,14 @@ __start:
>          jne     not_multiboot
>  
>          /* Set up trampoline segment 64k below EBDA */
> -        movzwl  0x40e,%eax          /* EBDA segment */
> -        cmp     $0xa000,%eax        /* sanity check (high) */
> +        movzwl  0x40e,%ecx          /* EBDA segment */
> +        cmp     $0xa000,%ecx        /* sanity check (high) */
>          jae     0f
> -        cmp     $0x4000,%eax        /* sanity check (low) */
> +        cmp     $0x4000,%ecx        /* sanity check (low) */
>          jae     1f
>  0:
> -        movzwl  0x413,%eax          /* use base memory size on failure */
> -        shl     $10-4,%eax
> +        movzwl  0x413,%ecx          /* use base memory size on failure */
> +        shl     $10-4,%ecx
>  1:
>          /*
>           * Compare the value in the BDA with the information from the
> @@ -105,19 +105,20 @@ __start:
>          cmp     $0x100,%edx         /* is the multiboot value too small? */
>          jb      2f                  /* if so, do not use it */
>          shl     $10-4,%edx
> -        cmp     %eax,%edx           /* compare with BDA value */
> -        cmovb   %edx,%eax           /* and use the smaller */
> +        cmp     %ecx,%edx           /* compare with BDA value */
> +        cmovb   %edx,%ecx           /* and use the smaller */
>  
>  2:      /* Reserve 64kb for the trampoline */
> -        sub     $0x1000,%eax
> +        sub     $0x1000,%ecx
>  
>          /* From arch/x86/smpboot.c: start_eip had better be page-aligned! */
> -        xor     %al, %al
> -        shl     $4, %eax
> -        mov     %eax,sym_phys(trampoline_phys)
> +        xor     %cl, %cl
> +        shl     $4, %ecx
> +        mov     %ecx,sym_phys(trampoline_phys)
>  
>          /* Save the Multiboot info struct (after relocation) for later use. */
>          mov     $sym_phys(cpu0_stack)+1024,%esp
> +        mov     %ecx,%eax

Probably useful to identify which parameters are in which registers when
calling into reloc

~Andrew

>          push    %ebx
>          call    reloc
>          mov     %eax,sym_phys(mbd_pa)