Hi Tom,

Thanks for your review.

> On Thu, Dec 10, 2015 at 10:49:01AM +0800, Dongsheng Wang wrote:
> 
> > From: Wang Dongsheng <dongsheng.w...@nxp.com>
> >
> > Fix PSCI hang up without CONFIG_ARMV7_SECURE_BASE define.
> > "DISCARD" will remove ._secure.text relocate, but PSCI framework has
> > already used some absolute address those need to relocate.
> >
> > Use readelf -t -r u-boot show us:
> > .__secure_start             addr: 601408e4
> > .__secure_end               addr: 60141460
> >
> > 60141140  00000017 R_ARM_RELATIVE
> > 46  _secure_monitor:
> > 47  #ifdef CONFIG_ARMV7_PSCI
> > 48      ldr     r5, =_psci_vectors
> >
> > 60141194  00000017 R_ARM_RELATIVE
> > 6014119c  00000017 R_ARM_RELATIVE
> > 601411a4  00000017 R_ARM_RELATIVE
> > 601411ac  00000017 R_ARM_RELATIVE
> > 64  _psci_table:
> > 66  .word   psci_cpu_suspend
> > ...
> > 72  .word   psci_migrate
> >
> > 60141344  00000017 R_ARM_RELATIVE
> > 6014145c  00000017 R_ARM_RELATIVE
> > 202 ldr     r5, =psci_text_end
> >
> > Solutions:
> > 1. Change absolute address to RelAdr.
> >    Based on LDR (immediate, ARM), we only have 4K offset to jump.
> > Now PSCI code size is close to 4K size that is LDR limit jump size, so
> > even if the LDR is based on the current instruction address, there is
> > also have a risk for RelAdr. If we use two jump steps I think we can
> > fix this issue, but looks too hack, so give up this way.
> >
> > 2. Enable "DISCARD" only for CONFIG_ARMV7_SECURE_BASE has defined.
> >    If CONFIG_ARMV7_SECURE_BASE is defined in platform, all of secure
> > will in the BASE address that is absolute. psci_update_dt will
> > relocate the PSCI code into link stage
> > address(CONFIG_ARMV7_SECURE_BASE address), so using this method.
> >
> > Signed-off-by: Wang Dongsheng <dongsheng.w...@nxp.com>
> > Reviewed-by: Peng Fan <peng....@nxp.com>
> > Cc: Albert Aribaud <albert.u.b...@aribaud.net>
> > Cc: Tom Warren <twar...@nvidia.com>
> > Cc: York Sun <york...@freescale.com>
> > Cc: Hans De Goede <hdego...@redhat.com>
> > Cc: Ian Campbell <i...@hellion.org.uk>
> > Cc: Tom Rini <tr...@konsulko.com>
> > Cc: Jan Kiszka <jan.kis...@siemens.com>
> > Cc: Stefano Babic <sba...@denx.de>
> >
> > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index
> > d48a905..413d459 100644
> > --- a/arch/arm/cpu/u-boot.lds
> > +++ b/arch/arm/cpu/u-boot.lds
> > @@ -14,6 +14,7 @@ OUTPUT_ARCH(arm)
> >  ENTRY(_start)
> >  SECTIONS
> >  {
> > +#if defined(CONFIG_ARMV7_SECURE_BASE) &&
> defined(CONFIG_ARMV7_NONSEC)
> >     /*
> >      * Discard the relocation entries for secure text.
> >      * The secure code is bundled with u-boot image, so there will @@
> > -31,6 +32,7 @@ SECTIONS
> >      * avoid hole in the final image.
> >      */

Update this comment, not my patch's comment, right?

Regards,
-Dongsheng

> >     /DISCARD/ : { *(.rel._secure*) }
> > +#endif
> >     . = 0x00000000;
> >
> >     . = ALIGN(4);
> 
> Please update the big comment that you're now #if'ing around to explain why
> the statement isn't true in some cases and hence the test, thanks!
> 
> --
> Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to