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

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@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

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@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) ?

From: Ang, Chee Hong chee.hong.ang@intel.com Sent: Monday, April 8, 2019 05:10 To: Lars Povlsen - M31675 Lars.Povlsen@microchip.com; trini@konsulko.com; u-boot@lists.denx.de; macro.wave.z@gmail.com; albert.u.boot@aribaud.net; york.sun@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@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

On Mon, 2019-04-08 at 07:27 +0000, Lars.Povlsen@microchip.com wrote:
From: Ang, Chee Hong chee.hong.ang@intel.com Sent: Monday, April 8, 2019 05:10 To: Lars Povlsen - M31675 Lars.Povlsen@microchip.com; trini@konsulko.com; u-boot@lists.denx.de; macro.wave.z@gmail.com; albert.u.boot@aribaud.net; york.sun@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@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.

From: Ang, Chee Hong chee.hong.ang@intel.com Sent: Monday, April 8, 2019 11:00 To: Lars Povlsen - M31675 Lars.Povlsen@microchip.com; trini@konsulko.com; u-boot@lists.denx.de; macro.wave.z@gmail.com; albert.u.boot@aribaud.net; york.sun@nxp.com Subject: Re: [RESEND PATCH] ARMv8: PSCI: Fix PSCI_TABLE relocation issue
On Mon, 2019-04-08 at 07:27 +0000, Lars.Povlsen@microchip.com wrote:
From: Ang, Chee Hong chee.hong.ang@intel.com Sent: Monday, April 8, 2019 05:10 To: Lars Povlsen - M31675 Lars.Povlsen@microchip.com; trini@konsulko.com; u-boot@lists.denx.de; macro.wave.z@gmail.com; albert.u.boot@aribaud.net; york.sun@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@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

On Thu, Apr 04, 2019 at 02:38:50PM +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@microchip.com
Applied to u-boot/master, thanks!
participants (4)
-
Ang, Chee Hong
-
Lars Povlsen
-
Lars.Povlsen@microchip.com
-
Tom Rini