| Message ID | 20190514122456.28559-7-julien.grall@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | xen/arm: Clean-up & fixes in boot/mm code | expand |
On Tue, 14 May 2019, 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. It is called "start_secondary" rather than "secondary_start". Please fix the commit message. Then you can add Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > Signed-off-by: Julien Grall <julien.grall@arm.com> > > - Re-order the patch with "xen/arm: Remove parameter cpuid from > start_xen". > --- > xen/arch/arm/arm32/head.S | 4 ++-- > xen/arch/arm/arm64/head.S | 3 ++- > xen/arch/arm/smpboot.c | 4 +--- > 3 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S > index cb8a3bf829..9f40face98 100644 > --- a/xen/arch/arm/arm32/head.S > +++ b/xen/arch/arm/arm32/head.S > @@ -445,9 +445,9 @@ 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 */ > teq r12, #0 > + moveq r0, r10 /* Marshal args: - phys_offset */ > + moveq r1, r8 /* - DTB address */ > 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 075013878e..cb30d6f22e 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -582,9 +582,10 @@ 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 */ > - 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; > > -- > 2.11.0 >
Hi, On 20/05/2019 23:56, Stefano Stabellini wrote: > On Tue, 14 May 2019, 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. > > It is called "start_secondary" rather than "secondary_start". Please fix > the commit message. Then you can add Whoops, I will update it. > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > >> Signed-off-by: Julien Grall <julien.grall@arm.com> I have also realized I forgot to add --- here. Thank you, >> >> - Re-order the patch with "xen/arm: Remove parameter cpuid from >> start_xen". >> --- >> xen/arch/arm/arm32/head.S | 4 ++-- >> xen/arch/arm/arm64/head.S | 3 ++- >> xen/arch/arm/smpboot.c | 4 +--- >> 3 files changed, 5 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S >> index cb8a3bf829..9f40face98 100644 >> --- a/xen/arch/arm/arm32/head.S >> +++ b/xen/arch/arm/arm32/head.S >> @@ -445,9 +445,9 @@ 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 */ >> teq r12, #0 >> + moveq r0, r10 /* Marshal args: - phys_offset */ >> + moveq r1, r8 /* - DTB address */ >> 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 075013878e..cb30d6f22e 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -582,9 +582,10 @@ 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 */ >> - 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; >> >> -- >> 2.11.0 >>
On 29.05.19 20:06, Julien Grall wrote: > Hi, > > On 20/05/2019 23:56, Stefano Stabellini wrote: >> On Tue, 14 May 2019, 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. >> >> It is called "start_secondary" rather than "secondary_start". Please fix >> the commit message. Then you can add > > Whoops, I will update it. > >> >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> >> >> >>> Signed-off-by: Julien Grall <julien.grall@arm.com> FWIW, with the name fixed Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index cb8a3bf829..9f40face98 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -445,9 +445,9 @@ 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 */ teq r12, #0 + moveq r0, r10 /* Marshal args: - phys_offset */ + moveq r1, r8 /* - DTB address */ 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 075013878e..cb30d6f22e 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -582,9 +582,10 @@ 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 */ - 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;
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> - Re-order the patch with "xen/arm: Remove parameter cpuid from start_xen". --- xen/arch/arm/arm32/head.S | 4 ++-- xen/arch/arm/arm64/head.S | 3 ++- xen/arch/arm/smpboot.c | 4 +--- 3 files changed, 5 insertions(+), 6 deletions(-)