All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: omap2+: Revert omap-smp.c changes resetting cpu1 during boot
@ 2017-02-13 21:50 ` Tony Lindgren
  0 siblings, 0 replies; 58+ messages in thread
From: Tony Lindgren @ 2017-02-13 21:50 UTC (permalink / raw)
  To: linux-omap; +Cc: Tero Kristo, Keerthy, linux-arm-kernel

Commit 3251885285e1 ("ARM: OMAP4+: Reset CPU1 properly for kexec") started
resetting cpu1 because of a kexec boot issue I was seeing earlier in 2016
on omap4 when doing kexec boot between two different kernel versions. The
booted kernel ended up trying to use the old kernel start-up address unless
cpu1 was reset before configuring the cpu1 start-up address.

It seems the reset part was not correct but probably working around some
other issue. I have not been able to reproduce this issue any longer despite
testing with backported patches back to v4.6 kernel. So it is possible this
issue was caused by other work in progress kexec patches I had applied. Or
it is possible some other fixes have made the issue go way.

The unconditional reset of cpu1 can cause issues booting some devices. For
example, bootloader configured secure OS running on cpu1 will fail as the
configuration is not preserved as reported by Andrew F. Davis <afd@ti.com>.

Let's fix the issue by reverting the cpu1 reset parts. If it turns out we
still need to reset cpu1 in some cases, we can add it back and do it
conditionally.

Fixes: 3251885285e1 ("ARM: OMAP4+: Reset CPU1 properly for kexec")
Cc: Keerthy <j-keerthy@ti.com>
Cc: Tero Kristo <t-kristo@ti.com>
Reported-by: Andrew F. Davis <afd@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/omap-smp.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
--- a/arch/arm/mach-omap2/omap-smp.c
+++ b/arch/arm/mach-omap2/omap-smp.c
@@ -300,16 +300,6 @@ static void __init omap4_smp_prepare_cpus(unsigned int max_cpus)
 		scu_enable(cfg.scu_base);
 
 	/*
-	 * Reset CPU1 before configuring, otherwise kexec will
-	 * end up trying to use old kernel startup address.
-	 */
-	if (cfg.cpu1_rstctrl_va) {
-		writel_relaxed(1, cfg.cpu1_rstctrl_va);
-		readl_relaxed(cfg.cpu1_rstctrl_va);
-		writel_relaxed(0, cfg.cpu1_rstctrl_va);
-	}
-
-	/*
 	 * Write the address of secondary startup routine into the
 	 * AuxCoreBoot1 where ROM code will jump and start executing
 	 * on secondary core once out of WFE
-- 
2.11.1

^ permalink raw reply	[flat|nested] 58+ messages in thread
* [PATCH] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-13 20:52 ` Tony Lindgren
  0 siblings, 0 replies; 58+ messages in thread
From: Tony Lindgren @ 2017-03-13 20:52 UTC (permalink / raw)
  To: linux-omap
  Cc: Tero Kristo, Keerthy, Russell King, linux-arm-kernel, Andrew F . Davis

Commit 3251885285e1 ("ARM: OMAP4+: Reset CPU1 properly for kexec") started
unconditionally resetting CPU1 because of a kexec boot issue I was seeing
earlier on omap4 when doing kexec boot between two different kernel
versions.

This caused issues on some systems. We should only reset CPU1 as a last
resort option, and try to avoid it where possible. Doing an unconditional
CPU1 reset causes issues for example when booting a bootloader configured
secure OS running on CPU1 as reported by Andrew F. Davis <afd@ti.com>.

We can't completely remove the reset of CPU1 as it would break
kexec booting from older kernels. Also it looks like omap4 suspend/resume
cycle occasionally fails to bring up CPU1 without the reset of CPU1.

The ideal fix is to park CPU1 to the bootrom loop during CPU hot-unplug.
This way we can be assured that the CPU1 is properly configured by
bootloader or the previous kernel. However, we can't do that for the
earlier kernels out there capable of kexec booting new kernels.

So let's fix the issue reported by Andrew by making CPU1 reset conditional
and only do it if CPU1 is configured to boot at an address that is within
the booting kernel. In these cases we know for sure that we just overwrote
the configured boot_secondary() during booting and that it's invalid.

And while at it, let's replace legacy cpu_is usage with soc_is to avoid
confusion with mixed use of both checks.

Additionally we also need to fix the hot-unplug code to properly park CPU1
to the bootrom loop so it's not affected by SDRAM changes done by kexec
booting kernel.

