Hi Stefano,
On 6/26/19 8:12 PM, Stefano Stabellini wrote:
> 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@arm.com>
>> ---
>> 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).
Sure.
>
>
>> + */
>> 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?
I think a remnant of early rework when start_secondary was taking 3
parameters. I will update it.
>
>
>> + 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)?
Sure.
>
>
>> + sub x4, x4, #CPUINFO_sizeof /* Make room for CPU save record */
>> + mov sp, x4
>> +
>> + /* Jump to C world */
>> + br x2
>> ENDPROC(launch)
>>
>> /* Fail-stop */
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel