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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 10 Jun 2019, Julien Grall wrote:
> Boot CPU and secondary CPUs will use different entry point to C code. At
> the moment, the decision on which entry to use is taken within launch().
> 
> In order to avoid a branch for the decision and make the code clearer,
> launch() is reworked to take in parameters the entry point and its
> arguments.
> 
> 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 | 41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 4f7fa6769f..130ab66d8e 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -312,6 +312,11 @@ primary_switched:
>          /* Use a virtual address to access the UART. */
>          ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
>  #endif
> +        PRINT("- Ready -\r\n")
> +        /* Setup the arguments for start_xen and jump to C world */
> +        mov   x0, x20                /* x0 := phys_offset */
> +        mov   x1, x21                /* x1 := paddr(FDT) */
> +        ldr   x2, =start_xen
>          b     launch
>  ENDPROC(real_start)
>  
> @@ -374,6 +379,9 @@ secondary_switched:
>          /* Use a virtual address to access the UART. */
>          ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
>  #endif
> +        PRINT("- Ready -\r\n")
> +        /* Jump to C world */
> +        ldr   x2, =start_secondary
>          b     launch
>  ENDPROC(init_secondary)
>  
> @@ -734,23 +742,24 @@ setup_fixmap:
>          ret
>  ENDPROC(setup_fixmap)
>  
> +/*
> + * Setup the initial stack and jump to the C world
> + *
> + * Inputs:
> + *   x0 : Argument 0 of the C function to call
> + *   x1 : Argument 1 of the C function to call
> + *   x2 : C entry point

I know it is the last one before C-land, but we might as well add a
"Clobbers" section too, just in case. Here it clobbers x4 (or x3, see
below).


> + */
>  launch:
> -        PRINT("- Ready -\r\n")
> -
> -        ldr   x0, =init_data
> -        add   x0, x0, #INITINFO_stack /* Find the boot-time stack */
> -        ldr   x0, [x0]
> -        add   x0, x0, #STACK_SIZE    /* (which grows down from the top). */
> -        sub   x0, x0, #CPUINFO_sizeof /* Make room for CPU save record */
> -        mov   sp, x0
> -
> -        cbnz  x22, 1f
> -
> -        mov   x0, x20                /* Marshal args: - phys_offset */
> -        mov   x1, x21                /*               - FDT */
> -        b     start_xen              /* and disappear into the land of C */
> -1:
> -        b     start_secondary        /* (to the appropriate entry point) */
> +        ldr   x4, =init_data

why not use x3 instead of x4?


> +        add   x4, x4, #INITINFO_stack /* Find the boot-time stack */
> +        ldr   x4, [x4]
> +        add   x4, x4, #STACK_SIZE    /* (which grows down from the top). */

If you are going to respin it, could you please align the comments a bit
better (one space to the right)?


> +        sub   x4, x4, #CPUINFO_sizeof /* Make room for CPU save record */
> +        mov   sp, x4
> +
> +        /* Jump to C world */
> +        br    x2
>  ENDPROC(launch)
>  
>  /* Fail-stop */

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