Fixes: 3251885285e1 ("ARM: OMAP4+: Reset CPU1 properly for kexec")
Reported-by: Andrew F. Davis <afd@ti.com>
Cc: Andrew F. Davis <afd@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/common.h              |  1 +
 arch/arm/mach-omap2/omap-mpuss-lowpower.c | 30 +++++++++----
 arch/arm/mach-omap2/omap-smp.c            | 70 +++++++++++++++++++++++++------
 3 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -270,6 +270,7 @@ extern const struct smp_operations omap4_smp_ops;
 extern int omap4_mpuss_init(void);
 extern int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state);
 extern int omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state);
+extern u32 omap4_get_cpu1_ns_pa_addr(void);
 #else
 static inline int omap4_enter_lowpower(unsigned int cpu,
 					unsigned int power_state)
diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
@@ -64,6 +64,7 @@
 #include "prm-regbits-44xx.h"
 
 static void __iomem *sar_base;
+static u32 old_cpu1_ns_pa_addr;
 
 #if defined(CONFIG_PM) && defined(CONFIG_SMP)
 
@@ -212,6 +213,11 @@ static void __init save_l2x0_context(void)
 {}
 #endif
 
+u32 omap4_get_cpu1_ns_pa_addr(void)
+{
+	return old_cpu1_ns_pa_addr;
+}
+
 /**
  * omap4_enter_lowpower: OMAP4 MPUSS Low Power Entry Function
  * The purpose of this function is to manage low power programming
@@ -371,7 +377,7 @@ int __init omap4_mpuss_init(void)
 	pm_info = &per_cpu(omap4_pm_info, 0x0);
 	if (sar_base) {
 		pm_info->scu_sar_addr = sar_base + SCU_OFFSET0;
-		if (cpu_is_omap44xx())
+		if (soc_is_omap44xx())
 			pm_info->wkup_sar_addr = sar_base +
 				CPU0_WAKEUP_NS_PA_ADDR_OFFSET;
 		else
@@ -395,7 +401,7 @@ int __init omap4_mpuss_init(void)
 	pm_info = &per_cpu(omap4_pm_info, 0x1);
 	if (sar_base) {
 		pm_info->scu_sar_addr = sar_base + SCU_OFFSET1;
-		if (cpu_is_omap44xx())
+		if (soc_is_omap44xx())
 			pm_info->wkup_sar_addr = sar_base +
 				CPU1_WAKEUP_NS_PA_ADDR_OFFSET;
 		else
@@ -432,7 +438,7 @@ int __init omap4_mpuss_init(void)
 		save_l2x0_context();
 	}
 
-	if (cpu_is_omap44xx()) {
+	if (soc_is_omap44xx()) {
 		omap_pm_ops.finish_suspend = omap4_finish_suspend;
 		omap_pm_ops.resume = omap4_cpu_resume;
 		omap_pm_ops.scu_prepare = scu_pwrst_prepare;
@@ -443,7 +449,7 @@ int __init omap4_mpuss_init(void)
 		enable_mercury_retention_mode();
 	}
 
-	if (cpu_is_omap446x())
+	if (soc_is_omap446x())
 		omap_pm_ops.hotplug_restart = omap4460_secondary_startup;
 
 	return 0;
@@ -460,22 +466,30 @@ int __init omap4_mpuss_init(void)
 void __init omap4_mpuss_early_init(void)
 {
 	unsigned long startup_pa;
+	void __iomem *ns_pa_addr;
 
-	if (!(cpu_is_omap44xx() || soc_is_omap54xx()))
+	if (!(soc_is_omap44xx() || soc_is_omap54xx()))
 		return;
 
 	sar_base = omap4_get_sar_ram_base();
 
-	if (cpu_is_omap443x())
+	/* Save old NS_PA_ADDR for validity checks later on */
+	if (soc_is_omap44xx())
+		ns_pa_addr = sar_base + CPU1_WAKEUP_NS_PA_ADDR_OFFSET;
+	else
+		ns_pa_addr = sar_base + OMAP5_CPU1_WAKEUP_NS_PA_ADDR_OFFSET;
+	old_cpu1_ns_pa_addr = readl_relaxed(ns_pa_addr);
+
+	if (soc_is_omap443x())
 		startup_pa = __pa_symbol(omap4_secondary_startup);
-	else if (cpu_is_omap446x())
+	else if (soc_is_omap446x())
 		startup_pa = __pa_symbol(omap4460_secondary_startup);
 	else if ((__boot_cpu_mode & MODE_MASK) == HYP_MODE)
 		startup_pa = __pa_symbol(omap5_secondary_hyp_startup);
 	else
 		startup_pa = __pa_symbol(omap5_secondary_startup);
 
-	if (cpu_is_omap44xx())
+	if (soc_is_omap44xx())
 		writel_relaxed(startup_pa, sar_base +
 			       CPU1_WAKEUP_NS_PA_ADDR_OFFSET);
 	else
diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
--- a/arch/arm/mach-omap2/omap-smp.c
+++ b/arch/arm/mach-omap2/omap-smp.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/irqchip/arm-gic.h>
 
+#include <asm/sections.h>
 #include <asm/smp_scu.h>
 #include <asm/virt.h>
 
@@ -44,6 +45,7 @@ struct omap_smp_config {
 	unsigned long cpu1_rstctrl_pa;
 	void __iomem *cpu1_rstctrl_va;
 	void __iomem *scu_base;
+	void __iomem *wakeupgen_base;
 	void *startup_addr;
 };
 
@@ -140,7 +142,6 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	static struct clockdomain *cpu1_clkdm;
 	static bool booted;
 	static struct powerdomain *cpu1_pwrdm;
-	void __iomem *base = omap_get_wakeupgen_base();
 
 	/*
 	 * Set synchronisation state between this boot processor
@@ -157,7 +158,7 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	if (omap_secure_apis_support())
 		omap_modify_auxcoreboot0(0x200, 0xfffffdff);
 	else
-		writel_relaxed(0x20, base + OMAP_AUX_CORE_BOOT_0);
+		writel_relaxed(0x20, cfg.wakeupgen_base + OMAP_AUX_CORE_BOOT_0);
 
 	if (!cpu1_clkdm && !cpu1_pwrdm) {
 		cpu1_clkdm = clkdm_lookup("mpu1_clkdm");
@@ -261,9 +262,59 @@ static void __init omap4_smp_init_cpus(void)
 		set_cpu_possible(i, true);
 }
 
+/*
+ * For now, just make sure the start-up address is not within the booting
+ * kernel space as that means we just overwrote whatever secondary_startup()
+ * code there was.
+ */
+static bool __init omap4_smp_cpu1_startup_valid(unsigned long addr)
+{
+	if ((addr >= __pa(PAGE_OFFSET)) && (addr <= __pa(__bss_start)))
+		return false;
+
+	return true;
+}
+
+/*
+ * We may need to reset CPU1 before configuring, otherwise kexec boot can end
+ * up trying to use old kernel startup address or suspend-resume will
+ * occasionally fail to bring up CPU1 on 4430 if CPU1 fails to enter deeper
+ * idle states.
+ */
+static void __init omap4_smp_maybe_reset_cpu1(struct omap_smp_config *c)
+{
+	unsigned long cpu1_startup_pa, cpu1_ns_pa_addr;
+	bool needs_reset = false;
+
+	cpu1_startup_pa = readl_relaxed(cfg.wakeupgen_base +
+					OMAP_AUX_CORE_BOOT_1);
+	cpu1_ns_pa_addr = omap4_get_cpu1_ns_pa_addr();
+
+	/* Did the configured secondary_startup() get overwritten? */
+	if (!omap4_smp_cpu1_startup_valid(cpu1_startup_pa))
+		needs_reset = true;
+
+	/*
+	 * If omap4 or 5 has NS_PA_ADDR configured, CPU1 may be in a
+	 * deeper idle state in WFI and wake to an invalid address.
+	 */
+	if ((soc_is_omap44xx() || soc_is_omap54xx()) &&
+	    !omap4_smp_cpu1_startup_valid(cpu1_ns_pa_addr))
+		needs_reset = true;
+
+	if (!needs_reset || !c->cpu1_rstctrl_va)
+		return;
+
+	pr_info("smp: Already configured CPU1, needs reset (0x%lx 0x%lx)\n",
+		cpu1_startup_pa, cpu1_ns_pa_addr);
+
+	writel_relaxed(1, c->cpu1_rstctrl_va);
+	readl_relaxed(c->cpu1_rstctrl_va);
+	writel_relaxed(0, c->cpu1_rstctrl_va);
+}
+
 static void __init omap4_smp_prepare_cpus(unsigned int max_cpus)
 {
-	void __iomem *base = omap_get_wakeupgen_base();
 	const struct omap_smp_config *c = NULL;
 
 	if (soc_is_omap443x())
@@ -281,6 +332,7 @@ static void __init omap4_smp_prepare_cpus(unsigned int max_cpus)
 	/* Must preserve cfg.scu_base set earlier */
 	cfg.cpu1_rstctrl_pa = c->cpu1_rstctrl_pa;
 	cfg.startup_addr = c->startup_addr;
+	cfg.wakeupgen_base = omap_get_wakeupgen_base();
 
 	if (soc_is_dra74x() || soc_is_omap54xx()) {
 		if ((__boot_cpu_mode & MODE_MASK) == HYP_MODE)
@@ -299,15 +351,7 @@ static void __init omap4_smp_prepare_cpus(unsigned int max_cpus)
 	if (cfg.scu_base)
 		scu_enable(cfg.scu_base);
 
-	/*
-	 * Reset CPU1 before configuring, otherwise kexec will
-	 * end up trying to use old kernel startup address.
-	 */
-	if (cfg.cpu1_rstctrl_va) {
-		writel_relaxed(1, cfg.cpu1_rstctrl_va);
-		readl_relaxed(cfg.cpu1_rstctrl_va);
-		writel_relaxed(0, cfg.cpu1_rstctrl_va);
-	}
+	omap4_smp_maybe_reset_cpu1(&cfg);
 
 	/*
 	 * Write the address of secondary startup routine into the
@@ -319,7 +363,7 @@ static void __init omap4_smp_prepare_cpus(unsigned int max_cpus)
 		omap_auxcoreboot_addr(__pa_symbol(cfg.startup_addr));
 	else
 		writel_relaxed(__pa_symbol(cfg.startup_addr),
-			       base + OMAP_AUX_CORE_BOOT_1);
+			       cfg.wakeupgen_base + OMAP_AUX_CORE_BOOT_1);
 }
 
 const struct smp_operations omap4_smp_ops __initconst = {
-- 
2.11.1

^ permalink raw reply	[flat|nested] 58+ messages in thread
end of thread, other threads:[~2017-03-22 17:57 UTC | newest]

Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-13 21:50 [PATCH] ARM: omap2+: Revert omap-smp.c changes resetting cpu1 during boot Tony Lindgren
2017-02-13 21:50 ` Tony Lindgren
2017-02-14 19:36 ` Tony Lindgren
2017-02-14 19:36   ` Tony Lindgren
2017-02-15 18:39   ` Tony Lindgren
2017-02-15 18:39     ` Tony Lindgren
2017-02-15 19:12     ` Tony Lindgren
2017-02-15 19:12       ` Tony Lindgren
2017-02-15 22:13       ` Andrew F. Davis
2017-02-15 22:13         ` Andrew F. Davis
2017-02-15 22:27         ` Tony Lindgren
2017-02-15 22:27           ` Tony Lindgren
2017-02-16 16:10           ` Tony Lindgren
2017-02-16 16:10             ` Tony Lindgren
2017-02-16 16:21             ` Tony Lindgren
2017-02-16 16:21               ` Tony Lindgren
2017-02-16 16:29             ` Andrew F. Davis
2017-02-16 16:29               ` Andrew F. Davis
2017-02-16 16:54               ` Tony Lindgren
2017-02-16 16:54                 ` Tony Lindgren
2017-02-16 19:07                 ` Tony Lindgren
2017-02-16 19:07                   ` Tony Lindgren
2017-02-17 15:55                   ` Tony Lindgren
2017-02-17 15:55                     ` Tony Lindgren
2017-02-17 20:27                     ` Andrew F. Davis
2017-02-17 20:27                       ` Andrew F. Davis
2017-02-17 21:09                       ` Tony Lindgren
2017-02-17 21:09                         ` Tony Lindgren
2017-03-13 20:52 [PATCH] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 " Tony Lindgren
2017-03-13 20:52 ` Tony Lindgren
2017-03-13 21:28 ` Andrew F. Davis
2017-03-13 21:28   ` Andrew F. Davis
2017-03-13 21:47   ` Tony Lindgren
2017-03-13 21:47     ` Tony Lindgren
2017-03-14  7:30 ` Tero Kristo
2017-03-14  7:30   ` Tero Kristo
2017-03-14 15:17   ` Tony Lindgren
2017-03-14 15:17     ` Tony Lindgren
2017-03-14 16:02     ` Andrew F. Davis
2017-03-14 16:02       ` Andrew F. Davis
2017-03-14 16:41       ` Tony Lindgren
2017-03-14 16:41         ` Tony Lindgren
2017-03-14 17:57         ` Andrew F. Davis
2017-03-14 17:57           ` Andrew F. Davis
2017-03-14 18:14           ` Tony Lindgren
2017-03-14 18:14             ` Tony Lindgren
2017-03-15 17:22             ` Tony Lindgren
2017-03-15 17:22               ` Tony Lindgren
2017-03-16 15:29               ` Tony Lindgren
2017-03-16 15:29                 ` Tony Lindgren
2017-03-17  9:24                 ` Russell King - ARM Linux
2017-03-17  9:24                   ` Russell King - ARM Linux
2017-03-17 13:57                   ` Tony Lindgren
2017-03-17 13:57                     ` Tony Lindgren
2017-03-17 16:25                     ` Andrew F. Davis
2017-03-17 16:25                       ` Andrew F. Davis
2017-03-22 17:57                       ` Tony Lindgren
2017-03-22 17:57                         ` Tony Lindgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.