* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-14 18:05 ` Tony Lindgren
0 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-14 18:05 UTC (permalink / raw)
To: linux-omap
Cc: Keerthy, Andrew F . Davis, Tero Kristo, Russell King,
Santosh Shilimkar, linux-arm-kernel
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. But we can limit the CPU1 reset to cases
where CPU1 is wrongly parked within the memory area used by the booting
kernel. Then later on we can add support for parking CPU1 for kexec out
of the SDRAM back to bootrom.
So let's first fix the regression reported by Andrew by making CPU1 reset
conditional. To do this, we need to:
1. Save configured AUX_CORE_BOOT_1 for later
2. Modify AUX_CORE_BOOT_0 reading code to for HS SoCs to return
the whole register instead of the CPU mask
3. Check if CPU1 is wrongly parked into the booting kernel by the
previous kernel and reset if needed
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: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Changes from v2:
- Add Santosh to Cc, sorry Santosh I forgot to add earlier
- Add check for CPU1 being parked in addition to checking
CPU1 start-up address being within booting kernel before
reset
- Left out extra cpu_is to soc_is changes, those can be done
later
- Updated description to leave out suspend/resume issues as
those can be dealt with separately if they still exist after
fixing kexec boot, they may have been happening only after
a kexec boot
Changes from v1:
- We can't remove the CPU1 reset completely because it breaks
kexec booting all the existing kernels as CPU1 might be
parked into the booting kernel
---
arch/arm/mach-omap2/common.h | 1 +
arch/arm/mach-omap2/omap-hotplug.c | 2 +-
arch/arm/mach-omap2/omap-mpuss-lowpower.c | 22 ++++++--
arch/arm/mach-omap2/omap-smc.S | 1 -
arch/arm/mach-omap2/omap-smp.c | 90 ++++++++++++++++++++++++++-----
5 files changed, 96 insertions(+), 20 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-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
--- a/arch/arm/mach-omap2/omap-hotplug.c
+++ b/arch/arm/mach-omap2/omap-hotplug.c
@@ -50,7 +50,7 @@ void omap4_cpu_die(unsigned int cpu)
omap4_hotplug_cpu(cpu, PWRDM_POWER_OFF);
if (omap_secure_apis_support())
- boot_cpu = omap_read_auxcoreboot0();
+ boot_cpu = omap_read_auxcoreboot0() >> 9;
else
boot_cpu =
readl_relaxed(base + OMAP_AUX_CORE_BOOT_0) >> 5;
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
@@ -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-smc.S b/arch/arm/mach-omap2/omap-smc.S
--- a/arch/arm/mach-omap2/omap-smc.S
+++ b/arch/arm/mach-omap2/omap-smc.S
@@ -94,6 +94,5 @@ ENTRY(omap_read_auxcoreboot0)
ldr r12, =0x103
dsb
smc #0
- mov r0, r0, lsr #9
ldmfd sp!, {r2-r12, pc}
ENDPROC(omap_read_auxcoreboot0)
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>
@@ -40,10 +41,14 @@
#define OMAP5_CORE_COUNT 0x2
+#define AUX_CORE_BOOT0_GP_RELEASE 0x020
+#define AUX_CORE_BOOT0_HS_RELEASE 0x200
+
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 +145,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
@@ -155,9 +159,11 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
* A barrier is added to ensure that write buffer is drained
*/
if (omap_secure_apis_support())
- omap_modify_auxcoreboot0(0x200, 0xfffffdff);
+ omap_modify_auxcoreboot0(AUX_CORE_BOOT0_HS_RELEASE,
+ 0xfffffdff);
else
- writel_relaxed(0x20, base + OMAP_AUX_CORE_BOOT_0);
+ writel_relaxed(AUX_CORE_BOOT0_GP_RELEASE,
+ cfg.wakeupgen_base + OMAP_AUX_CORE_BOOT_0);
if (!cpu1_clkdm && !cpu1_pwrdm) {
cpu1_clkdm = clkdm_lookup("mpu1_clkdm");
@@ -261,9 +267,72 @@ 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;
+ u32 released;
+
+ if (omap_secure_apis_support())
+ released = omap_read_auxcoreboot0() & AUX_CORE_BOOT0_HS_RELEASE;
+ else
+ released = readl_relaxed(cfg.wakeupgen_base +
+ OMAP_AUX_CORE_BOOT_0) &
+ AUX_CORE_BOOT0_GP_RELEASE;
+ if (released) {
+ pr_warn("smp: CPU1 not parked?\n");
+
+ return;
+ }
+
+ 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 will 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: CPU1 parked within kernel, 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 +350,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 +369,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 +381,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] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-14 18:05 ` Tony Lindgren
0 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-14 18:05 UTC (permalink / raw)
To: linux-arm-kernel
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. But we can limit the CPU1 reset to cases
where CPU1 is wrongly parked within the memory area used by the booting
kernel. Then later on we can add support for parking CPU1 for kexec out
of the SDRAM back to bootrom.
So let's first fix the regression reported by Andrew by making CPU1 reset
conditional. To do this, we need to:
1. Save configured AUX_CORE_BOOT_1 for later
2. Modify AUX_CORE_BOOT_0 reading code to for HS SoCs to return
the whole register instead of the CPU mask
3. Check if CPU1 is wrongly parked into the booting kernel by the
previous kernel and reset if needed
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: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Changes from v2:
- Add Santosh to Cc, sorry Santosh I forgot to add earlier
- Add check for CPU1 being parked in addition to checking
CPU1 start-up address being within booting kernel before
reset
- Left out extra cpu_is to soc_is changes, those can be done
later
- Updated description to leave out suspend/resume issues as
those can be dealt with separately if they still exist after
fixing kexec boot, they may have been happening only after
a kexec boot
Changes from v1:
- We can't remove the CPU1 reset completely because it breaks
kexec booting all the existing kernels as CPU1 might be
parked into the booting kernel
---
arch/arm/mach-omap2/common.h | 1 +
arch/arm/mach-omap2/omap-hotplug.c | 2 +-
arch/arm/mach-omap2/omap-mpuss-lowpower.c | 22 ++++++--
arch/arm/mach-omap2/omap-smc.S | 1 -
arch/arm/mach-omap2/omap-smp.c | 90 ++++++++++++++++++++++++++-----
5 files changed, 96 insertions(+), 20 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-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
--- a/arch/arm/mach-omap2/omap-hotplug.c
+++ b/arch/arm/mach-omap2/omap-hotplug.c
@@ -50,7 +50,7 @@ void omap4_cpu_die(unsigned int cpu)
omap4_hotplug_cpu(cpu, PWRDM_POWER_OFF);
if (omap_secure_apis_support())
- boot_cpu = omap_read_auxcoreboot0();
+ boot_cpu = omap_read_auxcoreboot0() >> 9;
else
boot_cpu =
readl_relaxed(base + OMAP_AUX_CORE_BOOT_0) >> 5;
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
@@ -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-smc.S b/arch/arm/mach-omap2/omap-smc.S
--- a/arch/arm/mach-omap2/omap-smc.S
+++ b/arch/arm/mach-omap2/omap-smc.S
@@ -94,6 +94,5 @@ ENTRY(omap_read_auxcoreboot0)
ldr r12, =0x103
dsb
smc #0
- mov r0, r0, lsr #9
ldmfd sp!, {r2-r12, pc}
ENDPROC(omap_read_auxcoreboot0)
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>
@@ -40,10 +41,14 @@
#define OMAP5_CORE_COUNT 0x2
+#define AUX_CORE_BOOT0_GP_RELEASE 0x020
+#define AUX_CORE_BOOT0_HS_RELEASE 0x200
+
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 +145,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
@@ -155,9 +159,11 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
* A barrier is added to ensure that write buffer is drained
*/
if (omap_secure_apis_support())
- omap_modify_auxcoreboot0(0x200, 0xfffffdff);
+ omap_modify_auxcoreboot0(AUX_CORE_BOOT0_HS_RELEASE,
+ 0xfffffdff);
else
- writel_relaxed(0x20, base + OMAP_AUX_CORE_BOOT_0);
+ writel_relaxed(AUX_CORE_BOOT0_GP_RELEASE,
+ cfg.wakeupgen_base + OMAP_AUX_CORE_BOOT_0);
if (!cpu1_clkdm && !cpu1_pwrdm) {
cpu1_clkdm = clkdm_lookup("mpu1_clkdm");
@@ -261,9 +267,72 @@ 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;
+ u32 released;
+
+ if (omap_secure_apis_support())
+ released = omap_read_auxcoreboot0() & AUX_CORE_BOOT0_HS_RELEASE;
+ else
+ released = readl_relaxed(cfg.wakeupgen_base +
+ OMAP_AUX_CORE_BOOT_0) &
+ AUX_CORE_BOOT0_GP_RELEASE;
+ if (released) {
+ pr_warn("smp: CPU1 not parked?\n");
+
+ return;
+ }
+
+ 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 will 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: CPU1 parked within kernel, 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 +350,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 +369,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 +381,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] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-14 18:05 ` Tony Lindgren
@ 2017-03-15 9:41 ` Keerthy
-1 siblings, 0 replies; 24+ messages in thread
From: Keerthy @ 2017-03-15 9:41 UTC (permalink / raw)
To: Tony Lindgren, linux-omap
Cc: Tero Kristo, Russell King, Santosh Shilimkar, linux-arm-kernel,
Andrew F . Davis
On Tuesday 14 March 2017 11:35 PM, Tony Lindgren wrote:
> 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. But we can limit the CPU1 reset to cases
> where CPU1 is wrongly parked within the memory area used by the booting
> kernel. Then later on we can add support for parking CPU1 for kexec out
> of the SDRAM back to bootrom.
>
> So let's first fix the regression reported by Andrew by making CPU1 reset
> conditional. To do this, we need to:
>
> 1. Save configured AUX_CORE_BOOT_1 for later
>
> 2. Modify AUX_CORE_BOOT_0 reading code to for HS SoCs to return
> the whole register instead of the CPU mask
>
> 3. Check if CPU1 is wrongly parked into the booting kernel by the
> previous kernel and reset if needed
Tested on top of:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/thread.html#491706
kexec seems working fine with both CPUs booting HYP mode on DRA7-EVM.
Tested-by: Keerthy <j-keerthy@ti.com>
>
> 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: Santosh Shilimkar <ssantosh@kernel.org>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> ---
>
> Changes from v2:
>
> - Add Santosh to Cc, sorry Santosh I forgot to add earlier
>
> - Add check for CPU1 being parked in addition to checking
> CPU1 start-up address being within booting kernel before
> reset
>
> - Left out extra cpu_is to soc_is changes, those can be done
> later
>
> - Updated description to leave out suspend/resume issues as
> those can be dealt with separately if they still exist after
> fixing kexec boot, they may have been happening only after
> a kexec boot
>
> Changes from v1:
>
> - We can't remove the CPU1 reset completely because it breaks
> kexec booting all the existing kernels as CPU1 might be
> parked into the booting kernel
>
> ---
> arch/arm/mach-omap2/common.h | 1 +
> arch/arm/mach-omap2/omap-hotplug.c | 2 +-
> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 22 ++++++--
> arch/arm/mach-omap2/omap-smc.S | 1 -
> arch/arm/mach-omap2/omap-smp.c | 90 ++++++++++++++++++++++++++-----
> 5 files changed, 96 insertions(+), 20 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-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> --- a/arch/arm/mach-omap2/omap-hotplug.c
> +++ b/arch/arm/mach-omap2/omap-hotplug.c
> @@ -50,7 +50,7 @@ void omap4_cpu_die(unsigned int cpu)
> omap4_hotplug_cpu(cpu, PWRDM_POWER_OFF);
>
> if (omap_secure_apis_support())
> - boot_cpu = omap_read_auxcoreboot0();
> + boot_cpu = omap_read_auxcoreboot0() >> 9;
> else
> boot_cpu =
> readl_relaxed(base + OMAP_AUX_CORE_BOOT_0) >> 5;
> 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
> @@ -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-smc.S b/arch/arm/mach-omap2/omap-smc.S
> --- a/arch/arm/mach-omap2/omap-smc.S
> +++ b/arch/arm/mach-omap2/omap-smc.S
> @@ -94,6 +94,5 @@ ENTRY(omap_read_auxcoreboot0)
> ldr r12, =0x103
> dsb
> smc #0
> - mov r0, r0, lsr #9
> ldmfd sp!, {r2-r12, pc}
> ENDPROC(omap_read_auxcoreboot0)
> 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>
>
> @@ -40,10 +41,14 @@
>
> #define OMAP5_CORE_COUNT 0x2
>
> +#define AUX_CORE_BOOT0_GP_RELEASE 0x020
> +#define AUX_CORE_BOOT0_HS_RELEASE 0x200
> +
> 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 +145,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
> @@ -155,9 +159,11 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * A barrier is added to ensure that write buffer is drained
> */
> if (omap_secure_apis_support())
> - omap_modify_auxcoreboot0(0x200, 0xfffffdff);
> + omap_modify_auxcoreboot0(AUX_CORE_BOOT0_HS_RELEASE,
> + 0xfffffdff);
> else
> - writel_relaxed(0x20, base + OMAP_AUX_CORE_BOOT_0);
> + writel_relaxed(AUX_CORE_BOOT0_GP_RELEASE,
> + cfg.wakeupgen_base + OMAP_AUX_CORE_BOOT_0);
>
> if (!cpu1_clkdm && !cpu1_pwrdm) {
> cpu1_clkdm = clkdm_lookup("mpu1_clkdm");
> @@ -261,9 +267,72 @@ 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;
> + u32 released;
> +
> + if (omap_secure_apis_support())
> + released = omap_read_auxcoreboot0() & AUX_CORE_BOOT0_HS_RELEASE;
> + else
> + released = readl_relaxed(cfg.wakeupgen_base +
> + OMAP_AUX_CORE_BOOT_0) &
> + AUX_CORE_BOOT0_GP_RELEASE;
> + if (released) {
> + pr_warn("smp: CPU1 not parked?\n");
> +
> + return;
> + }
> +
> + 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 will 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: CPU1 parked within kernel, 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 +350,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 +369,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 +381,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 = {
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-15 9:41 ` Keerthy
0 siblings, 0 replies; 24+ messages in thread
From: Keerthy @ 2017-03-15 9:41 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 14 March 2017 11:35 PM, Tony Lindgren wrote:
> 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. But we can limit the CPU1 reset to cases
> where CPU1 is wrongly parked within the memory area used by the booting
> kernel. Then later on we can add support for parking CPU1 for kexec out
> of the SDRAM back to bootrom.
>
> So let's first fix the regression reported by Andrew by making CPU1 reset
> conditional. To do this, we need to:
>
> 1. Save configured AUX_CORE_BOOT_1 for later
>
> 2. Modify AUX_CORE_BOOT_0 reading code to for HS SoCs to return
> the whole register instead of the CPU mask
>
> 3. Check if CPU1 is wrongly parked into the booting kernel by the
> previous kernel and reset if needed
Tested on top of:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/thread.html#491706
kexec seems working fine with both CPUs booting HYP mode on DRA7-EVM.
Tested-by: Keerthy <j-keerthy@ti.com>
>
> 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: Santosh Shilimkar <ssantosh@kernel.org>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> ---
>
> Changes from v2:
>
> - Add Santosh to Cc, sorry Santosh I forgot to add earlier
>
> - Add check for CPU1 being parked in addition to checking
> CPU1 start-up address being within booting kernel before
> reset
>
> - Left out extra cpu_is to soc_is changes, those can be done
> later
>
> - Updated description to leave out suspend/resume issues as
> those can be dealt with separately if they still exist after
> fixing kexec boot, they may have been happening only after
> a kexec boot
>
> Changes from v1:
>
> - We can't remove the CPU1 reset completely because it breaks
> kexec booting all the existing kernels as CPU1 might be
> parked into the booting kernel
>
> ---
> arch/arm/mach-omap2/common.h | 1 +
> arch/arm/mach-omap2/omap-hotplug.c | 2 +-
> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 22 ++++++--
> arch/arm/mach-omap2/omap-smc.S | 1 -
> arch/arm/mach-omap2/omap-smp.c | 90 ++++++++++++++++++++++++++-----
> 5 files changed, 96 insertions(+), 20 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-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> --- a/arch/arm/mach-omap2/omap-hotplug.c
> +++ b/arch/arm/mach-omap2/omap-hotplug.c
> @@ -50,7 +50,7 @@ void omap4_cpu_die(unsigned int cpu)
> omap4_hotplug_cpu(cpu, PWRDM_POWER_OFF);
>
> if (omap_secure_apis_support())
> - boot_cpu = omap_read_auxcoreboot0();
> + boot_cpu = omap_read_auxcoreboot0() >> 9;
> else
> boot_cpu =
> readl_relaxed(base + OMAP_AUX_CORE_BOOT_0) >> 5;
> 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
> @@ -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-smc.S b/arch/arm/mach-omap2/omap-smc.S
> --- a/arch/arm/mach-omap2/omap-smc.S
> +++ b/arch/arm/mach-omap2/omap-smc.S
> @@ -94,6 +94,5 @@ ENTRY(omap_read_auxcoreboot0)
> ldr r12, =0x103
> dsb
> smc #0
> - mov r0, r0, lsr #9
> ldmfd sp!, {r2-r12, pc}
> ENDPROC(omap_read_auxcoreboot0)
> 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>
>
> @@ -40,10 +41,14 @@
>
> #define OMAP5_CORE_COUNT 0x2
>
> +#define AUX_CORE_BOOT0_GP_RELEASE 0x020
> +#define AUX_CORE_BOOT0_HS_RELEASE 0x200
> +
> 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 +145,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
> @@ -155,9 +159,11 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * A barrier is added to ensure that write buffer is drained
> */
> if (omap_secure_apis_support())
> - omap_modify_auxcoreboot0(0x200, 0xfffffdff);
> + omap_modify_auxcoreboot0(AUX_CORE_BOOT0_HS_RELEASE,
> + 0xfffffdff);
> else
> - writel_relaxed(0x20, base + OMAP_AUX_CORE_BOOT_0);
> + writel_relaxed(AUX_CORE_BOOT0_GP_RELEASE,
> + cfg.wakeupgen_base + OMAP_AUX_CORE_BOOT_0);
>
> if (!cpu1_clkdm && !cpu1_pwrdm) {
> cpu1_clkdm = clkdm_lookup("mpu1_clkdm");
> @@ -261,9 +267,72 @@ 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;
> + u32 released;
> +
> + if (omap_secure_apis_support())
> + released = omap_read_auxcoreboot0() & AUX_CORE_BOOT0_HS_RELEASE;
> + else
> + released = readl_relaxed(cfg.wakeupgen_base +
> + OMAP_AUX_CORE_BOOT_0) &
> + AUX_CORE_BOOT0_GP_RELEASE;
> + if (released) {
> + pr_warn("smp: CPU1 not parked?\n");
> +
> + return;
> + }
> +
> + 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 will 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: CPU1 parked within kernel, 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 +350,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 +369,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 +381,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 = {
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-14 18:05 ` Tony Lindgren
@ 2017-03-27 16:33 ` Andrew F. Davis
-1 siblings, 0 replies; 24+ messages in thread
From: Andrew F. Davis @ 2017-03-27 16:33 UTC (permalink / raw)
To: Tony Lindgren, linux-omap
Cc: Tero Kristo, Keerthy, Russell King, linux-arm-kernel, Santosh Shilimkar
On 03/14/2017 01:05 PM, Tony Lindgren wrote:
> 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. But we can limit the CPU1 reset to cases
> where CPU1 is wrongly parked within the memory area used by the booting
> kernel. Then later on we can add support for parking CPU1 for kexec out
> of the SDRAM back to bootrom.
>
> So let's first fix the regression reported by Andrew by making CPU1 reset
> conditional. To do this, we need to:
>
> 1. Save configured AUX_CORE_BOOT_1 for later
>
> 2. Modify AUX_CORE_BOOT_0 reading code to for HS SoCs to return
> the whole register instead of the CPU mask
>
> 3. Check if CPU1 is wrongly parked into the booting kernel by the
> previous kernel and reset if needed
>
I still don't like this style of workaround, if kexec cannot regain
control of core1 without a hard core reset than kexec should BUG() and
force the user to reboot to a sane state.
Anyway, this patch does remove the unconditional reset and fixes my
use-case is some situations (when not using kexec) so:
Tested-by: Andrew F. Davis <afd@ti.com>
> 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: Santosh Shilimkar <ssantosh@kernel.org>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> ---
>
> Changes from v2:
>
> - Add Santosh to Cc, sorry Santosh I forgot to add earlier
>
> - Add check for CPU1 being parked in addition to checking
> CPU1 start-up address being within booting kernel before
> reset
>
> - Left out extra cpu_is to soc_is changes, those can be done
> later
>
> - Updated description to leave out suspend/resume issues as
> those can be dealt with separately if they still exist after
> fixing kexec boot, they may have been happening only after
> a kexec boot
>
> Changes from v1:
>
> - We can't remove the CPU1 reset completely because it breaks
> kexec booting all the existing kernels as CPU1 might be
> parked into the booting kernel
>
> ---
> arch/arm/mach-omap2/common.h | 1 +
> arch/arm/mach-omap2/omap-hotplug.c | 2 +-
> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 22 ++++++--
> arch/arm/mach-omap2/omap-smc.S | 1 -
> arch/arm/mach-omap2/omap-smp.c | 90 ++++++++++++++++++++++++++-----
> 5 files changed, 96 insertions(+), 20 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-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> --- a/arch/arm/mach-omap2/omap-hotplug.c
> +++ b/arch/arm/mach-omap2/omap-hotplug.c
> @@ -50,7 +50,7 @@ void omap4_cpu_die(unsigned int cpu)
> omap4_hotplug_cpu(cpu, PWRDM_POWER_OFF);
>
> if (omap_secure_apis_support())
> - boot_cpu = omap_read_auxcoreboot0();
> + boot_cpu = omap_read_auxcoreboot0() >> 9;
> else
> boot_cpu =
> readl_relaxed(base + OMAP_AUX_CORE_BOOT_0) >> 5;
> 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
> @@ -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-smc.S b/arch/arm/mach-omap2/omap-smc.S
> --- a/arch/arm/mach-omap2/omap-smc.S
> +++ b/arch/arm/mach-omap2/omap-smc.S
> @@ -94,6 +94,5 @@ ENTRY(omap_read_auxcoreboot0)
> ldr r12, =0x103
> dsb
> smc #0
> - mov r0, r0, lsr #9
> ldmfd sp!, {r2-r12, pc}
> ENDPROC(omap_read_auxcoreboot0)
> 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>
>
> @@ -40,10 +41,14 @@
>
> #define OMAP5_CORE_COUNT 0x2
>
> +#define AUX_CORE_BOOT0_GP_RELEASE 0x020
> +#define AUX_CORE_BOOT0_HS_RELEASE 0x200
> +
> 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 +145,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
> @@ -155,9 +159,11 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * A barrier is added to ensure that write buffer is drained
> */
> if (omap_secure_apis_support())
> - omap_modify_auxcoreboot0(0x200, 0xfffffdff);
> + omap_modify_auxcoreboot0(AUX_CORE_BOOT0_HS_RELEASE,
> + 0xfffffdff);
> else
> - writel_relaxed(0x20, base + OMAP_AUX_CORE_BOOT_0);
> + writel_relaxed(AUX_CORE_BOOT0_GP_RELEASE,
> + cfg.wakeupgen_base + OMAP_AUX_CORE_BOOT_0);
>
> if (!cpu1_clkdm && !cpu1_pwrdm) {
> cpu1_clkdm = clkdm_lookup("mpu1_clkdm");
> @@ -261,9 +267,72 @@ 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;
> + u32 released;
> +
> + if (omap_secure_apis_support())
> + released = omap_read_auxcoreboot0() & AUX_CORE_BOOT0_HS_RELEASE;
> + else
> + released = readl_relaxed(cfg.wakeupgen_base +
> + OMAP_AUX_CORE_BOOT_0) &
> + AUX_CORE_BOOT0_GP_RELEASE;
> + if (released) {
> + pr_warn("smp: CPU1 not parked?\n");
> +
> + return;
> + }
> +
> + 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 will 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: CPU1 parked within kernel, 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 +350,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 +369,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 +381,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 = {
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-27 16:33 ` Andrew F. Davis
0 siblings, 0 replies; 24+ messages in thread
From: Andrew F. Davis @ 2017-03-27 16:33 UTC (permalink / raw)
To: linux-arm-kernel
On 03/14/2017 01:05 PM, Tony Lindgren wrote:
> 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. But we can limit the CPU1 reset to cases
> where CPU1 is wrongly parked within the memory area used by the booting
> kernel. Then later on we can add support for parking CPU1 for kexec out
> of the SDRAM back to bootrom.
>
> So let's first fix the regression reported by Andrew by making CPU1 reset
> conditional. To do this, we need to:
>
> 1. Save configured AUX_CORE_BOOT_1 for later
>
> 2. Modify AUX_CORE_BOOT_0 reading code to for HS SoCs to return
> the whole register instead of the CPU mask
>
> 3. Check if CPU1 is wrongly parked into the booting kernel by the
> previous kernel and reset if needed
>
I still don't like this style of workaround, if kexec cannot regain
control of core1 without a hard core reset than kexec should BUG() and
force the user to reboot to a sane state.
Anyway, this patch does remove the unconditional reset and fixes my
use-case is some situations (when not using kexec) so:
Tested-by: Andrew F. Davis <afd@ti.com>
> 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: Santosh Shilimkar <ssantosh@kernel.org>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> ---
>
> Changes from v2:
>
> - Add Santosh to Cc, sorry Santosh I forgot to add earlier
>
> - Add check for CPU1 being parked in addition to checking
> CPU1 start-up address being within booting kernel before
> reset
>
> - Left out extra cpu_is to soc_is changes, those can be done
> later
>
> - Updated description to leave out suspend/resume issues as
> those can be dealt with separately if they still exist after
> fixing kexec boot, they may have been happening only after
> a kexec boot
>
> Changes from v1:
>
> - We can't remove the CPU1 reset completely because it breaks
> kexec booting all the existing kernels as CPU1 might be
> parked into the booting kernel
>
> ---
> arch/arm/mach-omap2/common.h | 1 +
> arch/arm/mach-omap2/omap-hotplug.c | 2 +-
> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 22 ++++++--
> arch/arm/mach-omap2/omap-smc.S | 1 -
> arch/arm/mach-omap2/omap-smp.c | 90 ++++++++++++++++++++++++++-----
> 5 files changed, 96 insertions(+), 20 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-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> --- a/arch/arm/mach-omap2/omap-hotplug.c
> +++ b/arch/arm/mach-omap2/omap-hotplug.c
> @@ -50,7 +50,7 @@ void omap4_cpu_die(unsigned int cpu)
> omap4_hotplug_cpu(cpu, PWRDM_POWER_OFF);
>
> if (omap_secure_apis_support())
> - boot_cpu = omap_read_auxcoreboot0();
> + boot_cpu = omap_read_auxcoreboot0() >> 9;
> else
> boot_cpu =
> readl_relaxed(base + OMAP_AUX_CORE_BOOT_0) >> 5;
> 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
> @@ -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-smc.S b/arch/arm/mach-omap2/omap-smc.S
> --- a/arch/arm/mach-omap2/omap-smc.S
> +++ b/arch/arm/mach-omap2/omap-smc.S
> @@ -94,6 +94,5 @@ ENTRY(omap_read_auxcoreboot0)
> ldr r12, =0x103
> dsb
> smc #0
> - mov r0, r0, lsr #9
> ldmfd sp!, {r2-r12, pc}
> ENDPROC(omap_read_auxcoreboot0)
> 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>
>
> @@ -40,10 +41,14 @@
>
> #define OMAP5_CORE_COUNT 0x2
>
> +#define AUX_CORE_BOOT0_GP_RELEASE 0x020
> +#define AUX_CORE_BOOT0_HS_RELEASE 0x200
> +
> 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 +145,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
> @@ -155,9 +159,11 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * A barrier is added to ensure that write buffer is drained
> */
> if (omap_secure_apis_support())
> - omap_modify_auxcoreboot0(0x200, 0xfffffdff);
> + omap_modify_auxcoreboot0(AUX_CORE_BOOT0_HS_RELEASE,
> + 0xfffffdff);
> else
> - writel_relaxed(0x20, base + OMAP_AUX_CORE_BOOT_0);
> + writel_relaxed(AUX_CORE_BOOT0_GP_RELEASE,
> + cfg.wakeupgen_base + OMAP_AUX_CORE_BOOT_0);
>
> if (!cpu1_clkdm && !cpu1_pwrdm) {
> cpu1_clkdm = clkdm_lookup("mpu1_clkdm");
> @@ -261,9 +267,72 @@ 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;
> + u32 released;
> +
> + if (omap_secure_apis_support())
> + released = omap_read_auxcoreboot0() & AUX_CORE_BOOT0_HS_RELEASE;
> + else
> + released = readl_relaxed(cfg.wakeupgen_base +
> + OMAP_AUX_CORE_BOOT_0) &
> + AUX_CORE_BOOT0_GP_RELEASE;
> + if (released) {
> + pr_warn("smp: CPU1 not parked?\n");
> +
> + return;
> + }
> +
> + 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 will 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: CPU1 parked within kernel, 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 +350,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 +369,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 +381,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 = {
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-27 16:33 ` Andrew F. Davis
@ 2017-03-27 16:43 ` Tony Lindgren
-1 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-27 16:43 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Keerthy, Tero Kristo, Russell King, Santosh Shilimkar,
linux-omap, linux-arm-kernel
* Andrew F. Davis <afd@ti.com> [170327 09:36]:
> On 03/14/2017 01:05 PM, Tony Lindgren wrote:
> > 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. But we can limit the CPU1 reset to cases
> > where CPU1 is wrongly parked within the memory area used by the booting
> > kernel. Then later on we can add support for parking CPU1 for kexec out
> > of the SDRAM back to bootrom.
> >
> > So let's first fix the regression reported by Andrew by making CPU1 reset
> > conditional. To do this, we need to:
> >
> > 1. Save configured AUX_CORE_BOOT_1 for later
> >
> > 2. Modify AUX_CORE_BOOT_0 reading code to for HS SoCs to return
> > the whole register instead of the CPU mask
> >
> > 3. Check if CPU1 is wrongly parked into the booting kernel by the
> > previous kernel and reset if needed
> >
>
> I still don't like this style of workaround, if kexec cannot regain
> control of core1 without a hard core reset than kexec should BUG() and
> force the user to reboot to a sane state.
Yes problems still remains. I think the immediate fix there is to
disable kexec during runtime based on some criteria for your use
case rather than BUG() though. Somehow kexec needs to know if CPU1
reset is acceptable, then reset CPU1 before kexec.
> Anyway, this patch does remove the unconditional reset and fixes my
> use-case is some situations (when not using kexec) so:
>
> Tested-by: Andrew F. Davis <afd@ti.com>
OK thanks for testing.
Regards,
Tony
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-27 16:43 ` Tony Lindgren
0 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-27 16:43 UTC (permalink / raw)
To: linux-arm-kernel
* Andrew F. Davis <afd@ti.com> [170327 09:36]:
> On 03/14/2017 01:05 PM, Tony Lindgren wrote:
> > 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. But we can limit the CPU1 reset to cases
> > where CPU1 is wrongly parked within the memory area used by the booting
> > kernel. Then later on we can add support for parking CPU1 for kexec out
> > of the SDRAM back to bootrom.
> >
> > So let's first fix the regression reported by Andrew by making CPU1 reset
> > conditional. To do this, we need to:
> >
> > 1. Save configured AUX_CORE_BOOT_1 for later
> >
> > 2. Modify AUX_CORE_BOOT_0 reading code to for HS SoCs to return
> > the whole register instead of the CPU mask
> >
> > 3. Check if CPU1 is wrongly parked into the booting kernel by the
> > previous kernel and reset if needed
> >
>
> I still don't like this style of workaround, if kexec cannot regain
> control of core1 without a hard core reset than kexec should BUG() and
> force the user to reboot to a sane state.
Yes problems still remains. I think the immediate fix there is to
disable kexec during runtime based on some criteria for your use
case rather than BUG() though. Somehow kexec needs to know if CPU1
reset is acceptable, then reset CPU1 before kexec.
> Anyway, this patch does remove the unconditional reset and fixes my
> use-case is some situations (when not using kexec) so:
>
> Tested-by: Andrew F. Davis <afd@ti.com>
OK thanks for testing.
Regards,
Tony
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-27 16:33 ` Andrew F. Davis
@ 2017-03-28 11:33 ` Russell King - ARM Linux
-1 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2017-03-28 11:33 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Tony Lindgren, Keerthy, Tero Kristo, Santosh Shilimkar,
linux-omap, linux-arm-kernel
On Mon, Mar 27, 2017 at 11:33:30AM -0500, Andrew F. Davis wrote:
> I still don't like this style of workaround, if kexec cannot regain
> control of core1 without a hard core reset than kexec should BUG() and
> force the user to reboot to a sane state.
No, because kexec might already be in progress, and that would create
an infinite loop of kexec -> bug -> kexec -> bug etc.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-28 11:33 ` Russell King - ARM Linux
0 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2017-03-28 11:33 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 27, 2017 at 11:33:30AM -0500, Andrew F. Davis wrote:
> I still don't like this style of workaround, if kexec cannot regain
> control of core1 without a hard core reset than kexec should BUG() and
> force the user to reboot to a sane state.
No, because kexec might already be in progress, and that would create
an infinite loop of kexec -> bug -> kexec -> bug etc.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-27 16:43 ` Tony Lindgren
@ 2017-03-28 11:36 ` Russell King - ARM Linux
-1 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2017-03-28 11:36 UTC (permalink / raw)
To: Tony Lindgren
Cc: Keerthy, Andrew F. Davis, Tero Kristo, Santosh Shilimkar,
linux-omap, linux-arm-kernel
On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
> Yes problems still remains. I think the immediate fix there is to
> disable kexec during runtime based on some criteria for your use
> case rather than BUG() though. Somehow kexec needs to know if CPU1
> reset is acceptable, then reset CPU1 before kexec.
The only acceptable way to do that is to make the decision when loading
the image(s), and refuse to load the image(s) if kexec is not possible.
(As is already done when dealing with whether we can CPU hot-unplug the
secondary CPUs.)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-28 11:36 ` Russell King - ARM Linux
0 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2017-03-28 11:36 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
> Yes problems still remains. I think the immediate fix there is to
> disable kexec during runtime based on some criteria for your use
> case rather than BUG() though. Somehow kexec needs to know if CPU1
> reset is acceptable, then reset CPU1 before kexec.
The only acceptable way to do that is to make the decision when loading
the image(s), and refuse to load the image(s) if kexec is not possible.
(As is already done when dealing with whether we can CPU hot-unplug the
secondary CPUs.)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-28 11:36 ` Russell King - ARM Linux
@ 2017-03-28 17:09 ` Tony Lindgren
-1 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-28 17:09 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Keerthy, Andrew F. Davis, Tero Kristo, Santosh Shilimkar,
linux-omap, linux-arm-kernel
* Russell King - ARM Linux <linux@armlinux.org.uk> [170328 04:39]:
> On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
> > Yes problems still remains. I think the immediate fix there is to
> > disable kexec during runtime based on some criteria for your use
> > case rather than BUG() though. Somehow kexec needs to know if CPU1
> > reset is acceptable, then reset CPU1 before kexec.
>
> The only acceptable way to do that is to make the decision when loading
> the image(s), and refuse to load the image(s) if kexec is not possible.
> (As is already done when dealing with whether we can CPU hot-unplug the
> secondary CPUs.)
OK so how about the following to disable kexec for Andrew's
test case?
That is assuming that omap_type() test works for HS dra7
like it does for the other omap2 variants.
Regards,
Tony
8< ---------------------
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
@@ -260,6 +260,7 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
extern void omap_auxcoreboot_addr(u32 cpu_addr);
extern u32 omap_read_auxcoreboot0(void);
+extern bool omap4_cpu_can_disable(unsigned int cpu);
extern void omap4_cpu_die(unsigned int cpu);
extern int omap4_cpu_kill(unsigned int cpu);
diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
--- a/arch/arm/mach-omap2/omap-hotplug.c
+++ b/arch/arm/mach-omap2/omap-hotplug.c
@@ -22,6 +22,15 @@
#include "omap-wakeupgen.h"
#include "common.h"
#include "powerdomain.h"
+#include "soc.h"
+
+bool omap4_cpu_can_disable(unsigned int cpu)
+{
+ if (soc_is_dra7xx() && (omap_type() != OMAP2_DEVICE_TYPE_GP))
+ return false;
+
+ return true;
+}
/*
* platform-specific code to shutdown a CPU
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
@@ -390,6 +390,7 @@ const struct smp_operations omap4_smp_ops __initconst = {
.smp_secondary_init = omap4_secondary_init,
.smp_boot_secondary = omap4_boot_secondary,
#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_can_disable = omap4_cpu_can_disable,
.cpu_die = omap4_cpu_die,
.cpu_kill = omap4_cpu_kill,
#endif
--
2.12.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-28 17:09 ` Tony Lindgren
0 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-28 17:09 UTC (permalink / raw)
To: linux-arm-kernel
* Russell King - ARM Linux <linux@armlinux.org.uk> [170328 04:39]:
> On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
> > Yes problems still remains. I think the immediate fix there is to
> > disable kexec during runtime based on some criteria for your use
> > case rather than BUG() though. Somehow kexec needs to know if CPU1
> > reset is acceptable, then reset CPU1 before kexec.
>
> The only acceptable way to do that is to make the decision when loading
> the image(s), and refuse to load the image(s) if kexec is not possible.
> (As is already done when dealing with whether we can CPU hot-unplug the
> secondary CPUs.)
OK so how about the following to disable kexec for Andrew's
test case?
That is assuming that omap_type() test works for HS dra7
like it does for the other omap2 variants.
Regards,
Tony
8< ---------------------
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
@@ -260,6 +260,7 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
extern void omap_auxcoreboot_addr(u32 cpu_addr);
extern u32 omap_read_auxcoreboot0(void);
+extern bool omap4_cpu_can_disable(unsigned int cpu);
extern void omap4_cpu_die(unsigned int cpu);
extern int omap4_cpu_kill(unsigned int cpu);
diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
--- a/arch/arm/mach-omap2/omap-hotplug.c
+++ b/arch/arm/mach-omap2/omap-hotplug.c
@@ -22,6 +22,15 @@
#include "omap-wakeupgen.h"
#include "common.h"
#include "powerdomain.h"
+#include "soc.h"
+
+bool omap4_cpu_can_disable(unsigned int cpu)
+{
+ if (soc_is_dra7xx() && (omap_type() != OMAP2_DEVICE_TYPE_GP))
+ return false;
+
+ return true;
+}
/*
* platform-specific code to shutdown a CPU
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
@@ -390,6 +390,7 @@ const struct smp_operations omap4_smp_ops __initconst = {
.smp_secondary_init = omap4_secondary_init,
.smp_boot_secondary = omap4_boot_secondary,
#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_can_disable = omap4_cpu_can_disable,
.cpu_die = omap4_cpu_die,
.cpu_kill = omap4_cpu_kill,
#endif
--
2.12.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-28 17:09 ` Tony Lindgren
@ 2017-03-28 17:52 ` Andrew F. Davis
-1 siblings, 0 replies; 24+ messages in thread
From: Andrew F. Davis @ 2017-03-28 17:52 UTC (permalink / raw)
To: Tony Lindgren, Russell King - ARM Linux
Cc: Tero Kristo, Keerthy, linux-omap, linux-arm-kernel, Santosh Shilimkar
On 03/28/2017 12:09 PM, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@armlinux.org.uk> [170328 04:39]:
>> On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
>>> Yes problems still remains. I think the immediate fix there is to
>>> disable kexec during runtime based on some criteria for your use
>>> case rather than BUG() though. Somehow kexec needs to know if CPU1
>>> reset is acceptable, then reset CPU1 before kexec.
>>
>> The only acceptable way to do that is to make the decision when loading
>> the image(s), and refuse to load the image(s) if kexec is not possible.
>> (As is already done when dealing with whether we can CPU hot-unplug the
>> secondary CPUs.)
>
> OK so how about the following to disable kexec for Andrew's
> test case?
>
> That is assuming that omap_type() test works for HS dra7
> like it does for the other omap2 variants.
>
> Regards,
>
> Tony
>
> 8< ---------------------
> 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
> @@ -260,6 +260,7 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
> extern void omap_auxcoreboot_addr(u32 cpu_addr);
> extern u32 omap_read_auxcoreboot0(void);
>
> +extern bool omap4_cpu_can_disable(unsigned int cpu);
> extern void omap4_cpu_die(unsigned int cpu);
> extern int omap4_cpu_kill(unsigned int cpu);
>
> diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> --- a/arch/arm/mach-omap2/omap-hotplug.c
> +++ b/arch/arm/mach-omap2/omap-hotplug.c
> @@ -22,6 +22,15 @@
> #include "omap-wakeupgen.h"
> #include "common.h"
> #include "powerdomain.h"
> +#include "soc.h"
> +
> +bool omap4_cpu_can_disable(unsigned int cpu)
> +{
> + if (soc_is_dra7xx() && (omap_type() != OMAP2_DEVICE_TYPE_GP))
> + return false;
> +
NAK!
HS variants can have aux cores disabled, we are already doing it right
now, this is another hack to avoid having to fix kexec.
You are failing to park cores correctly, so instead of fixing that you
hack around it on non-HS devices by hard-resetting those cores, now
because you can't do that hack on HS devices you suggest we disable
hotplug on HS devices altogether!?
Kexec *does* work on HS devices. It only fails if we fail to park a core
correctly, something that should never happen, the check here must be
for that, unconditionally disabling Kexec for all HS devices is not a
valid option.
> + return true;
> +}
>
> /*
> * platform-specific code to shutdown a CPU
> 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
> @@ -390,6 +390,7 @@ const struct smp_operations omap4_smp_ops __initconst = {
> .smp_secondary_init = omap4_secondary_init,
> .smp_boot_secondary = omap4_boot_secondary,
> #ifdef CONFIG_HOTPLUG_CPU
> + .cpu_can_disable = omap4_cpu_can_disable,
> .cpu_die = omap4_cpu_die,
> .cpu_kill = omap4_cpu_kill,
> #endif
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-28 17:52 ` Andrew F. Davis
0 siblings, 0 replies; 24+ messages in thread
From: Andrew F. Davis @ 2017-03-28 17:52 UTC (permalink / raw)
To: linux-arm-kernel
On 03/28/2017 12:09 PM, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@armlinux.org.uk> [170328 04:39]:
>> On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
>>> Yes problems still remains. I think the immediate fix there is to
>>> disable kexec during runtime based on some criteria for your use
>>> case rather than BUG() though. Somehow kexec needs to know if CPU1
>>> reset is acceptable, then reset CPU1 before kexec.
>>
>> The only acceptable way to do that is to make the decision when loading
>> the image(s), and refuse to load the image(s) if kexec is not possible.
>> (As is already done when dealing with whether we can CPU hot-unplug the
>> secondary CPUs.)
>
> OK so how about the following to disable kexec for Andrew's
> test case?
>
> That is assuming that omap_type() test works for HS dra7
> like it does for the other omap2 variants.
>
> Regards,
>
> Tony
>
> 8< ---------------------
> 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
> @@ -260,6 +260,7 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
> extern void omap_auxcoreboot_addr(u32 cpu_addr);
> extern u32 omap_read_auxcoreboot0(void);
>
> +extern bool omap4_cpu_can_disable(unsigned int cpu);
> extern void omap4_cpu_die(unsigned int cpu);
> extern int omap4_cpu_kill(unsigned int cpu);
>
> diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> --- a/arch/arm/mach-omap2/omap-hotplug.c
> +++ b/arch/arm/mach-omap2/omap-hotplug.c
> @@ -22,6 +22,15 @@
> #include "omap-wakeupgen.h"
> #include "common.h"
> #include "powerdomain.h"
> +#include "soc.h"
> +
> +bool omap4_cpu_can_disable(unsigned int cpu)
> +{
> + if (soc_is_dra7xx() && (omap_type() != OMAP2_DEVICE_TYPE_GP))
> + return false;
> +
NAK!
HS variants can have aux cores disabled, we are already doing it right
now, this is another hack to avoid having to fix kexec.
You are failing to park cores correctly, so instead of fixing that you
hack around it on non-HS devices by hard-resetting those cores, now
because you can't do that hack on HS devices you suggest we disable
hotplug on HS devices altogether!?
Kexec *does* work on HS devices. It only fails if we fail to park a core
correctly, something that should never happen, the check here must be
for that, unconditionally disabling Kexec for all HS devices is not a
valid option.
> + return true;
> +}
>
> /*
> * platform-specific code to shutdown a CPU
> 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
> @@ -390,6 +390,7 @@ const struct smp_operations omap4_smp_ops __initconst = {
> .smp_secondary_init = omap4_secondary_init,
> .smp_boot_secondary = omap4_boot_secondary,
> #ifdef CONFIG_HOTPLUG_CPU
> + .cpu_can_disable = omap4_cpu_can_disable,
> .cpu_die = omap4_cpu_die,
> .cpu_kill = omap4_cpu_kill,
> #endif
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-28 11:33 ` Russell King - ARM Linux
@ 2017-03-28 17:53 ` Andrew F. Davis
-1 siblings, 0 replies; 24+ messages in thread
From: Andrew F. Davis @ 2017-03-28 17:53 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Tony Lindgren, Keerthy, Tero Kristo, Santosh Shilimkar,
linux-omap, linux-arm-kernel
On 03/28/2017 06:33 AM, Russell King - ARM Linux wrote:
> On Mon, Mar 27, 2017 at 11:33:30AM -0500, Andrew F. Davis wrote:
>> I still don't like this style of workaround, if kexec cannot regain
>> control of core1 without a hard core reset than kexec should BUG() and
>> force the user to reboot to a sane state.
>
> No, because kexec might already be in progress, and that would create
> an infinite loop of kexec -> bug -> kexec -> bug etc.
>
Ah, yes, I was assuming BUG would halt and force a real reboot.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-28 17:53 ` Andrew F. Davis
0 siblings, 0 replies; 24+ messages in thread
From: Andrew F. Davis @ 2017-03-28 17:53 UTC (permalink / raw)
To: linux-arm-kernel
On 03/28/2017 06:33 AM, Russell King - ARM Linux wrote:
> On Mon, Mar 27, 2017 at 11:33:30AM -0500, Andrew F. Davis wrote:
>> I still don't like this style of workaround, if kexec cannot regain
>> control of core1 without a hard core reset than kexec should BUG() and
>> force the user to reboot to a sane state.
>
> No, because kexec might already be in progress, and that would create
> an infinite loop of kexec -> bug -> kexec -> bug etc.
>
Ah, yes, I was assuming BUG would halt and force a real reboot.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-28 17:52 ` Andrew F. Davis
@ 2017-03-28 18:51 ` Tony Lindgren
-1 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-28 18:51 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Keerthy, Russell King - ARM Linux, Tero Kristo,
Santosh Shilimkar, linux-omap, linux-arm-kernel
* Andrew F. Davis <afd@ti.com> [170328 10:55]:
> On 03/28/2017 12:09 PM, Tony Lindgren wrote:
> > * Russell King - ARM Linux <linux@armlinux.org.uk> [170328 04:39]:
> >> On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
> >>> Yes problems still remains. I think the immediate fix there is to
> >>> disable kexec during runtime based on some criteria for your use
> >>> case rather than BUG() though. Somehow kexec needs to know if CPU1
> >>> reset is acceptable, then reset CPU1 before kexec.
> >>
> >> The only acceptable way to do that is to make the decision when loading
> >> the image(s), and refuse to load the image(s) if kexec is not possible.
> >> (As is already done when dealing with whether we can CPU hot-unplug the
> >> secondary CPUs.)
> >
> > OK so how about the following to disable kexec for Andrew's
> > test case?
> >
> > That is assuming that omap_type() test works for HS dra7
> > like it does for the other omap2 variants.
> >
> > Regards,
> >
> > Tony
> >
> > 8< ---------------------
> > 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
> > @@ -260,6 +260,7 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
> > extern void omap_auxcoreboot_addr(u32 cpu_addr);
> > extern u32 omap_read_auxcoreboot0(void);
> >
> > +extern bool omap4_cpu_can_disable(unsigned int cpu);
> > extern void omap4_cpu_die(unsigned int cpu);
> > extern int omap4_cpu_kill(unsigned int cpu);
> >
> > diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> > --- a/arch/arm/mach-omap2/omap-hotplug.c
> > +++ b/arch/arm/mach-omap2/omap-hotplug.c
> > @@ -22,6 +22,15 @@
> > #include "omap-wakeupgen.h"
> > #include "common.h"
> > #include "powerdomain.h"
> > +#include "soc.h"
> > +
> > +bool omap4_cpu_can_disable(unsigned int cpu)
> > +{
> > + if (soc_is_dra7xx() && (omap_type() != OMAP2_DEVICE_TYPE_GP))
> > + return false;
> > +
>
> NAK!
>
> HS variants can have aux cores disabled, we are already doing it right
> now, this is another hack to avoid having to fix kexec.
Oh sorry the intention was to not break things for your. So probably
just having a separate dra7_hs_smp_ops with no cpu_kill initialized
should do the trick for kexec.
> You are failing to park cores correctly, so instead of fixing that you
> hack around it on non-HS devices by hard-resetting those cores, now
> because you can't do that hack on HS devices you suggest we disable
> hotplug on HS devices altogether!?
Me failing to park cores? The current code has always assumed that
CPU1 enters a low power state for hot-unplug. And AFAIK, there's no
way to have CPU1 continue at a different address at that point
without a reset. The need for preserving CPU1 state through kexec
boot without a reset is something new that you need.
> Kexec *does* work on HS devices. It only fails if we fail to park a core
> correctly, something that should never happen, the check here must be
> for that, unconditionally disabling Kexec for all HS devices is not a
> valid option.
Well sounds like you have to come up with checks for when we
need to disable kexec and use dra7_hs_smp_ops with no cpu_kill.
Regards,
Tony
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-28 18:51 ` Tony Lindgren
0 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-28 18:51 UTC (permalink / raw)
To: linux-arm-kernel
* Andrew F. Davis <afd@ti.com> [170328 10:55]:
> On 03/28/2017 12:09 PM, Tony Lindgren wrote:
> > * Russell King - ARM Linux <linux@armlinux.org.uk> [170328 04:39]:
> >> On Mon, Mar 27, 2017 at 09:43:09AM -0700, Tony Lindgren wrote:
> >>> Yes problems still remains. I think the immediate fix there is to
> >>> disable kexec during runtime based on some criteria for your use
> >>> case rather than BUG() though. Somehow kexec needs to know if CPU1
> >>> reset is acceptable, then reset CPU1 before kexec.
> >>
> >> The only acceptable way to do that is to make the decision when loading
> >> the image(s), and refuse to load the image(s) if kexec is not possible.
> >> (As is already done when dealing with whether we can CPU hot-unplug the
> >> secondary CPUs.)
> >
> > OK so how about the following to disable kexec for Andrew's
> > test case?
> >
> > That is assuming that omap_type() test works for HS dra7
> > like it does for the other omap2 variants.
> >
> > Regards,
> >
> > Tony
> >
> > 8< ---------------------
> > 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
> > @@ -260,6 +260,7 @@ extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
> > extern void omap_auxcoreboot_addr(u32 cpu_addr);
> > extern u32 omap_read_auxcoreboot0(void);
> >
> > +extern bool omap4_cpu_can_disable(unsigned int cpu);
> > extern void omap4_cpu_die(unsigned int cpu);
> > extern int omap4_cpu_kill(unsigned int cpu);
> >
> > diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c
> > --- a/arch/arm/mach-omap2/omap-hotplug.c
> > +++ b/arch/arm/mach-omap2/omap-hotplug.c
> > @@ -22,6 +22,15 @@
> > #include "omap-wakeupgen.h"
> > #include "common.h"
> > #include "powerdomain.h"
> > +#include "soc.h"
> > +
> > +bool omap4_cpu_can_disable(unsigned int cpu)
> > +{
> > + if (soc_is_dra7xx() && (omap_type() != OMAP2_DEVICE_TYPE_GP))
> > + return false;
> > +
>
> NAK!
>
> HS variants can have aux cores disabled, we are already doing it right
> now, this is another hack to avoid having to fix kexec.
Oh sorry the intention was to not break things for your. So probably
just having a separate dra7_hs_smp_ops with no cpu_kill initialized
should do the trick for kexec.
> You are failing to park cores correctly, so instead of fixing that you
> hack around it on non-HS devices by hard-resetting those cores, now
> because you can't do that hack on HS devices you suggest we disable
> hotplug on HS devices altogether!?
Me failing to park cores? The current code has always assumed that
CPU1 enters a low power state for hot-unplug. And AFAIK, there's no
way to have CPU1 continue at a different address at that point
without a reset. The need for preserving CPU1 state through kexec
boot without a reset is something new that you need.
> Kexec *does* work on HS devices. It only fails if we fail to park a core
> correctly, something that should never happen, the check here must be
> for that, unconditionally disabling Kexec for all HS devices is not a
> valid option.
Well sounds like you have to come up with checks for when we
need to disable kexec and use dra7_hs_smp_ops with no cpu_kill.
Regards,
Tony
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-28 17:09 ` Tony Lindgren
@ 2017-03-28 19:15 ` Russell King - ARM Linux
-1 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2017-03-28 19:15 UTC (permalink / raw)
To: Tony Lindgren
Cc: Keerthy, Andrew F. Davis, Tero Kristo, Santosh Shilimkar,
linux-omap, linux-arm-kernel
On Tue, Mar 28, 2017 at 10:09:44AM -0700, Tony Lindgren wrote:
> OK so how about the following to disable kexec for Andrew's
> test case?
That won't work - the hook you're using is to decide whether a particular
CPU (normally the boot CPU) can be hot-plugged. It isn't expected to
return false for all.
The code in question is:
/*
* Validate that if the current HW supports SMP, then the SW supports
* and implements CPU hotplug for the current HW. If not, we won't be
* able to kexec reliably, so fail the prepare operation.
*/
if (num_possible_cpus() > 1 && platform_can_secondary_boot() &&
!platform_can_cpu_hotplug())
return -EINVAL;
and:
int platform_can_cpu_hotplug(void)
{
#ifdef CONFIG_HOTPLUG_CPU
if (smp_ops.cpu_kill)
return 1;
#endif
return 0;
}
Platforms don't get a say in that, other than whether they implement
the operations necessary for hotplug.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-28 19:15 ` Russell King - ARM Linux
0 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2017-03-28 19:15 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 28, 2017 at 10:09:44AM -0700, Tony Lindgren wrote:
> OK so how about the following to disable kexec for Andrew's
> test case?
That won't work - the hook you're using is to decide whether a particular
CPU (normally the boot CPU) can be hot-plugged. It isn't expected to
return false for all.
The code in question is:
/*
* Validate that if the current HW supports SMP, then the SW supports
* and implements CPU hotplug for the current HW. If not, we won't be
* able to kexec reliably, so fail the prepare operation.
*/
if (num_possible_cpus() > 1 && platform_can_secondary_boot() &&
!platform_can_cpu_hotplug())
return -EINVAL;
and:
int platform_can_cpu_hotplug(void)
{
#ifdef CONFIG_HOTPLUG_CPU
if (smp_ops.cpu_kill)
return 1;
#endif
return 0;
}
Platforms don't get a say in that, other than whether they implement
the operations necessary for hotplug.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
2017-03-28 19:15 ` Russell King - ARM Linux
@ 2017-03-28 20:12 ` Tony Lindgren
-1 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-28 20:12 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Keerthy, Andrew F. Davis, Tero Kristo, Santosh Shilimkar,
linux-omap, linux-arm-kernel
* Russell King - ARM Linux <linux@armlinux.org.uk> [170328 12:17]:
> On Tue, Mar 28, 2017 at 10:09:44AM -0700, Tony Lindgren wrote:
> > OK so how about the following to disable kexec for Andrew's
> > test case?
>
> That won't work - the hook you're using is to decide whether a particular
> CPU (normally the boot CPU) can be hot-plugged. It isn't expected to
> return false for all.
>
> The code in question is:
>
> /*
> * Validate that if the current HW supports SMP, then the SW supports
> * and implements CPU hotplug for the current HW. If not, we won't be
> * able to kexec reliably, so fail the prepare operation.
> */
> if (num_possible_cpus() > 1 && platform_can_secondary_boot() &&
> !platform_can_cpu_hotplug())
> return -EINVAL;
>
> and:
>
> int platform_can_cpu_hotplug(void)
> {
> #ifdef CONFIG_HOTPLUG_CPU
> if (smp_ops.cpu_kill)
> return 1;
> #endif
>
> return 0;
> }
>
> Platforms don't get a say in that, other than whether they implement
> the operations necessary for hotplug.
Yeah OK so a separate dra7_hs_smp_ops with no cpu_kill initialized
should do the trick, right?
Regards,
Tony
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot
@ 2017-03-28 20:12 ` Tony Lindgren
0 siblings, 0 replies; 24+ messages in thread
From: Tony Lindgren @ 2017-03-28 20:12 UTC (permalink / raw)
To: linux-arm-kernel
* Russell King - ARM Linux <linux@armlinux.org.uk> [170328 12:17]:
> On Tue, Mar 28, 2017 at 10:09:44AM -0700, Tony Lindgren wrote:
> > OK so how about the following to disable kexec for Andrew's
> > test case?
>
> That won't work - the hook you're using is to decide whether a particular
> CPU (normally the boot CPU) can be hot-plugged. It isn't expected to
> return false for all.
>
> The code in question is:
>
> /*
> * Validate that if the current HW supports SMP, then the SW supports
> * and implements CPU hotplug for the current HW. If not, we won't be
> * able to kexec reliably, so fail the prepare operation.
> */
> if (num_possible_cpus() > 1 && platform_can_secondary_boot() &&
> !platform_can_cpu_hotplug())
> return -EINVAL;
>
> and:
>
> int platform_can_cpu_hotplug(void)
> {
> #ifdef CONFIG_HOTPLUG_CPU
> if (smp_ops.cpu_kill)
> return 1;
> #endif
>
> return 0;
> }
>
> Platforms don't get a say in that, other than whether they implement
> the operations necessary for hotplug.
Yeah OK so a separate dra7_hs_smp_ops with no cpu_kill initialized
should do the trick, right?
Regards,
Tony
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2017-03-28 20:12 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-14 18:05 [PATCHv3] ARM: omap2+: Revert omap-smp.c changes resetting CPU1 during boot Tony Lindgren
2017-03-14 18:05 ` Tony Lindgren
2017-03-15 9:41 ` Keerthy
2017-03-15 9:41 ` Keerthy
2017-03-27 16:33 ` Andrew F. Davis
2017-03-27 16:33 ` Andrew F. Davis
2017-03-27 16:43 ` Tony Lindgren
2017-03-27 16:43 ` Tony Lindgren
2017-03-28 11:36 ` Russell King - ARM Linux
2017-03-28 11:36 ` Russell King - ARM Linux
2017-03-28 17:09 ` Tony Lindgren
2017-03-28 17:09 ` Tony Lindgren
2017-03-28 17:52 ` Andrew F. Davis
2017-03-28 17:52 ` Andrew F. Davis
2017-03-28 18:51 ` Tony Lindgren
2017-03-28 18:51 ` Tony Lindgren
2017-03-28 19:15 ` Russell King - ARM Linux
2017-03-28 19:15 ` Russell King - ARM Linux
2017-03-28 20:12 ` Tony Lindgren
2017-03-28 20:12 ` Tony Lindgren
2017-03-28 11:33 ` Russell King - ARM Linux
2017-03-28 11:33 ` Russell King - ARM Linux
2017-03-28 17:53 ` Andrew F. Davis
2017-03-28 17:53 ` Andrew F. Davis
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.