[U-Boot] [RESEND PATCH] ARMv8: PSCI: Fix PSCI_TABLE relocation issue

[Lars.Povlsen at microchip.com]

> From: Ang, Chee Hong <chee.hong.ang at intel.com>
> Sent: Monday, April 8, 2019 11:00
> To: Lars Povlsen - M31675 <Lars.Povlsen at microchip.com>;
> trini at konsulko.com; u-boot at lists.denx.de; macro.wave.z at gmail.com;
> albert.u.boot at aribaud.net; york.sun at nxp.com
> Subject: Re: [RESEND PATCH] ARMv8: PSCI: Fix PSCI_TABLE relocation issue
> 
> On Mon, 2019-04-08 at 07:27 +0000, Lars.Povlsen at microchip.com wrote:
> > >
> > > From: Ang, Chee Hong <chee.hong.ang at intel.com>
> > > Sent: Monday, April 8, 2019 05:10
> > > To: Lars Povlsen - M31675 <Lars.Povlsen at microchip.com>;
> > > trini at konsulko.com; u-boot at lists.denx.de; macro.wave.z at gmail.com;
> > > albert.u.boot at aribaud.net; york.sun at nxp.com
> > > Subject: Re: [RESEND PATCH] ARMv8: PSCI: Fix PSCI_TABLE relocation
> > > issue
> > >
> > > On Thu, 2019-04-04 at 14:38 +0200, Lars Povlsen wrote:
> > > >
> > > > This fixes relaction isses with the PSCI_TABLE entries in
> > > > the psci_32_table and psci_64_table.
> > > >
> > > > When using 32-bit adress pointers relocation was not being
> > > > applied to
> > > > the tables, causing PSCI handlers to point to the un-relocated
> > > > code
> > > > area. By using 64-bit data relocation is properly applied. The
> > > > handlers are thus in the "secure data" area, which is protected
> > > > by
> > > > /memreserve/ in the FDT.
> > > >
> > > > Signed-off-by: Lars Povlsen <lars.povlsen at microchip.com>
> > > > ---
> > > >  arch/arm/cpu/armv8/psci.S | 13 ++++++-------
> > > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/arch/arm/cpu/armv8/psci.S
> > > > b/arch/arm/cpu/armv8/psci.S
> > > > index 358df8fee9..b4568cf053 100644
> > > > --- a/arch/arm/cpu/armv8/psci.S
> > > > +++ b/arch/arm/cpu/armv8/psci.S
> > > > @@ -19,8 +19,8 @@
> > > >
> > > >  /* PSCI function and ID table definition*/
> > > >  #define PSCI_TABLE(__id, __fn) \
> > > > -	.word __id; \
> > > > -	.word __fn
> > > > +	.quad __id; \
> > > > +	.quad __fn
> > > >
> > > >  .pushsection ._secure.text, "ax"
> > > >
> > > > @@ -132,16 +132,15 @@ PSCI_TABLE(0, 0)
> > > >  /* Caller must put PSCI function-ID table base in x9 */
> > > >  handle_psci:
> > > >  	psci_enter
> > > > -1:	ldr x10, [x9]			/* Load PSCI
> > > > function
> > > > table */
> > > > -	ubfx x11, x10, #32, #32
> > > > -	ubfx x10, x10, #0, #32
> > > > +1:	ldr	x10, [x9]		/* Load PSCI
> > > > function
> > > > table */
> > > >  	cbz	x10, 3f			/* If reach
> > > > the
> > > > end, bail out */
> > > >  	cmp	x10, x0
> > > >  	b.eq	2f			/* PSCI function
> > > > found
> > > > */
> > > > -	add x9, x9, #8			/* If not match,
> > > > try
> > > > next entry */
> > > > +	add x9, x9, #16			/* If not match,
> > > > try
> > > > next entry */
> > > >  	b	1b
> > > >
> > > > -2:	blr	x11			/* Call PSCI
> > > > function */
> > > > +2:	ldr	x11, [x9, #8]		/* Load PSCI
> > > > function */
> > > > +	blr	x11			/* Call PSCI
> > > > function
> > > > */
> > > >  	psci_return
> > > >
> > > >  3:	mov	x0, #ARM_PSCI_RET_NI
> > > Hmmm...I presumed you made these changes because your relocation
> > > address for "secure" section is beyond 32bits (4GB). How your page
> > > table for MMU is being setup ? Why do you need such large address
> > > space
> > > (beyond 4GB) ?
> > No, as I tried to describe in the log message, the relocation was
> > simply
> > not being applied. Changing the offsets to 64-bit fixed this. The
> > .text base
> > was 0x80000000 and the relocation was done in a 2Gb window (so
> > somewhere ~ 0xfff.....)
> >
> > Never the less, I assume a 64-bit target should not be constrained to
> > 32-bit addresses?
> >
> > When debugging the code, I noticed my debugger was able to match the
> > original symbols
> > even though relocation was done. That’s how I became aware. I could
> > see that the vector table
> > and the first level code *was* relocated, but the individual handlers
> > not.
> > ---Lars
> CONFIG_ARMV8_SECURE_BASE defines the target relocation address for the
> PSCI code and the table. I just checked the PSCI handler addresses in
> the PSCI table entry (_psci_32_table & _psci_64_table) of my uboot,
> they point to the relocated address without using the 64bits size.

CONFIG_ARMV8_SECURE_BASE depend on SYS_HAS_ARMV8_SECURE_BASE, which is
not set on this platform. Might this be the source of the problem?
In this case, I would assume that the "secure data" just stays with the
relocated U-boot, and gets carved out with a fdt_add_mem_rsv when
booting Linux.

For what it matters, I have changed to using the spintable PSCI variant,
so I'm not really depending on this patch. But it looks like there is a
problem, at least in the configuration I used.


---Lars