arm: discard relocation entry for secure section
PATCH v2 1/3 arm: discard relocation entry for secure section
The code such as PSCI in section named secure is bundled with u-boot image, and when bootm, the code will be copied to their runtime address same to compliation/linking address - CONFIG_ARMV7_SECURE_BASE.
When compile the PSCI code and link it into the u-boot image, there will be relocation entries in .rel.dyn section for PSCI. Actually, we do not needs these relocation entries.
If still keep the relocation entries in .rel.dyn section, r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid address which may not support read/write for one SoC.
102 /* relative fix: increase location by offset */ 103 add r0, r0, r4 104 ldr r1, [r0] 105 add r1, r1, r4 106 str r1, [r0]
So discard the relocation entries for code in secure section.
---
V1 thread: http://lists.denx.de/pipermail/u-boot/2015-October/229426.html
Changes V2:
Refine commit msg.
Discard the relocation entry section for secure text.
arch/arm/cpu/u-boot.lds | 2 ++ 1 file changed, 2 insertions(+)
arch/arm/cpu/u-boot.lds +2 -0
@@ -51,6 +51,8 @@ SECTIONS *(.__secure_end) LONG(0x1d1071c); /* Must output something to reset LMA */ } + + /DISCARD/ : { *(.rel._secure*) } #endif . = ALIGN(4);
Signature · 1 line
1.8.4
PATCH v2 2/3 mx7: psci: add basic psci support
1. add basic psci support for imx7 chip.
2. support cpu_on and cpu_off.
3. switch to non-secure mode when boot linux kernel.
4. set csu allow accessing all peripherial register in non-secure mode.
---
Changes V2:
Refine commit msg.
arch/arm/cpu/armv7/mx7/Makefile | 4 ++ arch/arm/cpu/armv7/mx7/psci-mx7.c | 78 +++++++++++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/psci.S | 54 +++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/soc.c | 9 +++++ 4 files changed, 145 insertions(+) create mode 100644 arch/arm/cpu/armv7/mx7/psci-mx7.c create mode 100644 arch/arm/cpu/armv7/mx7/psci.S
arch/arm/cpu/armv7/mx7/Makefile +4 -0
@@ -6,3 +6,7 @@ # obj-y := soc.o clock.o clock_slice.o + +ifdef CONFIG_ARMV7_PSCI +obj-y += psci.o psci-mx7.o +endif
arch/arm/cpu/armv7/mx7/psci-mx7.c new file +78 -0
78 lines not on this page · raw message
arch/arm/cpu/armv7/mx7/psci.S new file +54 -0
54 lines not on this page · raw message
arch/arm/cpu/armv7/mx7/soc.c +9 -0
@@ -114,10 +114,19 @@ u32 __weak get_board_rev(void) }#endif +/* enable all periherial can be access in nosec mode */ +static void init_csu (void ) +{ +int i =0 ; +for (i =0 ; i <64 ; i++) +writel (0x00FF00FF ,0x303e0000 + i *4 ); +} +int arch_cpu_init (void ) {init_aips (); +init_csu ();/* Disable PDE bit of WMCR register */ imx_set_wdog_powerdown (false);
Signature · 1 line
1.8.4
PATCH v2 2/3 mx7: psci: add basic psci support
arch/arm/cpu/armv7/mx7/psci-mx7.c line 29
Quoted 9 lines · Peng Fan
arch/arm/cpu/armv7/mx7/psci-mx7.c new file +9
-0+static inline void psci_writel (u32 value,u32 reg) +{ + *(volatile u32 *)reg = value; +} + +static inline int psci_readl (u32 reg) +{ +return *(volatile u32 *)reg; +}
Do you really need psci_writel() psci_readl() functions? Why not simple call writel and readl instead?
arch/arm/cpu/armv7/mx7/soc.c u32 __weak get_board_rev(void) · line 122
Quoted 6 lines
arch/arm/cpu/armv7/mx7/soc.c +6
-0+/* enable all periherial can be access in nosec mode */ +static void init_csu (void ) +{ +int i =0 ; +for (i =0 ; i <64 ; i++) +writel (0x00FF00FF ,0x303e0000 + i *4 );
Lots of magic values here. Please add defines for improving code readability.
Thanks
PATCH v2 2/3 mx7: psci: add basic psci support
In reply to Peng FanQuoted 39 lines
-----Original Message-----
From: Peng Fan [mailto:Peng.Fan at freescale.com]
Sent: Tuesday, October 20, 2015 1:00 AM
To: u-boot at lists.denx.de
Cc: Fan Peng-B51431 <[email protected]>; Li Frank-B20596 <[email protected]>; Stefano Babic <[email protected]>; Estevam Fabio- R49496 <[email protected]>
Subject: [PATCH V2 2/3] mx7: psci: add basic psci support1. add basic psci support for imx7 chip.
2. supportcpu_onandcpu_off.
3. switch to non-secure mode when boot linux kernel.
4. set csu allow accessing all peripherial register in non-secure mode.Signed-off-by Frank Li <[email protected]>Signed-off-by Peng Fan <[email protected]>Cc Stefano Babic <[email protected]>Cc Fabio Estevam <[email protected]>---
Changes V2:
Refine commit msg.arch/arm/cpu/armv7/mx7/Makefile | 4 ++ arch/arm/cpu/armv7/mx7/psci-mx7.c | 78 +++++++++++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/psci.S | 54 +++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/soc.c | 9 +++++ 4 files changed, 145 insertions(+) create mode 100644 arch/arm/cpu/armv7/mx7/psci-mx7.c create mode100644
arch/arm/cpu/armv7/mx7/psci.S(quoted diff) +0
-0b/arch/arm/cpu/armv7/mx7/Makefile index e6ecef0..f25461c 100644
arch/arm/cpu/armv7/mx7/Makefile line 11
Quoted 6 lines
arch/arm/cpu/armv7/mx7/Makefile +3
-0@@ -6,3 +6,7 @@ # obj-y := soc.o clock.o clock_slice.o + +ifdef CONFIG_ARMV7_PSCI +obj-y += psci.o psci-mx7.o
Obj-y += psci-mx7.o psci.o
The otherwise psci_text_end will not be last one.
Best regards
Frank Li
Quoted 160 lines
arch/arm/cpu/armv7/mx7/Makefile +1
-0+endifarch/arm/cpu/armv7/mx7/psci-mx7.c new file +34
-0@@ -0,0 +1,78 @@ +#include <common.h> +#include <asm/psci.h> +#include <asm/arch/imx-regs.h> + +#define __secure __attribute__((section("._secure.text"))) + +#define GPC_CPU_PGC_SW_PDN_REQ 0xfc +#define GPC_CPU_PGC_SW_PUP_REQ 0xf0 +#define GPC_PGC_C1 0x840 + +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7 0x2 + +/* below is for i.MX7D */ +#define SRC_GPR1_MX7D 0x074 +#define SRC_A7RCR0 0x004 +#define SRC_A7RCR1 0x008 + +#define BP_SRC_A7RCR0_A7_CORE_RESET0 0 +#define BP_SRC_A7RCR1_A7_CORE1_ENABLE 1 + +static inline void psci_writel (u32 value,u32 reg) { + *(volatile u32 *)reg = value; +} + +static inline int psci_readl (u32 reg) +{ +return *(volatile u32 *)reg; +} + +static inline void imx_gpcv2_set_m_core_pgc (bool enable,u32 offset) { +psci_writel (enable,GPC_IPS_BASE_ADDR + offset); } + +__secure void imx_gpcv2_set_core1_power(bool pdn) { + u32 reg = pdn ? GPC_CPU_PGC_SW_PUP_REQ :
GPC_CPU_PGC_SW_PDN_REQ;arch/arm/cpu/armv7/mx7/psci-mx7.c new file +36
-0+u32 val; + +imx_gpcv2_set_m_core_pgc (true,GPC_PGC_C1 ); + + val =psci_readl (GPC_IPS_BASE_ADDR + reg); + val |=BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7 ; +psci_writel (val,GPC_IPS_BASE_ADDR + reg); + +while ((psci_readl (GPC_IPS_BASE_ADDR + reg) & +BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7 ) !=0 ) + ; + +imx_gpcv2_set_m_core_pgc (false,GPC_PGC_C1 ); } + +__secure void imx_enable_cpu_ca7(int cpu, bool enable) { +u32 mask, val; + + mask =1 << (BP_SRC_A7RCR1_A7_CORE1_ENABLE + cpu -1 ); + val =psci_readl (SRC_BASE_ADDR +SRC_A7RCR1 ); + val = enable ? val | mask : val & ~mask; +psci_writel (val,SRC_BASE_ADDR +SRC_A7RCR1 ); } + +__secure int imx_cpu_on(int fn, int cpu, int pc) { +psci_writel (pc,SRC_BASE_ADDR + cpu *8 +SRC_GPR1_MX7D ); +imx_gpcv2_set_core1_power (true); +imx_enable_cpu_ca7 (cpu, true); +return 0 ; +} + +__secure int imx_cpu_off(int cpu) +{ +imx_enable_cpu_ca7 (cpu, false); +imx_gpcv2_set_core1_power (false); +psci_writel (0 ,SRC_BASE_ADDR + cpu *8 +SRC_GPR1_MX7D +4 ); +return 0 ; +}arch/arm/cpu/armv7/mx7/psci.S new file +53
-0@@ -0,0 +1,54 @@ +#include <config.h> +#include <linux/linkage.h> + +#include <asm/armv7.h> +#include <asm/arch-armv7/generictimer.h> #include <asm/psci.h> + + .pushsection ._secure.text, "ax" + + .arch_extension sec + + @ r1 = target CPU + @ r2 = target PC + +.globl psci_arch_init +psci_arch_init: + mov r6, lr + + bl psci_get_cpu_id + bl psci_get_cpu_stack_top + mov sp, r0 + + bx r6 + + @ r1 = target CPU + @ r2 = target PC + +.globl psci_cpu_on +psci_cpu_on: + push {lr} + + mov r0, r1 + bl psci_get_cpu_stack_top + str r2, [r0] + dsb + + ldr r2, =psci_cpu_entry + bl imx_cpu_on + + pop {pc} + +.globl psci_cpu_off +psci_cpu_off: + + bl psci_cpu_off_common + bl psci_get_cpu_id + bl imx_cpu_off + +1: wfi + b 1b + + .globl psci_text_end +psci_text_end: + .popsectionarch/arm/cpu/armv7/mx7/soc.c +7
-1@@ -114,10 +114,19 @@ u32 __weak get_board_rev(void) } #endif +/* enable all periherial can be access in nosec mode */ static void +init_csu (void ) { +int i =0 ; +for (i =0 ; i <64 ; i++) +writel (0x00FF00FF ,0x303e0000 + i *4 ); } +int arch_cpu_init (void ) {init_aips (); +init_csu ();/* Disable PDE bit of WMCR register */ imx_set_wdog_powerdown (false); --1.8.4
PATCH v2 2/3 mx7: psci: add basic psci support
Hello Li,
Quoted 41 lines · Li Frank
Quoted 41 lines
-----Original Message-----
From: Peng Fan [mailto:Peng.Fan at freescale.com]
Sent: Tuesday, October 20, 2015 1:00 AM
To: u-boot at lists.denx.de
Cc: Fan Peng-B51431 <[email protected]>; Li Frank-B20596 <[email protected]>; Stefano Babic <[email protected]>; Estevam Fabio- R49496 <[email protected]>
Subject: [PATCH V2 2/3] mx7: psci: add basic psci support1. add basic psci support for imx7 chip.
2. supportcpu_onandcpu_off.
3. switch to non-secure mode when boot linux kernel.
4. set csu allow accessing all peripherial register in non-secure mode.Signed-off-by Frank Li <[email protected]>Signed-off-by Peng Fan <[email protected]>Cc Stefano Babic <[email protected]>Cc Fabio Estevam <[email protected]>---
Changes V2:
Refine commit msg.arch/arm/cpu/armv7/mx7/Makefile | 4 ++ arch/arm/cpu/armv7/mx7/psci-mx7.c | 78 +++++++++++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/psci.S | 54 +++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/soc.c | 9 +++++ 4 files changed, 145 insertions(+) create mode 100644 arch/arm/cpu/armv7/mx7/psci-mx7.c create mode100644
arch/arm/cpu/armv7/mx7/psci.S(quoted diff) +0
-0b/arch/arm/cpu/armv7/mx7/Makefile index e6ecef0..f25461c 100644
arch/arm/cpu/armv7/mx7/Makefile +0
-0@@ -6,3 +6,7 @@ #
Quoted 7 lines
arch/arm/cpu/armv7/mx7/Makefile line 11
Quoted 4 lines
arch/arm/cpu/armv7/mx7/Makefile +3
-0obj-y := soc.o clock.o clock_slice.o + +ifdef CONFIG_ARMV7_PSCI +obj-y += psci.o psci-mx7.oObj-y += psci-mx7.o psci.o
The otherwisepsci_text_endwill not be last one.
I don't like this object module order sensitivity.
The object module order of secure code modules should not affect the resulting binary to the point of possibly preventing it from working -- after all, the object module order of 'vanilla' image code does not matter (1). We don't have this kind of problem when defining the image start and end, why would we have it with the secure code start and end?
IOW, psci_text_end could (and should) be defined in the linker script, not in an object module.
(1) except for start.S, which *must* be linked first, and even that is not done through object order but through linker script section order.
Quoted 2 lines
Best regards
Frank Li
Amicalement,
Signature · 1 line
Albert.
PATCH v2 2/3 mx7: psci: add basic psci support
Quoted 64 lines
-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, October 20, 2015 9:25 AM
To: Li Frank-B20596 <[email protected]>
Cc: Fan Peng-B51431 <[email protected]>; u-boot at lists.denx.de; Estevam Fabio-R49496 <[email protected]>
Subject: Re: [U-Boot] [PATCH V2 2/3] mx7: psci: add basic psci supportHello Li,
Quoted 42 lines · Li Frank
Quoted 42 lines
-----Original Message-----
From: Peng Fan [mailto:Peng.Fan at freescale.com]
Sent: Tuesday, October 20, 2015 1:00 AM
To: u-boot at lists.denx.de
Cc: Fan Peng-B51431 <[email protected]>; Li Frank-B20596 <[email protected]>; Stefano Babic <[email protected]>; Estevam Fabio-
R49496 <[email protected]>
Subject: [PATCH V2 2/3] mx7: psci: add basic psci support1. add basic psci support for imx7 chip.
2. supportcpu_onandcpu_off.
3. switch to non-secure mode when boot linux kernel.
4. set csu allow accessing all peripherial register in non-secure mode.Signed-off-by Frank Li <[email protected]>Signed-off-by Peng Fan <[email protected]>Cc Stefano Babic <[email protected]>Cc Fabio Estevam <[email protected]>---
Changes V2:
Refine commit msg.arch/arm/cpu/armv7/mx7/Makefile | 4 ++ arch/arm/cpu/armv7/mx7/psci-mx7.c | 78 +++++++++++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/psci.S | 54 +++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/soc.c | 9 +++++ 4 files changed, 145 insertions(+) create mode 100644 arch/arm/cpu/armv7/mx7/psci-mx7.c create mode100644
arch/arm/cpu/armv7/mx7/psci.S(quoted diff) +0
-0b/arch/arm/cpu/armv7/mx7/Makefile index e6ecef0..f25461c 100644
arch/arm/cpu/armv7/mx7/Makefile +0
-0@@ -6,3 +6,7 @@ #Quoted 7 lines
arch/arm/cpu/armv7/mx7/Makefile line 11
Quoted 4 lines
arch/arm/cpu/armv7/mx7/Makefile +3
-0obj-y := soc.o clock.o clock_slice.o + +ifdef CONFIG_ARMV7_PSCI +obj-y += psci.o psci-mx7.oObj-y += psci-mx7.o psci.o
The otherwisepsci_text_endwill not be last one.
Quoted 7 lines
I don't like this object module order sensitivity.
The object module order of secure code modules should not affect the resulting binary to the point of possibly preventing it from working -- after all, the object module order of 'vanilla' image code does not matter (1). We don't have this kind of problem when defining the image start and end, why would we have it with the secure code start and end?
Secure code use psci_text_end to calculate stack top point.
Quoted 3 lines
IOW,
psci_text_endcould (and should) be defined in the linker script, not in an object module.
I agree. The other platform put psci_text_end to object module.
Best regards
Frank Li
Quoted 10 lines
(1) except for
start.S, which *must* be linked first, and even that is not done through object order but through linker script section order.Quoted 2 lines
Best regards
Frank LiAmicalement,
--
Albert.
A message that is not in the archive (sent off-list or not yet received): 20151020165526.1ae0c332@lilith
A message that is not in the archive (sent off-list or not yet received): BY2PR0301MB163850B2C1169D80C249AB2782390@BY2PR0301MB1638.namprd03.prod.outlook.com
PATCH v2 2/3 mx7: psci: add basic psci support
Hello Li,
(sorry for my dropping other receipients from the discussion; restoring them
Quoted 89 lines · Li Frank
Quoted 89 lines
-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, October 20, 2015 9:55 AM
To: Li Frank-B20596 <[email protected]>
Subject: Re: [U-Boot] [PATCH V2 2/3] mx7: psci: add basic psci supportHello Li,
Quoted 40 lines · Li Frank
Quoted 40 lines
-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, October 20, 2015 9:25 AM
To: Li Frank-B20596 <[email protected]>
Cc: Fan Peng-B51431 <[email protected]>; u-boot at lists.denx.de; Estevam Fabio-R49496 <[email protected]>
Subject: Re: [U-Boot] [PATCH V2 2/3] mx7: psci: add basic psci supportHello Li,
Quoted 24 lines · Li Frank
Quoted 24 lines
-----Original Message-----
From: Peng Fan [mailto:Peng.Fan at freescale.com]
Sent: Tuesday, October 20, 2015 1:00 AM
To: u-boot at lists.denx.de
Cc: Fan Peng-B51431 <[email protected]>; Li Frank-B20596 <[email protected]>; Stefano Babic <[email protected]>; Estevam
Fabio-
R49496 <[email protected]>
Subject: [PATCH V2 2/3] mx7: psci: add basic psci support1. add basic psci support for imx7 chip.
2. supportcpu_onandcpu_off.
3. switch to non-secure mode when boot linux kernel.
4. set csu allow accessing all peripherial register in non-secure mode.Signed-off-by Frank Li <[email protected]>Signed-off-by Peng Fan <[email protected]>Cc Stefano Babic <[email protected]>Cc Fabio Estevam <[email protected]>---
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Changes V2:
Refine commit msg.arch/arm/cpu/armv7/mx7/Makefile | 4 ++ arch/arm/cpu/armv7/mx7/psci-mx7.c | 78 +++++++++++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/psci.S | 54+++++++++++++++++++++++++++
Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
Quoted 10 lines
arch/arm/cpu/armv7/mx7/soc.c | 9 +++++ 4 files changed, 145 insertions(+) create mode 100644
arch/arm/cpu/armv7/mx7/psci-mx7.ccreate mode
100644arch/arm/cpu/armv7/mx7/psci.S(quoted diff) +0
-0b/arch/arm/cpu/armv7/mx7/Makefile index e6ecef0..f25461c 100644
arch/arm/cpu/armv7/mx7/Makefile line 11
Quoted 6 lines
arch/arm/cpu/armv7/mx7/Makefile +3
-0@@ -6,3 +6,7 @@ # obj-y := soc.o clock.o clock_slice.o + +ifdef CONFIG_ARMV7_PSCI +obj-y += psci.o psci-mx7.oObj-y += psci-mx7.o psci.o
The otherwisepsci_text_endwill not be last one.Quoted 7 lines
Quoted 7 lines
I don't like this object module order sensitivity.
The object module order of secure code modules should not affect the resulting binary to the point of possibly preventing it from working -- after all, the object module order of 'vanilla' image code does not matter (1). We don't have this kind of problem when defining the image start and end, why would we have it with the secure code start and
end?
Quoted 1 line
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Secure code use
psci_text_endto calculate stack top point.Yes, and that does not require defining
psci_text_endin an object module, nor does it require specific object module ordering.If
psci_text_endin mid of secure text,
Andpsci_text_end+ 1k < total secure text size, stack will overwrite the code.
This is a good reason to allocate the PSCCI stack and create a symbol for its top in the linker script.
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
IOW,
psci_text_endcould (and should) be defined in the linker script, not in an object module.I agree. The other platform put
psci_text_endto object module.Which 'other platform' is this?
arch/arm/cpu/armv7/sunxiarch/arm/cpu/armv7/ls102xa
Ok, but they all use the same routine for moving the PSCI code and setting its stack, right?
Amicalement,
Signature · 1 line
Albert.
PATCH v2 2/3 mx7: psci: add basic psci support
Quoted 128 lines
-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, October 20, 2015 4:04 PM
To: Li Frank-B20596 <[email protected]>
Cc: Fan Peng-B51431 <[email protected]>; Estevam Fabio-R49496 <[email protected]>; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH V2 2/3] mx7: psci: add basic psci supportHello Li,
(sorry for my dropping other receipients from the discussion; restoring them
Quoted 44 lines · Li Frank
Quoted 44 lines
-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, October 20, 2015 9:55 AM
To: Li Frank-B20596 <[email protected]>
Subject: Re: [U-Boot] [PATCH V2 2/3] mx7: psci: add basic psci supportHello Li,
Quoted 30 lines · Li Frank
Quoted 30 lines
-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, October 20, 2015 9:25 AM
To: Li Frank-B20596 <[email protected]>
Cc: Fan Peng-B51431 <[email protected]>;
u-boot at lists.denx.de; Estevam Fabio-R49496 <[email protected]>
Subject: Re: [U-Boot] [PATCH V2 2/3] mx7: psci: add basic psci supportHello Li,
Quoted 13 lines · Li Frank
Quoted 13 lines
-----Original Message-----
From: Peng Fan [mailto:Peng.Fan at freescale.com]
Sent: Tuesday, October 20, 2015 1:00 AM
To: u-boot at lists.denx.de
Cc: Fan Peng-B51431 <[email protected]>; Li Frank-B20596 <[email protected]>; Stefano Babic <[email protected]>; Estevam
Fabio-
R49496 <[email protected]>
Subject: [PATCH V2 2/3] mx7: psci: add basic psci supportQuoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
1. add basic psci support for imx7 chip.
2. supportcpu_onandcpu_off.
3. switch to non-secure mode when boot linux kernel.
4. set csu allow accessing all peripherial register in non-securemode.
Quoted 47 lines
Quoted 47 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Signed-off-by Frank Li <[email protected]>Signed-off-by Peng Fan <[email protected]>Cc Stefano Babic <[email protected]>Cc Fabio Estevam <[email protected]>---
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Changes V2:
Refine commit msg.arch/arm/cpu/armv7/mx7/Makefile | 4 ++ arch/arm/cpu/armv7/mx7/psci-mx7.c | 78 +++++++++++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/mx7/psci.S | 54+++++++++++++++++++++++++++
Quoted 23 lines
Quoted 23 lines
Quoted 13 lines
Quoted 13 lines
arch/arm/cpu/armv7/mx7/soc.c | 9 +++++ 4 files changed, 145 insertions(+) create mode 100644
arch/arm/cpu/armv7/mx7/psci-mx7.ccreate mode 100644arch/arm/cpu/armv7/mx7/psci.S(quoted diff) +0
-0b/arch/arm/cpu/armv7/mx7/Makefile index e6ecef0..f25461c 100644
arch/arm/cpu/armv7/mx7/Makefile +0
-0@@ -6,3 +6,7 @@ #Quoted 7 lines
arch/arm/cpu/armv7/mx7/Makefile line 11
Quoted 4 lines
arch/arm/cpu/armv7/mx7/Makefile +3
-0obj-y := soc.o clock.o clock_slice.o + +ifdef CONFIG_ARMV7_PSCI +obj-y += psci.o psci-mx7.oObj-y += psci-mx7.o psci.o
The otherwisepsci_text_endwill not be last one.I don't like this object module order sensitivity.
Quoted 7 lines
Quoted 7 lines
The object module order of secure code modules should not affect the resulting binary to the point of possibly preventing it from working
-- after all, the object module order of 'vanilla' image code does not matter (1). We don't have this kind of problem when defining the image start and end, why would we have it with the secure code start andend?
Quoted 1 line
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Secure code use
psci_text_endto calculate stack top point.Yes, and that does not require defining
psci_text_endin an object module, nor does it require specific object module ordering.If
psci_text_endin mid of secure text,
Andpsci_text_end+ 1k < total secure text size, stack will overwrite the code.This is a good reason to allocate the PSCCI stack and create a symbol for its top in the linker script.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
IOW,
psci_text_endcould (and should) be defined in the linker script, not in an object module.I agree. The other platform put
psci_text_endto object module.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Which 'other platform' is this?
arch/arm/cpu/armv7/sunxiarch/arm/cpu/armv7/ls102xaOk, but they all use the same routine for moving the PSCI code and setting its stack, right?
Yes.
Best regards
Frank Li
Quoted 4 lines
Amicalement,
--
Albert.
PATCH v2 3/3 imx: mx7: default enable non-secure mode
In reply to Peng FanSupport PSCI and switch to non-secure mode when booting linux.
---
Changes V2:
default no enable CONFIG_ARMV7_VIRT
include/configs/mx7_common.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
include/configs/mx7_common.h +10 -0
@@ -92,4 +92,14 @@#define CONFIG_CMD_FUSE #define CONFIG_MXC_OCOTP +/* Default boot linux kernel in no secure mode. +* If want to boot kernel in secure mode, please define CONFIG_MX7_SEC +*/ +#ifndef CONFIG_MX7_SEC +#define CONFIG_ARMV7_NONSEC 1 +#define CONFIG_ARMV7_PSCI 1 +#define CONFIG_ARMV7_PSCI_NR_CPUS 2 +#define CONFIG_ARMV7_SECURE_BASE 0x00900000 +#endif +#endif
Signature · 1 line
1.8.4
PATCH v2 1/3 arm: discard relocation entry for secure section
In reply to Peng FanHello Peng,
Quoted 12 lines · Peng Fan
The code such as PSCI in section named secure is bundled with u-boot image, and when bootm, the code will be copied to their runtime address same to compliation/linking address -
CONFIG_ARMV7_SECURE_BASE.When compile the PSCI code and link it into the u-boot image, there will be relocation entries in .rel.dyn section for PSCI. Actually, we do not needs these relocation entries.
If still keep the relocation entries in .rel.dyn section, r0 at line 103 and 106 in
arch/arm/lib/relocate.Smay be an invalid address which may not support read/write for one SoC.
Quoted 7 lines
102 /* relative fix: increase location by offset */ 103 add r0, r0, r4 104 ldr r1, [r0] 105 add r1, r1, r4 106 str r1, [r0]So discard the relocation entries for code in secure section.
Actually, I'll need you to do a slight change -- that's my fault, and karma even ensured that my own self of two years ago would be the one to come and kick me. See:
http://lists.denx.de/pipermail/u-boot/2013-December/168652.html
Which basically is about never discarding any section in the ELF file, and only copying a subset of the ELF sections into the binary file.
So rather than discarding the secure relocation records, let's move them to another section as you had proposed, and thus change the line
arch/arm/cpu/u-boot.lds SECTIONS · line 55
Quoted 1 line
arch/arm/cpu/u-boot.lds +1
-0+ /DISCARD/ : { *(.rel._secure*) }
Into a
.rel._secure { *(.rel._secure*) }Placed right after the already present
.dynamic : { *(.dynamic*) }With my apologies for the very late realization.
Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/3 arm: discard relocation entry for secure section
Hi Albert,
Quoted 22 lines · Albert ARIBAUD
Hello Peng,
Quoted 18 lines · Peng Fan
The code such as PSCI in section named secure is bundled with u-boot image, and when bootm, the code will be copied to their runtime address same to compliation/linking address -
CONFIG_ARMV7_SECURE_BASE.When compile the PSCI code and link it into the u-boot image, there will be relocation entries in .rel.dyn section for PSCI. Actually, we do not needs these relocation entries.
If still keep the relocation entries in .rel.dyn section, r0 at line 103 and 106 in
arch/arm/lib/relocate.Smay be an invalid address which may not support read/write for one SoC.102 /* relative fix: increase location by offset */ 103 add r0, r0, r4 104 ldr r1, [r0] 105 add r1, r1, r4 106 str r1, [r0]
Quoted 7 lines
Quoted 1 line
So discard the relocation entries for code in secure section.
Actually, I'll need you to do a slight change -- that's my fault, and karma even ensured that my own self of two years ago would be the one to come and kick me. See:
http://lists.denx.de/pipermail/u-boot/2013-December/168652.html
Ok. Then arch/arm/cpu/armv8/u-boot.lds should also have such fix, since lots sections are discarded in u-boot.lds for armv8.
Quoted 11 lines
Which basically is about never discarding any section in the ELF file, and only copying a subset of the ELF sections into the binary file.
So rather than discarding the secure relocation records, let's move them to another section as you had proposed, and thus change the line
arch/arm/cpu/u-boot.lds SECTIONS · line 55
Quoted 1 line
arch/arm/cpu/u-boot.lds +1
-0+ /DISCARD/ : { *(.rel._secure*) }Into a
Quoted 5 lines
.rel._secure { *(.rel._secure*) }Placed right after the already present
.dynamic : { *(.dynamic*) }
It can not be placed after .dynamic, since the following is at front.
87 .rel.dyn : {
88 *(.rel*)
89 }
So relocation entry from secure text will first placed into .rel.dyn section.
If not DISCARD, then I prefer to put ".rel.secure : { *(.rel._secure*) }" at line 55 which is wrapped by CONFIG_ARMV7_NONSEC in arch/arm/cpu/u-boot.lds.
See following patch:
PATCH v2 1/3 arm: discard relocation entry for secure section
Hello Peng,
Quoted 27 lines · Peng Fan
Hi Albert,
Quoted 24 lines · Albert ARIBAUD
Hello Peng,
Quoted 19 lines · Peng Fan
The code such as PSCI in section named secure is bundled with u-boot image, and when bootm, the code will be copied to their runtime address same to compliation/linking address -
CONFIG_ARMV7_SECURE_BASE.When compile the PSCI code and link it into the u-boot image, there will be relocation entries in .rel.dyn section for PSCI. Actually, we do not needs these relocation entries.
If still keep the relocation entries in .rel.dyn section, r0 at line 103 and 106 in
arch/arm/lib/relocate.Smay be an invalid address which may not support read/write for one SoC.102 /* relative fix: increase location by offset */ 103 add r0, r0, r4 104 ldr r1, [r0] 105 add r1, r1, r4 106 str r1, [r0]So discard the relocation entries for code in secure section.
Quoted 8 lines
Quoted 5 lines
Actually, I'll need you to do a slight change -- that's my fault, and karma even ensured that my own self of two years ago would be the one to come and kick me. See:
http://lists.denx.de/pipermail/u-boot/2013-December/168652.html
Ok. Then
arch/arm/cpu/armv8/u-boot.ldsshould also have such fix, since lots sections are discarded inu-boot.ldsfor armv8.
You are correct, armv8 needs to be fixed, as well as zynq (and x86 but that's outside of my province). Anyway, that's a different problem which does not need to be addressed in this series.
Quoted 16 lines
Quoted 10 lines
Which basically is about never discarding any section in the ELF file, and only copying a subset of the ELF sections into the binary file.
So rather than discarding the secure relocation records, let's move them to another section as you had proposed, and thus change the line
arch/arm/cpu/u-boot.lds SECTIONS · line 55
Quoted 1 line
arch/arm/cpu/u-boot.lds +1
-0+ /DISCARD/ : { *(.rel._secure*) }Into a
Quoted 5 lines
.rel._secure { *(.rel._secure*) }Placed right after the already present
.dynamic : { *(.dynamic*) }
Quoted 8 lines
It can not be placed after .dynamic, since the following is at front.
87 .rel.dyn : {
88 *(.rel*)
89 }
So relocation entry from secure text will first placed into .rel.dyn section.If not DISCARD, then I prefer to put ".rel.secure : { *(.rel._secure*) }" at line 55 which is wrapped by
CONFIG_ARMV7_NONSECinarch/arm/cpu/u-boot.lds.
I prefer all 'unused' sections to be kept together near the end of the LDS file -- I'll add an explicit comment in the LDS about it.
Besides, there no need to wrap such a section with a preprocessor conditional, as the linker will simply not emit an output section if there are no input sections at all for it; IOW, adding the '.rel._secure { *(.rel._secure*) }' line will be binary-invariant for platforms which do not have PSCI or other secure code.
Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/3 arm: discard relocation entry for secure section
Hi Albert,
Quoted 69 lines · Albert ARIBAUD
Hello Peng,
Quoted 27 lines · Peng Fan
Hi Albert,
Quoted 24 lines · Albert ARIBAUD
Hello Peng,
Quoted 19 lines · Peng Fan
The code such as PSCI in section named secure is bundled with u-boot image, and when bootm, the code will be copied to their runtime address same to compliation/linking address -
CONFIG_ARMV7_SECURE_BASE.When compile the PSCI code and link it into the u-boot image, there will be relocation entries in .rel.dyn section for PSCI. Actually, we do not needs these relocation entries.
If still keep the relocation entries in .rel.dyn section, r0 at line 103 and 106 in
arch/arm/lib/relocate.Smay be an invalid address which may not support read/write for one SoC.102 /* relative fix: increase location by offset */ 103 add r0, r0, r4 104 ldr r1, [r0] 105 add r1, r1, r4 106 str r1, [r0]So discard the relocation entries for code in secure section.
Quoted 8 lines
Quoted 5 lines
Actually, I'll need you to do a slight change -- that's my fault, and karma even ensured that my own self of two years ago would be the one to come and kick me. See:
http://lists.denx.de/pipermail/u-boot/2013-December/168652.html
Ok. Then
arch/arm/cpu/armv8/u-boot.ldsshould also have such fix, since lots sections are discarded inu-boot.ldsfor armv8.You are correct, armv8 needs to be fixed, as well as zynq (and x86 but that's outside of my province). Anyway, that's a different problem which does not need to be addressed in this series.
Quoted 16 lines
Quoted 10 lines
Which basically is about never discarding any section in the ELF file, and only copying a subset of the ELF sections into the binary file.
So rather than discarding the secure relocation records, let's move them to another section as you had proposed, and thus change the line
arch/arm/cpu/u-boot.lds SECTIONS · line 55
Quoted 1 line
arch/arm/cpu/u-boot.lds +1
-0+ /DISCARD/ : { *(.rel._secure*) }Into a
Quoted 5 lines
.rel._secure { *(.rel._secure*) }Placed right after the already present
.dynamic : { *(.dynamic*) }Quoted 8 lines
It can not be placed after .dynamic, since the following is at front.
87 .rel.dyn : {
88 *(.rel*)
89 }
So relocation entry from secure text will first placed into .rel.dyn section.If not DISCARD, then I prefer to put ".rel.secure : { *(.rel._secure*) }" at line 55 which is wrapped by
CONFIG_ARMV7_NONSECinarch/arm/cpu/u-boot.lds.
Quoted 8 lines
I prefer all 'unused' sections to be kept together near the end of the LDS file -- I'll add an explicit comment in the LDS about it.
Besides, there no need to wrap such a section with a preprocessor conditional, as the linker will simply not emit an output section if there are no input sections at all for it; IOW, adding the '.rel._secure { *(.rel._secure*) }' line will be binary-invariant for platforms which do not have PSCI or other secure code.
But ".rel._secure { *(.rel._secure*) }" can not be placed after ".dynamic : { *(.dynamic*) }". Actually ".rel._secure { *(.rel._secure*) }" should be placed before ".rel_dyn_start" in u-boot.lds, otherwise the secure relocation entries will be placed into ".rel.dyn", but not ".rel._secure".
Regards,
Peng.
Quoted 4 lines
Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/3 arm: discard relocation entry for secure section
Hello Peng,
Quoted 81 lines · Peng Fan
Hi Albert,
Quoted 69 lines · Albert ARIBAUD
Hello Peng,
Quoted 27 lines · Peng Fan
Hi Albert,
Quoted 24 lines · Albert ARIBAUD
Hello Peng,
Quoted 19 lines · Peng Fan
The code such as PSCI in section named secure is bundled with u-boot image, and when bootm, the code will be copied to their runtime address same to compliation/linking address -
CONFIG_ARMV7_SECURE_BASE.When compile the PSCI code and link it into the u-boot image, there will be relocation entries in .rel.dyn section for PSCI. Actually, we do not needs these relocation entries.
If still keep the relocation entries in .rel.dyn section, r0 at line 103 and 106 in
arch/arm/lib/relocate.Smay be an invalid address which may not support read/write for one SoC.102 /* relative fix: increase location by offset */ 103 add r0, r0, r4 104 ldr r1, [r0] 105 add r1, r1, r4 106 str r1, [r0]So discard the relocation entries for code in secure section.
Quoted 8 lines
Quoted 5 lines
Actually, I'll need you to do a slight change -- that's my fault, and karma even ensured that my own self of two years ago would be the one to come and kick me. See:
http://lists.denx.de/pipermail/u-boot/2013-December/168652.html
Ok. Then
arch/arm/cpu/armv8/u-boot.ldsshould also have such fix, since lots sections are discarded inu-boot.ldsfor armv8.You are correct, armv8 needs to be fixed, as well as zynq (and x86 but that's outside of my province). Anyway, that's a different problem which does not need to be addressed in this series.
Quoted 16 lines
Quoted 10 lines
Which basically is about never discarding any section in the ELF file, and only copying a subset of the ELF sections into the binary file.
So rather than discarding the secure relocation records, let's move them to another section as you had proposed, and thus change the line
arch/arm/cpu/u-boot.lds SECTIONS · line 55
Quoted 1 line
arch/arm/cpu/u-boot.lds +1
-0+ /DISCARD/ : { *(.rel._secure*) }Into a
Quoted 5 lines
.rel._secure { *(.rel._secure*) }Placed right after the already present
.dynamic : { *(.dynamic*) }Quoted 8 lines
It can not be placed after .dynamic, since the following is at front.
87 .rel.dyn : {
88 *(.rel*)
89 }
So relocation entry from secure text will first placed into .rel.dyn section.If not DISCARD, then I prefer to put ".rel.secure : { *(.rel._secure*) }" at line 55 which is wrapped by
CONFIG_ARMV7_NONSECinarch/arm/cpu/u-boot.lds.Quoted 8 lines
I prefer all 'unused' sections to be kept together near the end of the LDS file -- I'll add an explicit comment in the LDS about it.
Besides, there no need to wrap such a section with a preprocessor conditional, as the linker will simply not emit an output section if there are no input sections at all for it; IOW, adding the '.rel._secure { *(.rel._secure*) }' line will be binary-invariant for platforms which do not have PSCI or other secure code.
Quoted 5 lines
But ".rel._secure { *(.rel._secure*) }" can not be placed after ".dynamic : { *(.dynamic*) }". Actually ".rel._secure { *(.rel._secure*) }" should be placed before ".rel_dyn_start" in
u-boot.lds, otherwise the secure relocation entries will be placed into ".rel.dyn", but not ".rel._secure".
Hmm, you're correct, the linker will use the first pattern match, not the longest or best. :(
But then, the secure reloc input section cannot go in line 55, because that's still inside the binary part of the image, which means it will waste space in there.
So it's either use DISCARD at the very start of the SECTIONS (round line 17) or back to not generating relocatable code at all for PSCI.
Quoted 9 lines
Regards,
Peng.Quoted 4 lines
Amicalement,
Signature · 1 line
Albert.
Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/3 arm: discard relocation entry for secure section
Hello Albert,
Quoted 94 lines · Albert ARIBAUD
Hello Peng,
Quoted 81 lines · Peng Fan
Hi Albert,
Quoted 69 lines · Albert ARIBAUD
Hello Peng,
Quoted 27 lines · Peng Fan
Hi Albert,
Quoted 24 lines · Albert ARIBAUD
Hello Peng,
Quoted 19 lines · Peng Fan
The code such as PSCI in section named secure is bundled with u-boot image, and when bootm, the code will be copied to their runtime address same to compliation/linking address -
CONFIG_ARMV7_SECURE_BASE.When compile the PSCI code and link it into the u-boot image, there will be relocation entries in .rel.dyn section for PSCI. Actually, we do not needs these relocation entries.
If still keep the relocation entries in .rel.dyn section, r0 at line 103 and 106 in
arch/arm/lib/relocate.Smay be an invalid address which may not support read/write for one SoC.102 /* relative fix: increase location by offset */ 103 add r0, r0, r4 104 ldr r1, [r0] 105 add r1, r1, r4 106 str r1, [r0]So discard the relocation entries for code in secure section.
Quoted 8 lines
Quoted 5 lines
Actually, I'll need you to do a slight change -- that's my fault, and karma even ensured that my own self of two years ago would be the one to come and kick me. See:
http://lists.denx.de/pipermail/u-boot/2013-December/168652.html
Ok. Then
arch/arm/cpu/armv8/u-boot.ldsshould also have such fix, since lots sections are discarded inu-boot.ldsfor armv8.You are correct, armv8 needs to be fixed, as well as zynq (and x86 but that's outside of my province). Anyway, that's a different problem which does not need to be addressed in this series.
Quoted 16 lines
Quoted 10 lines
Which basically is about never discarding any section in the ELF file, and only copying a subset of the ELF sections into the binary file.
So rather than discarding the secure relocation records, let's move them to another section as you had proposed, and thus change the line
Review
Quoted 1 line
(quoted diff) +1
-0+ /DISCARD/ : { *(.rel._secure*) }Into a
Quoted 5 lines
.rel._secure { *(.rel._secure*) }Placed right after the already present
.dynamic : { *(.dynamic*) }Quoted 8 lines
It can not be placed after .dynamic, since the following is at front.
87 .rel.dyn : {
88 *(.rel*)
89 }
So relocation entry from secure text will first placed into .rel.dyn section.If not DISCARD, then I prefer to put ".rel.secure : { *(.rel._secure*) }" at line 55 which is wrapped by
CONFIG_ARMV7_NONSECinarch/arm/cpu/u-boot.lds.Quoted 8 lines
I prefer all 'unused' sections to be kept together near the end of the LDS file -- I'll add an explicit comment in the LDS about it.
Besides, there no need to wrap such a section with a preprocessor conditional, as the linker will simply not emit an output section if there are no input sections at all for it; IOW, adding the '.rel._secure { *(.rel._secure*) }' line will be binary-invariant for platforms which do not have PSCI or other secure code.
Quoted 5 lines
But ".rel._secure { *(.rel._secure*) }" can not be placed after ".dynamic : { *(.dynamic*) }". Actually ".rel._secure { *(.rel._secure*) }" should be placed before ".rel_dyn_start" in
u-boot.lds, otherwise the secure relocation entries will be placed into ".rel.dyn", but not ".rel._secure".Hmm, you're correct, the linker will use the first pattern match, not the longest or best. :(
Quoted 6 lines
But then, the secure reloc input section cannot go in line 55, because that's still inside the binary part of the image, which means it will waste space in there.
So it's either use DISCARD at the very start of the SECTIONS (round line 17) or back to not generating relocatable code at all for PSCI.
I do not know how to generate non-relocatable code only for the secure text. Since -mword-relocations and -pie are global flags, I do not know how to disable mword-relocations when compiling PSCI and other secure text.
Is this ok for you?
arch/arm/cpu/u-boot.lds +0 -0
@@ -14,6 +14,7 @@ OUTPUT_ARCH(arm)
ENTRY(_start)
SECTIONS
{
+ /DISCARD/ : { *(.rel._secure*) }
. = 0x00000000;
. = ALIGN(4);Regards,
Peng.
Quoted 16 lines
Quoted 10 lines
Regards,
Peng.Quoted 4 lines
Amicalement,
Signature · 1 line
Albert.Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/3 arm: discard relocation entry for secure section
Hello Peng,
Quoted 5 lines · Peng Fan
Hello Albert,
I do not know how to generate non-relocatable code only for the secure text. Since -mword-relocations and -pie are global flags, I do not know how to disable mword-relocations when compiling PSCI and other secure text.
Actually, I haven't even found a compiler or gcc option to entirely remove relocations altogether.
Quoted 11 lines
Is this ok for you?
arch/arm/cpu/u-boot.lds +0
-0@@ -14,6 +14,7 @@ OUTPUT_ARCH(arm) ENTRY(_start)SECTIONS
{
+ /DISCARD/ : { *(.rel._secure*) }
Quoted 3 lines
. = 0x00000000;
. = ALIGN(4);Functionally, it works -- I've just checked it by comparing u-boot.map files of a mx7dsabresd build with and without the DISCARD. Not a single symbol from the text and data section gets changed.
The only missing thing left is a comment before the DISCARD line, explaining both why .rel._secure* input sections have to be discarded, and also explaining why this is done it right at the start of the output sections.
Thanks for your patience. :)
Quoted 2 lines
Regards,
Peng.
Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/3 arm: discard relocation entry for secure section
Hello Albert,
Quoted 32 lines · Albert ARIBAUD
Hello Peng,
Quoted 5 lines · Peng Fan
Hello Albert,
I do not know how to generate non-relocatable code only for the secure text. Since -mword-relocations and -pie are global flags, I do not know how to disable mword-relocations when compiling PSCI and other secure text.
Actually, I haven't even found a compiler or gcc option to entirely remove relocations altogether.
Quoted 11 lines
Is this ok for you?
arch/arm/cpu/u-boot.lds +0
-0@@ -14,6 +14,7 @@ OUTPUT_ARCH(arm) ENTRY(_start)SECTIONS
{
+ /DISCARD/ : { *(.rel._secure*) }Quoted 3 lines
. = 0x00000000; . = ALIGN(4);Functionally, it works -- I've just checked it by comparing u-boot.map files of a mx7dsabresd build with and without the DISCARD. Not a single symbol from the text and data section gets changed.
Quoted 4 lines
The only missing thing left is a comment before the DISCARD line, explaining both why .rel._secure* input sections have to be discarded, and also explaining why this is done it right at the start of the output sections.
Ok. Will add the comments in V3.
Regards,
Peng.
Quoted 9 lines
Thanks for your patience. :)
Quoted 2 lines
Regards,
Peng.Amicalement,
Signature · 1 line
Albert.