Re: [Xen-devel] [PATCH 08/17] xen/arm64: head: Rework and document zero_bss()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 10 Jun 2019, Julien Grall wrote:
> On secondary CPUs, zero_bss() will be a NOP because BSS only need to be
> zeroed once at boot. So the call in the secondary CPUs path can be
> removed. It also means that x26 does not need to set and is now only
> used by the boot CPU.
> 
> Lastly, document the behavior and the main registers usage within the
> function.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/arm64/head.S | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 87fcd3be6c..6aa3148192 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -71,7 +71,7 @@
>   *  x23 - UART address
>   *  x24 -
>   *  x25 - identity map in place
> - *  x26 - skip_zero_bss
> + *  x26 - skip_zero_bss (boot cpu only)

you could remove this, see below...


>   *  x27 -
>   *  x28 -
>   *  x29 -
> @@ -313,8 +313,6 @@ GLOBAL(init_secondary)
>          sub   x20, x19, x0           /* x20 := phys-offset */
>  
>          mov   x22, #1                /* x22 := is_secondary_cpu */
> -        /* Boot CPU already zero BSS so skip it on secondary CPUs. */
> -        mov   x26, #1                /* X26 := skip_zero_bss */
>  
>          mrs   x0, mpidr_el1
>          ldr   x13, =(~MPIDR_HWID_MASK)
> @@ -337,7 +335,6 @@ GLOBAL(init_secondary)
>          PRINT(" booting -\r\n")
>  #endif
>          bl    check_cpu_mode
> -        bl    zero_bss
>          bl    cpu_init
>          bl    create_page_tables
>          bl    enable_mmu
> @@ -375,6 +372,14 @@ check_cpu_mode:
>          b fail
>  ENDPROC(check_cpu_mode)
>  
> +/*
> + * Zero BSS
> + *
> + * Inputs:
> + *   x26: Do we need to zero BSS?
> + *
> + * Clobbers x0 - x3
> + */
>  zero_bss:
>          /* Zero BSS only when requested */
>          cbnz  x26, skip_bss

In the commit message you wrote: "It also means that x26 does not need
to set and is now only used by the boot CPU." I think this statement is
correct, so you could also remove this "cbnz  x26, skip_bss" and also
the skip_bss label below.

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