[Xen-devel,05/20] xen/arm: Rework secondary_start prototype

Message ID 20190422164937.21350-6-julien.grall@arm.com
State Superseded
Headers show
Series
  • xen/arm: Clean-up & fixes in boot/mm code
Related show

Commit Message

Julien Grall April 22, 2019, 4:49 p.m.
None of the parameters of secondary_start are actually used. So turn
secondary_start to a function with no parameters.

Also modify the assembly code to avoid setting-up the registers before
calling secondary_start.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm32/head.S | 6 +++---
 xen/arch/arm/arm64/head.S | 3 ++-
 xen/arch/arm/smpboot.c    | 4 +---
 3 files changed, 6 insertions(+), 7 deletions(-)

Comments

Andrii Anisov May 3, 2019, 3:56 p.m. | #1
Hello Julien,

On 22.04.19 19:49, Julien Grall wrote:
> None of the parameters of secondary_start are actually used. So turn
> secondary_start to a function with no parameters.
> 
> Also modify the assembly code to avoid setting-up the registers before
> calling secondary_start.

What is not really mandatory.

> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Though:

Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
Julien Grall May 3, 2019, 4:15 p.m. | #2
On 03/05/2019 16:56, Andrii Anisov wrote:
> Hello Julien,
> 
> On 22.04.19 19:49, Julien Grall wrote:
>> None of the parameters of secondary_start are actually used. So turn
>> secondary_start to a function with no parameters.
>>
>> Also modify the assembly code to avoid setting-up the registers before
>> calling secondary_start.
> 
> What is not really mandatory.

So...? You just don't setup parameter when it is not necessary. The more it is 
quite confusing for a reader to see the registers are setup but not used by the 
caller.

> 
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> Though:
> 
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

Thank you.

Cheers,

Patch

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 8a98607459..b71d7fb11d 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -445,10 +445,10 @@  launch:
         ldr   sp, [r0]
         add   sp, #STACK_SIZE        /* (which grows down from the top). */
         sub   sp, #CPUINFO_sizeof    /* Make room for CPU save record */
-        mov   r0, r10                /* Marshal args: - phys_offset */
-        mov   r1, r8                 /*               - DTB address */
-        mov   r2, r7                 /*               - CPU ID */
         teq   r12, #0
+        moveq r0, r10                /* Marshal args: - phys_offset */
+        moveq r1, r8                 /*               - DTB address */
+        moveq r2, r7                 /*               - CPU ID */
         beq   start_xen              /* and disappear into the land of C */
         b     start_secondary        /* (to the appropriate entry point) */
 
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 4fe904c51d..b26126de53 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -582,10 +582,11 @@  launch:
         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 */
         mov   x2, x24                /*               - CPU ID */
-        cbnz  x22, 1f
         b     start_xen              /* and disappear into the land of C */
 1:
         b     start_secondary        /* (to the appropriate entry point) */
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index f756444362..00b64c3322 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -297,9 +297,7 @@  smp_prepare_cpus(void)
 }
 
 /* Boot the current CPU */
-void start_secondary(unsigned long boot_phys_offset,
-                     unsigned long fdt_paddr,
-                     unsigned long hwid)
+void start_secondary(void)
 {
     unsigned int cpuid = init_data.cpuid;