Re: [Xen-devel] [PATCH v2 22/35] xen/arm32: head: Rework UART initialization on boot CPU

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 22 Jul 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.
> 
> After this rework, the CPUID is only used at the very beginning of the
> secondary CPUs boot path. So there is no need to "reserve" x24 for the
> CPUID.
> 
> Lastly, take the opportunity to replace load from literal pool with the
> new macro mov_w.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> 
> ---
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/arm32/head.S | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index b54331c19d..134c3dda92 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -54,7 +54,7 @@
>   *   r4  -
>   *   r5  -
>   *   r6  - identity map in place
> - *   r7  - CPUID
> + *   r7  -
>   *   r8  - DTB address (boot CPU only)
>   *   r9  - paddr(start)
>   *   r10 - phys offset
> @@ -123,6 +123,12 @@ past_zImage:
>          add   r8, r10                /* r8 := paddr(DTB) */
>  #endif
>  
> +        /* Initialize the UART if earlyprintk has been enabled. */
> +#ifdef CONFIG_EARLY_PRINTK
> +        bl    init_uart
> +#endif
> +        PRINT("- Boot CPU booting -\r\n")
> +
>          mov   r12, #0                /* r12 := is_secondary_cpu */
>  
>          b     common_start
> @@ -137,14 +143,9 @@ GLOBAL(init_secondary)
>  
>          mov   r12, #1                /* r12 := is_secondary_cpu */
>  
> -common_start:
>          mrc   CP32(r1, MPIDR)
>          bic   r7, r1, #(~MPIDR_HWID_MASK) /* Mask out flags to get CPU ID */
>  
> -        /* Non-boot CPUs wait here until __cpu_up is ready for them */
> -        teq   r12, #0
> -        beq   1f
> -
>          ldr   r0, =smp_up_cpu
>          add   r0, r0, r10            /* Apply physical offset */
>          dsb
> @@ -156,15 +157,14 @@ common_start:
>  1:
>  
>  #ifdef CONFIG_EARLY_PRINTK
> -        ldr   r11, =EARLY_UART_BASE_ADDRESS  /* r11 := UART base address */
> -        teq   r12, #0                /* Boot CPU sets up the UART too */
> -        bleq  init_uart
> +        mov_w r11, EARLY_UART_BASE_ADDRESS   /* r11 := UART base address */
>          PRINT("- CPU ")
>          mov   r0, r7
>          bl    putn
>          PRINT(" booting -\r\n")
>  #endif
>  
> +common_start:
>          /* Check that this CPU has Hyp mode */
>          mrc   CP32(r0, ID_PFR1)
>          and   r0, r0, #0xf000        /* Bits 12-15 define virt extensions */
> @@ -497,11 +497,15 @@ ENTRY(switch_ttbr)
>  
>  #ifdef CONFIG_EARLY_PRINTK
>  /*
> - * Bring up the UART.
> - * r11: Early UART base address
> - * Clobbers r0-r2
> + * Initialize the UART. Should only be called on the boot CPU.
> + *
> + * Ouput:
      ^ this should be output, and in the arm64 patch too (already committed)

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>



> + *  r11: Early UART base physical address
> + *
> + * Clobbers r0 - r3
>   */
>  init_uart:
> +        mov_w r11, EARLY_UART_BASE_ADDRESS
>  #ifdef EARLY_PRINTK_INIT_UART
>          early_uart_init r11, r1, r2
>  #endif


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