Skip to contentSkip to conversation

ARM: exynos: Fix imprecise abort during Exynos5422 suspend to RAM

RFC In review Krzysztof Kozlowski 2 participants 3 messages Jul 18, 2018 – Jul 24, 2018

Subsystems (4)
lore
Share & tools
lore

Krzysztof Kozlowski

author maintainer
#0 lore raw

RFC 1/1 ARM: exynos: Fix imprecise abort during Exynos5422 suspend to RAM

Suspend to RAM on Odroid XU3/XU4/HC1 family (Exynos5422) causes imprecise abort:

PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.003 seconds) done. OOM killer disabled.
Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done. wake enabled for irq 139
Disabling non-boot CPUs ...
IRQ51 no longer affine to CPU1
IRQ52 no longer affine to CPU2
IRQ53 no longer affine to CPU3
IRQ54 no longer affine to CPU4
IRQ55 no longer affine to CPU5
IRQ56 no longer affine to CPU6
cpu cpu4: Dropping the link to regulator.40
IRQ57 no longer affine to CPU7
Unhandled fault: external abort on non-linefetch (0x1008) at 0xf081a028 Internal error: : 1008 [#1] PREEMPT SMP ARM

with last call trace in exynos_suspend_enter().

The abort is caused by writing to register in secure part of sysram. All Exynos5422 devices, including Hardkernel Odroid boards, boot with TrustZone in non-secure mode therefore they should access non-secure sysram.

---

Comments, whether my understanding is correct, are welcomed!

---
 Documentation/arm/Samsung/Bootloader-interface.txt | 1 +
 arch/arm/mach-exynos/suspend.c                     | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)
Documentation/arm/Samsung/Bootloader-interface.txt +1 -0
index ed494ac0beb2..d17ed518a7ea 100644
@@ -26,6 +26,7 @@ Offset        Value                                        Purpose
 0x20          0xfcba0d10 (Magic cookie)                    AFTR
 0x24          exynos_cpu_resume_ns                         AFTR
 0x28 + 4*cpu  0x8 (Magic cookie, Exynos3250)               AFTR
+0x28          0x0 or last value during resume (Exynos542x) System suspend
 
 
 2. Secure mode
arch/arm/mach-exynos/suspend.c +3 -3
index d3db306a5a70..0ec52f442b97 100644
@@ -256,7 +256,7 @@ static int exynos5420_cpu_suspend(unsigned long arg)
 	unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
 	unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
 
