Re: [PATCH 1/5] xen: arm: correct whitespace/comments and use #defines in head.S

[Julien Grall <julien.grall@linaro.org>]

Hi Ian,

On 07/14/2014 05:39 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

> ---
>  xen/arch/arm/arm32/head.S |   11 +++++------
>  xen/arch/arm/arm64/head.S |   15 +++++++--------
>  2 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 8a6594a..1319a13 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -266,10 +266,10 @@ cpu_init_done:
>          strd  r2, r3, [r4, #0]       /* Map it in slot 0 */
>  
>          /* ... map of paddr(start) in boot_pgtable */
> -        lsrs  r1, r9, #30            /* Offset of base paddr in boot_pgtable */
> +        lsrs  r1, r9, #FIRST_SHIFT   /* Offset of base paddr in boot_pgtable */
>          beq   1f                     /* If it is in slot 0 then map in boot_second
>                                        * later on */
> -        lsl   r2, r1, #30            /* Base address for 1GB mapping */
> +        lsl   r2, r1, #FIRST_SHIFT   /* Base address for 1GB mapping */
>          orr   r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */
>          orr   r2, r2, #PT_LOWER(MEM)
>          lsl   r1, r1, #3             /* r1 := Slot offset */
> @@ -277,7 +277,7 @@ cpu_init_done:
>  
>  1:      /* Setup boot_second: */
>          ldr   r4, =boot_second
> -        add   r4, r4, r10            /* r1 := paddr (boot_second) */
> +        add   r4, r4, r10            /* r4 := paddr (boot_second) */
>  
>          lsr   r2, r9, #SECOND_SHIFT  /* Base address for 2MB mapping */
>          lsl   r2, r2, #SECOND_SHIFT
> @@ -320,7 +320,7 @@ paging:
>          dsb
>  #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
>          /* Non-boot CPUs don't need to rebuild the fixmap itself, just
> -	 * the mapping from boot_second to xen_fixmap */
> +         * the mapping from boot_second to xen_fixmap */
>          teq   r12, #0
>          bne   1f
>  
> @@ -397,8 +397,7 @@ launch:
>          beq   start_xen              /* and disappear into the land of C */
>          b     start_secondary        /* (to the appropriate entry point) */
>  
> -/* Fail-stop
> - * r0: string explaining why */
> +/* Fail-stop */
>  fail:   PRINT("- Boot failed -\r\n")
>  1:      wfe
>          b     1b
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 2a13527..883640c 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -267,11 +267,11 @@ skip_bss:
>          str   x2, [x4, #0]           /* Map it in slot 0 */
>  
>          /* ... map of paddr(start) in boot_pgtable */
> -        lsr   x1, x19, #39           /* Offset of base paddr in boot_pgtable */
> +        lsr   x1, x19, #ZEROETH_SHIFT/* Offset of base paddr in boot_pgtable */
>          cbz   x1, 1f                 /* It's in slot 0, map in boot_first
>                                        * or boot_second later on */
>  
> -        lsl   x2, x1, #39            /* Base address for 512GB mapping */
> +        lsl   x2, x1, #ZEROETH_SHIFT /* Base address for 512GB mapping */
>          mov   x3, #PT_MEM            /* x2 := Section mapping */
>          orr   x2, x2, x3
>          lsl   x1, x1, #3             /* x1 := Slot offset */
> @@ -284,7 +284,7 @@ skip_bss:
>          /* ... map boot_second in boot_first[0] */
>          ldr   x1, =boot_second
>          add   x1, x1, x20            /* x1 := paddr(boot_second) */
> -        mov   x3, #PT_PT             /* x2 := table map of boot_first */
> +        mov   x3, #PT_PT             /* x2 := table map of boot_second */
>          orr   x2, x1, x3             /*       + rights for linear PT */
>          str   x2, [x4, #0]           /* Map it in slot 0 */
>  
> @@ -300,8 +300,8 @@ skip_bss:
>          str   x2, [x4, x1]           /* Create mapping of paddr(start)*/
>  
>  1:      /* Setup boot_second: */
> -        ldr   x4, =boot_second
> -        add   x4, x4, x20            /* x4 := paddr (boot_second) */
> +        ldr   x4, =boot_second       /* Next level into boot_second */
> +        add   x4, x4, x20            /* x4 := paddr(boot_second) */
>  
>          lsr   x2, x19, #SECOND_SHIFT /* Base address for 2MB mapping */
>          lsl   x2, x2, #SECOND_SHIFT
> @@ -345,7 +345,7 @@ paging:
>          dsb   sy
>  #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
>          /* Non-boot CPUs don't need to rebuild the fixmap itself, just
> -	 * the mapping from boot_second to xen_fixmap */
> +         * the mapping from boot_second to xen_fixmap */
>          cbnz  x22, 1f
>  
>          /* Add UART to the fixmap table */
> @@ -417,8 +417,7 @@ launch:
>          cbz   x22, start_xen         /* and disappear into the land of C */
>          b     start_secondary        /* (to the appropriate entry point) */
>  
> -/* Fail-stop
> - * r0: string explaining why */
> +/* Fail-stop */
>  fail:   PRINT("- Boot failed -\r\n")
>  1:      wfe
>          b     1b
> 

Regards,

-- 
Julien Grall