Message ID | 1474867290-17726-1-git-send-email-yamada.masahiro@socionext.com |
---|---|
State | Accepted |
Commit | 2fe1281c79cb8232cf0ca1c441db66e576a9acf4 |
Delegated to: | Tom Rini |
Headers | show |
On Mon, Sep 26, 2016 at 1:21 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Jon Master reports that QEMU refuses to load a U-Boot image built > with CONFIG_ARMV7_NONSEC, but without CONFIG_ARMV7_PSCI since > commit 5a3aae68c74e ("ARM: armv7: guard memory reserve for PSCI > with #ifdef CONFIG_ARMV7_PSCI"). > > It looks like only PSCI that needs the Secure stack, so move > the #ifdef to guard the whole of .secure_stack allocation in order > not to create the empty section. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > Reported-by: Jon Masters <jcm@redhat.com> > Link: http://patchwork.ozlabs.org/patch/664025/ > --- > > With this commit, the SECURE_MAX_SIZE check will go inside > the #ifdef CONFIG_ARMV7_PSCI, so this patch is probably wrong. I wonder if you could move the SECURE_MAX_SIZE check outside of the secure_stack section. I might have put it where it is because of some issues, but I can't remember. > I am CCing Chen-Yu Tsai. He mostly expanded this linker script > for PSCI work, so I hope he can suggest the correct way > for fixing this problem. The patch looks good. Though I wonder if you need to guard the __secure_stack_start and __secure_stack_end symbols in arch/arm/lib/sections.c as well. Otherwise they might end up in the data section? Regards ChenYu > > arch/arm/cpu/u-boot.lds | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > index 0a5fae6..37d4c60 100644 > --- a/arch/arm/cpu/u-boot.lds > +++ b/arch/arm/cpu/u-boot.lds > @@ -74,6 +74,7 @@ SECTIONS > *(._secure.data) > } > > +#ifdef CONFIG_ARMV7_PSCI > .secure_stack ALIGN(ADDR(.secure_data) + SIZEOF(.secure_data), > CONSTANT(COMMONPAGESIZE)) (NOLOAD) : > #ifdef __ARMV7_PSCI_STACK_IN_RAM > @@ -83,10 +84,10 @@ SECTIONS > #endif > { > KEEP(*(.__secure_stack_start)) > -#ifdef CONFIG_ARMV7_PSCI > + > /* Skip addreses for stack */ > . = . + CONFIG_ARMV7_PSCI_NR_CPUS * ARM_PSCI_STACK_SIZE; > -#endif > + > /* Align end of stack section to page boundary */ > . = ALIGN(CONSTANT(COMMONPAGESIZE)); > > @@ -109,6 +110,8 @@ SECTIONS > . = LOADADDR(.secure_stack); > #endif > > +#endif > + > .__secure_end : AT(ADDR(.__secure_end)) { > *(.__secure_end) > LONG(0x1d1071c); /* Must output something to reset LMA */ > -- > 1.9.1 >
On Mon, Sep 26, 2016 at 02:21:30PM +0900, Masahiro Yamada wrote: > Jon Master reports that QEMU refuses to load a U-Boot image built > with CONFIG_ARMV7_NONSEC, but without CONFIG_ARMV7_PSCI since > commit 5a3aae68c74e ("ARM: armv7: guard memory reserve for PSCI > with #ifdef CONFIG_ARMV7_PSCI"). > > It looks like only PSCI that needs the Secure stack, so move > the #ifdef to guard the whole of .secure_stack allocation in order > not to create the empty section. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > Reported-by: Jon Masters <jcm@redhat.com> > Link: http://patchwork.ozlabs.org/patch/664025/ We're close I think, but this breaks boards like ls1021aqds_nor_lpuart
On Mon, Sep 26, 2016 at 02:21:30PM +0900, Masahiro Yamada wrote: > Jon Master reports that QEMU refuses to load a U-Boot image built > with CONFIG_ARMV7_NONSEC, but without CONFIG_ARMV7_PSCI since > commit 5a3aae68c74e ("ARM: armv7: guard memory reserve for PSCI > with #ifdef CONFIG_ARMV7_PSCI"). > > It looks like only PSCI that needs the Secure stack, so move > the #ifdef to guard the whole of .secure_stack allocation in order > not to create the empty section. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > Reported-by: Jon Masters <jcm@redhat.com> > Link: http://patchwork.ozlabs.org/patch/664025/ Applied to u-boot/master, thanks! ... and no, I don't know how / why this caused build failures before, but it did and I replicated it at the time too. But I just got a clean build everywhere. And I've added vexpress-a15 and vexpress-a9 via QEMU to my test setup now.
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 0a5fae6..37d4c60 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -74,6 +74,7 @@ SECTIONS *(._secure.data) } +#ifdef CONFIG_ARMV7_PSCI .secure_stack ALIGN(ADDR(.secure_data) + SIZEOF(.secure_data), CONSTANT(COMMONPAGESIZE)) (NOLOAD) : #ifdef __ARMV7_PSCI_STACK_IN_RAM @@ -83,10 +84,10 @@ SECTIONS #endif { KEEP(*(.__secure_stack_start)) -#ifdef CONFIG_ARMV7_PSCI + /* Skip addreses for stack */ . = . + CONFIG_ARMV7_PSCI_NR_CPUS * ARM_PSCI_STACK_SIZE; -#endif + /* Align end of stack section to page boundary */ . = ALIGN(CONSTANT(COMMONPAGESIZE)); @@ -109,6 +110,8 @@ SECTIONS . = LOADADDR(.secure_stack); #endif +#endif + .__secure_end : AT(ADDR(.__secure_end)) { *(.__secure_end) LONG(0x1d1071c); /* Must output something to reset LMA */
Jon Master reports that QEMU refuses to load a U-Boot image built with CONFIG_ARMV7_NONSEC, but without CONFIG_ARMV7_PSCI since commit 5a3aae68c74e ("ARM: armv7: guard memory reserve for PSCI with #ifdef CONFIG_ARMV7_PSCI"). It looks like only PSCI that needs the Secure stack, so move the #ifdef to guard the whole of .secure_stack allocation in order not to create the empty section. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Reported-by: Jon Masters <jcm@redhat.com> Link: http://patchwork.ozlabs.org/patch/664025/ --- With this commit, the SECURE_MAX_SIZE check will go inside the #ifdef CONFIG_ARMV7_PSCI, so this patch is probably wrong. I am CCing Chen-Yu Tsai. He mostly expanded this linker script for PSCI work, so I hope he can suggest the correct way for fixing this problem. arch/arm/cpu/u-boot.lds | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)