-	writel_relaxed(0x0, sysram_base_addr + EXYNOS5420_CPU_STATE);
+	writel_relaxed(0x0, sysram_ns_base_addr + EXYNOS5420_CPU_STATE);
 
 	if (IS_ENABLED(CONFIG_EXYNOS5420_MCPM)) {
 		mcpm_set_entry_vector(cpu, cluster, exynos_cpu_resume);
@@ -328,7 +328,7 @@ static void exynos5420_pm_prepare(void)
 	 * needs to restore it back in case, the primary cpu fails to
 	 * suspend for any reason.
 	 */
-	exynos5420_cpu_state = readl_relaxed(sysram_base_addr +
+	exynos5420_cpu_state = readl_relaxed(sysram_ns_base_addr +
 					     EXYNOS5420_CPU_STATE);
 
 	exynos_pm_enter_sleep_mode();
@@ -448,7 +448,7 @@ static void exynos5420_pm_resume(void)
 
 	/* Restore the sysram cpu state register */
 	writel_relaxed(exynos5420_cpu_state,
-		       sysram_base_addr + EXYNOS5420_CPU_STATE);
+		       sysram_ns_base_addr + EXYNOS5420_CPU_STATE);
 
 	pmu_raw_writel(EXYNOS5420_USE_STANDBY_WFI_ALL,
 			S5P_CENTRAL_SEQ_OPTION);
Signature · 1 line
2.14.1
#1 lore raw

Hi Krzysztof,

Quoted 27 lines · Krzysztof Kozlowski

Suspend to RAM on Odroid XU3/XU4/HC1 family (Exynos5422) causes imprecise abort:

PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.003 seconds) done. OOM killer disabled.
Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done. wake enabled for irq 139
Disabling non-boot CPUs ...
IRQ51 no longer affine to CPU1
IRQ52 no longer affine to CPU2
IRQ53 no longer affine to CPU3
IRQ54 no longer affine to CPU4
IRQ55 no longer affine to CPU5
IRQ56 no longer affine to CPU6
cpu cpu4: Dropping the link to regulator.40
IRQ57 no longer affine to CPU7
Unhandled fault: external abort on non-linefetch (0x1008) at 0xf081a028 Internal error: : 1008 [#1] PREEMPT SMP ARM

with last call trace in exynos_suspend_enter().

The abort is caused by writing to register in secure part of sysram. All Exynos5422 devices, including Hardkernel Odroid boards, boot with TrustZone in non-secure mode therefore they should access non-secure sysram.

Quoted 5 lines

---

Comments, whether my understanding is correct, are welcomed!

The decision, weather to use secure or non-secure sysram depends on the enabled TrustZone interface IMHO. Exynos5420 PeachPIT and Exynos5800 PeachPI don't use TrustZone and don't have 'firmware' node in dts, thus they should keep existing code.

Quoted 40 lines
---
  Documentation/arm/Samsung/Bootloader-interface.txt | 1 +
  arch/arm/mach-exynos/suspend.c                     | 6 +++---
  2 files changed, 4 insertions(+), 3 deletions(-)
Documentation/arm/Samsung/Bootloader-interface.txt +1 -0
index ed494ac0beb2..d17ed518a7ea 100644
@@ -26,6 +26,7 @@ Offset        Value                                        Purpose
  0x20          0xfcba0d10 (Magic cookie)                    AFTR
  0x24          exynos_cpu_resume_ns                         AFTR
  0x28 + 4*cpu  0x8 (Magic cookie, Exynos3250)               AFTR
+0x28          0x0 or last value during resume (Exynos542x) System suspend
 
 
  2. Secure mode
arch/arm/mach-exynos/suspend.c +2 -2
index d3db306a5a70..0ec52f442b97 100644
@@ -256,7 +256,7 @@ static int exynos5420_cpu_suspend(unsigned long arg)
  	unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
  	unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
 
-	writel_relaxed(0x0, sysram_base_addr + EXYNOS5420_CPU_STATE);
+	writel_relaxed(0x0, sysram_ns_base_addr + EXYNOS5420_CPU_STATE);
 
  	if (IS_ENABLED(CONFIG_EXYNOS5420_MCPM)) {
  		mcpm_set_entry_vector(cpu, cluster, exynos_cpu_resume);
@@ -328,7 +328,7 @@ static void exynos5420_pm_prepare(void)
  	 * needs to restore it back in case, the primary cpu fails to
  	 * suspend for any reason.
  	 */
-	exynos5420_cpu_state = readl_relaxed(sysram_base_addr +
+	exynos5420_cpu_state = readl_relaxed(sysram_ns_base_addr +
  					     EXYNOS5420_CPU_STATE);
 
  	exynos_pm_enter_sleep_mode();

arch/arm/mach-exynos/suspend.c static void exynos5420_pm_resume(void) · line 454

Quoted 8 lines
arch/arm/mach-exynos/suspend.c +1 -1
@@ -448,7 +448,7 @@ static void exynos5420_pm_resume(void)
 
  	/* Restore the sysram cpu state register */
  	writel_relaxed(exynos5420_cpu_state,
-		       sysram_base_addr + EXYNOS5420_CPU_STATE);
+		       sysram_ns_base_addr + EXYNOS5420_CPU_STATE);
 
  	pmu_raw_writel(EXYNOS5420_USE_STANDBY_WFI_ALL,
  			S5P_CENTRAL_SEQ_OPTION);

Best regards

Signature · 2 lines
Marek Szyprowski, PhD
Samsung R&D Institute Poland
author maintainer
#2 lore raw
Quoted 32 lines · Marek Szyprowski

Hi Krzysztof,

Quoted 29 lines · Krzysztof Kozlowski

Suspend to RAM on Odroid XU3/XU4/HC1 family (Exynos5422) causes imprecise abort:

PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.003 seconds) done. OOM killer disabled.
Freezing remaining freezable tasks ... (elapsed 0.003 seconds) done. wake enabled for irq 139
Disabling non-boot CPUs ...
IRQ51 no longer affine to CPU1
IRQ52 no longer affine to CPU2
IRQ53 no longer affine to CPU3
IRQ54 no longer affine to CPU4
IRQ55 no longer affine to CPU5
IRQ56 no longer affine to CPU6
cpu cpu4: Dropping the link to regulator.40
IRQ57 no longer affine to CPU7
Unhandled fault: external abort on non-linefetch (0x1008) at 0xf081a028 Internal error: : 1008 [#1] PREEMPT SMP ARM

with last call trace in exynos_suspend_enter().

The abort is caused by writing to register in secure part of sysram. All Exynos5422 devices, including Hardkernel Odroid boards, boot with TrustZone in non-secure mode therefore they should access non-secure sysram.

Quoted 8 lines
Quoted 3 lines

---

Comments, whether my understanding is correct, are welcomed!

The decision, weather to use secure or non-secure sysram depends on the enabled TrustZone interface IMHO. Exynos5420 PeachPIT and Exynos5800 PeachPI don't use TrustZone and don't have 'firmware' node in dts, thus they should keep existing code.

Ah, you are right. SMDK5420 also would be affected. I'll fix the patch.

Thanks for feedback!

Best regards,
Krzysztof