Re: [Xen-devel] [PATCH 03/17] xen/arm64: head: Rework UART initialization on boot CPU

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 10 Jun 2019, Julien Grall wrote:
> Anything executed after the label common_start can be executed on all
> CPUs. However most of the instructions executed between the label
> common_start and init_uart are not executed on the boot CPU.
> 
> The only instructions executed are to lookup the CPUID so it can be
> printed on the console (if earlyprintk is enabled). Printing the CPUID
> is not entirely useful to have for the boot CPU and requires a
> conditional branch to bypass unused instructions.
> 
> Furthermore, the function init_uart is only called for boot CPU
> requiring another conditional branch. This makes the code a bit tricky
> to follow.
> 
> The UART initialization is now moved before the label common_start. This
> now requires to have a slightly altered print for the boot CPU and set
> the early UART base address in each the two path (boot CPU and
> secondary CPUs).
> 
> This has the nice effect to remove a couple of conditional branch in
> the code.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

> ---
>  xen/arch/arm/arm64/head.S | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index a5147c8d80..fd432ee15d 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -265,6 +265,12 @@ real_start_efi:
>          load_paddr x21, _sdtb
>  #endif
>  
> +        /* Initialize the UART if earlyprintk has been enabled. */
> +#ifdef CONFIG_EARLY_PRINTK
> +        bl    init_uart
> +#endif
> +        PRINT("- Boot CPU booting -\r\n")
> +
>          mov   x22, #0                /* x22 := is_secondary_cpu */
>  
>          b     common_start
> @@ -281,14 +287,11 @@ GLOBAL(init_secondary)
>          /* Boot CPU already zero BSS so skip it on secondary CPUs. */
>          mov   x26, #1                /* X26 := skip_zero_bss */
>  
> -common_start:
>          mrs   x0, mpidr_el1
>          ldr   x13, =(~MPIDR_HWID_MASK)
>          bic   x24, x0, x13           /* Mask out flags to get CPU ID */
>  
> -        /* Non-boot CPUs wait here until __cpu_up is ready for them */
> -        cbz   x22, 1f
> -
> +        /* Wait here until __cpu_up is ready to handle the CPU */
>          load_paddr x0, smp_up_cpu
>          dsb   sy
>  2:      ldr   x1, [x0]
> @@ -300,14 +303,14 @@ common_start:
>  
>  #ifdef CONFIG_EARLY_PRINTK
>          ldr   x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */
> -        cbnz  x22, 1f
> -        bl    init_uart                 /* Boot CPU sets up the UART too */
> -1:      PRINT("- CPU ")
> +        PRINT("- CPU ")
>          mov   x0, x24
>          bl    putn
>          PRINT(" booting -\r\n")
>  #endif
>  
> +common_start:
> +
>          PRINT("- Current EL ")
>          mrs   x4, CurrentEL
>          mov   x0, x4
> @@ -628,10 +631,16 @@ ENTRY(switch_ttbr)
>          ret
>  
>  #ifdef CONFIG_EARLY_PRINTK
> -/* Bring up the UART.
> - * x23: Early UART base address
> - * Clobbers x0-x1 */
> +/*
> + * Initialize the UART. Should only be called on the boot CPU.
> + *
> + * Ouput:
> + *  x23: Early UART base physical address
> + *
> + * Clobbers x0 - x1
> + */
>  init_uart:
> +        ldr   x23, =EARLY_UART_BASE_ADDRESS
>  #ifdef EARLY_PRINTK_INIT_UART
>          early_uart_init x23, 0
>  #endif
> -- 
> 2.11.0
> 

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