[PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

The issues are mentioned in patches 1/4/5/6. I will update kdump
documentation for MIPS if the series gets accepted. Testing has been done
on single core i6500/Boston with IOCU, dual core i6500 without IOCU, and
dual core interAptiv without IOCU.

Changes:

v4 - v3:
* In patch #1, idle_task_exit() is moved out from play_dead() to its sole
  caller arch_cpu_idle_dead(). So no interface change of play_dead().
* In patch #6, the kexec_prepare method for the Generic platform is defined
  as uhi_machine_kexec_prepare() for all platforms using UHI boot protocol.

v3 - v2:
* Code style changes according to `scripts/checkpatch.pl --strict`.
  Patch #6, like before, still has a warning message reminding if
  MAINTAINERS needs updating. But it does NOT involve a maintainer change.
* Add LIBFDT to CPU_LOONGSON3 for default_machine_kexec_prepare().

v2 - v1:
* Tested on MIPS32R2 platform in addition to MIPS64R6.
* Added patches #5 and #6.
* In patch #2, removed the unnecessary inclusion of asm/mipsmtregs.h

Dengcheng Zhu (6):
  MIPS: Make play_dead() work for kexec
  MIPS: kexec: Let the new kernel handle all CPUs
  MIPS: kexec: Deprecate (relocated_)kexec_smp_wait
  MIPS: kexec: Do not flush system wide caches in machine_kexec()
  MIPS: kexec: Relax memory restriction
  MIPS: kexec: Use prepare method from Generic for UHI platforms

 arch/mips/Kconfig                     |  4 ++
 arch/mips/cavium-octeon/setup.c       |  2 +-
 arch/mips/cavium-octeon/smp.c         | 34 ++++++-----
 arch/mips/generic/Makefile            |  1 -
 arch/mips/generic/kexec.c             | 44 --------------
 arch/mips/include/asm/kexec.h         | 11 ++--
 arch/mips/include/asm/smp.h           |  4 +-
 arch/mips/kernel/crash.c              |  4 +-
 arch/mips/kernel/machine_kexec.c      | 82 +++++++++++++++++++++++----
 arch/mips/kernel/process.c            |  2 +
 arch/mips/kernel/relocate_kernel.S    | 39 -------------
 arch/mips/kernel/smp-bmips.c          |  8 ++-
 arch/mips/kernel/smp-cps.c            | 49 +++++++++-------
 arch/mips/loongson64/loongson-3/smp.c | 82 ++++++++++++++-------------
 14 files changed, 183 insertions(+), 183 deletions(-)
 delete mode 100644 arch/mips/generic/kexec.c

-- 
2.17.1

[PATCH v4 1/6] MIPS: Make play_dead() work for kexec

[Dengcheng Zhu]

Extract play_dead() from CONFIG_HOTPLUG_CPU and share with CONFIG_KEXEC.
Meanwhile, move idle_task_exit() out to arch_cpu_idle_dead() because it's
not meant for kexec.

This is needed to correctly support SMP new kernel in kexec. Before doing
this, all non-crashing CPUs are waiting for the reboot signal
(kexec_ready_to_reboot) to jump to kexec_start_address in kexec_smp_wait(),
which creates some problems like incorrect CPU topology upon booting from
new UP kernel, sluggish performance in MT environment during and after
reboot, new SMP kernel not able to bring up secondary CPU etc.

It would make sense to let the new (SMP) kernel manage all CPUs, instead of
crashing CPU booting from reboot_code_buffer whereas others jumping to
kexec_start_address. To do this, play_dead() is well suitable for preparing
a boot environment for the new kernel.

Tested-by: Rachel Mozes <rachel.mozes@intel.com>
Reported-by: Rachel Mozes <rachel.mozes@intel.com>
Signed-off-by: Dengcheng Zhu <dzhu@wavecomp.com>
---
Changes:

* idle_task_exit() is moved out from play_dead() to its sole caller
  arch_cpu_idle_dead(). So no interface change of play_dead().

 arch/mips/cavium-octeon/smp.c         | 34 ++++++-----
 arch/mips/include/asm/smp.h           |  4 +-
 arch/mips/kernel/process.c            |  2 +
 arch/mips/kernel/smp-bmips.c          |  8 ++-
 arch/mips/kernel/smp-cps.c            | 49 +++++++++-------
 arch/mips/loongson64/loongson-3/smp.c | 82 ++++++++++++++-------------
 6 files changed, 99 insertions(+), 80 deletions(-)

diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index 75e7c8625659..19dea9622fe5 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -280,11 +280,29 @@ static void octeon_smp_finish(void)
 	local_irq_enable();
 }
 
-#ifdef CONFIG_HOTPLUG_CPU
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_KEXEC)
 
 /* State of each CPU. */
 DEFINE_PER_CPU(int, cpu_state);
 
+void play_dead(void)
+{
+	int cpu = cpu_number_map(cvmx_get_core_num());
+
+	octeon_processor_boot = 0xff;
+	per_cpu(cpu_state, cpu) = CPU_DEAD;
+
+	/* make the change visible before going off */
+	mb();
+
+	while (1)	/* core will be reset here */
+		;
+}
+
+#endif /* CONFIG_HOTPLUG_CPU || CONFIG_KEXEC */
+
+#ifdef CONFIG_HOTPLUG_CPU
+
 static int octeon_cpu_disable(void)
 {
 	unsigned int cpu = smp_processor_id();
@@ -343,20 +361,6 @@ static void octeon_cpu_die(unsigned int cpu)
 	cvmx_write_csr(CVMX_CIU_PP_RST, 0);
 }
 
-void play_dead(void)
-{
-	int cpu = cpu_number_map(cvmx_get_core_num());
-
-	idle_task_exit();
-	octeon_processor_boot = 0xff;
-	per_cpu(cpu_state, cpu) = CPU_DEAD;
-
-	mb();
-
-	while (1)	/* core will be reset here */
-		;
-}
-
 static void start_after_reset(void)
 {
 	kernel_entry(0, 0, 0);	/* set a2 = 0 for secondary core */
diff --git a/arch/mips/include/asm/smp.h b/arch/mips/include/asm/smp.h
index 88ebd83b3bf9..cad5e78889c0 100644
--- a/arch/mips/include/asm/smp.h
+++ b/arch/mips/include/asm/smp.h
@@ -77,8 +77,10 @@ static inline void __cpu_die(unsigned int cpu)
 
 	mp_ops->cpu_die(cpu);
 }
+#endif
 
-extern void play_dead(void);
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_KEXEC)
+void play_dead(void);
 #endif
 
 /*
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 8d85046adcc8..8e5868362e55 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -14,6 +14,7 @@
 #include <linux/sched/debug.h>
 #include <linux/sched/task.h>
 #include <linux/sched/task_stack.h>
+#include <linux/sched/hotplug.h>
 #include <linux/tick.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
@@ -53,6 +54,7 @@
 #ifdef CONFIG_HOTPLUG_CPU
 void arch_cpu_idle_dead(void)
 {
+	idle_task_exit();
 	play_dead();
 }
 #endif
diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c
index 159e83add4bb..284e49c8222f 100644
--- a/arch/mips/kernel/smp-bmips.c
+++ b/arch/mips/kernel/smp-bmips.c
@@ -381,10 +381,12 @@ static void bmips_cpu_die(unsigned int cpu)
 {
 }
 
+#endif /* CONFIG_HOTPLUG_CPU */
+
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_KEXEC)
+
 void __ref play_dead(void)
 {
-	idle_task_exit();
-
 	/* flush data cache */
 	_dma_cache_wback_inv(0, ~0);
 
@@ -409,7 +411,7 @@ void __ref play_dead(void)
 	: : : "memory");
 }
 
-#endif /* CONFIG_HOTPLUG_CPU */
+#endif /* CONFIG_HOTPLUG_CPU || CONFIG_KEXEC */
 
 const struct plat_smp_ops bmips43xx_smp_ops = {
 	.smp_setup		= bmips_smp_setup,
diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
index 03f1026ad148..2b0ab25109f9 100644
--- a/arch/mips/kernel/smp-cps.c
+++ b/arch/mips/kernel/smp-cps.c
@@ -398,27 +398,7 @@ static void cps_smp_finish(void)
 	local_irq_enable();
 }
 
-#ifdef CONFIG_HOTPLUG_CPU
-
-static int cps_cpu_disable(void)
-{
-	unsigned cpu = smp_processor_id();
-	struct core_boot_config *core_cfg;
-
-	if (!cpu)
-		return -EBUSY;
-
-	if (!cps_pm_support_state(CPS_PM_POWER_GATED))
-		return -EINVAL;
-
-	core_cfg = &mips_cps_core_bootcfg[cpu_core(&current_cpu_data)];
-	atomic_sub(1 << cpu_vpe_id(&current_cpu_data), &core_cfg->vpe_mask);
-	smp_mb__after_atomic();
-	set_cpu_online(cpu, false);
-	calculate_cpu_foreign_map();
-
-	return 0;
-}
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_KEXEC)
 
 static unsigned cpu_death_sibling;
 static enum {
@@ -431,7 +411,7 @@ void play_dead(void)
 	unsigned int cpu, core, vpe_id;
 
 	local_irq_disable();
-	idle_task_exit();
+
 	cpu = smp_processor_id();
 	core = cpu_core(&cpu_data[cpu]);
 	cpu_death = CPU_DEATH_POWER;
@@ -480,6 +460,31 @@ void play_dead(void)
 	panic("Failed to offline CPU %u", cpu);
 }
 
+#endif /* CONFIG_HOTPLUG_CPU || CONFIG_KEXEC */
+
+#ifdef CONFIG_HOTPLUG_CPU
+
+static int cps_cpu_disable(void)
+{
+	unsigned int cpu = smp_processor_id();
+	struct core_boot_config *core_cfg;
+
+	if (!cpu)
+		return -EBUSY;
+
+	if (!cps_pm_support_state(CPS_PM_POWER_GATED))
+		return -EINVAL;
+
+	core_cfg = &mips_cps_core_bootcfg[cpu_core(&current_cpu_data)];
+	atomic_sub(1 << cpu_vpe_id(&current_cpu_data), &core_cfg->vpe_mask);
+	/* make sure the change is perceived before setting offline */
+	smp_mb__after_atomic();
+	set_cpu_online(cpu, false);
+	calculate_cpu_foreign_map();
+
+	return 0;
+}
+
 static void wait_for_sibling_halt(void *ptr_cpu)
 {
 	unsigned cpu = (unsigned long)ptr_cpu;
diff --git a/arch/mips/loongson64/loongson-3/smp.c b/arch/mips/loongson64/loongson-3/smp.c
index 8501109bb0f0..29628beed0f2 100644
--- a/arch/mips/loongson64/loongson-3/smp.c
+++ b/arch/mips/loongson64/loongson-3/smp.c
@@ -455,6 +455,48 @@ static void loongson3_cpu_die(unsigned int cpu)
 	mb();
 }
 
+static int loongson3_disable_clock(unsigned int cpu)
+{
+	u64 core_id = cpu_core(&cpu_data[cpu]);
+	u64 package_id = cpu_data[cpu].package;
+
+	if ((read_c0_prid() & PRID_REV_MASK) == PRID_REV_LOONGSON3A_R1) {
+		LOONGSON_CHIPCFG(package_id) &= ~(1 << (12 + core_id));
+	} else {
+		if (!(loongson_sysconf.workarounds & WORKAROUND_CPUHOTPLUG))
+			LOONGSON_FREQCTRL(package_id) &=
+				~(1 << (core_id * 4 + 3));
+	}
+	return 0;
+}
+
+static int loongson3_enable_clock(unsigned int cpu)
+{
+	u64 core_id = cpu_core(&cpu_data[cpu]);
+	u64 package_id = cpu_data[cpu].package;
+
+	if ((read_c0_prid() & PRID_REV_MASK) == PRID_REV_LOONGSON3A_R1) {
+		LOONGSON_CHIPCFG(package_id) |= 1 << (12 + core_id);
+	} else {
+		if (!(loongson_sysconf.workarounds & WORKAROUND_CPUHOTPLUG))
+			LOONGSON_FREQCTRL(package_id) |= 1 << (core_id * 4 + 3);
+	}
+	return 0;
+}
+
+static int register_loongson3_notifier(void)
+{
+	return cpuhp_setup_state_nocalls(CPUHP_MIPS_SOC_PREPARE,
+					 "mips/loongson:prepare",
+					 loongson3_enable_clock,
+					 loongson3_disable_clock);
+}
+early_initcall(register_loongson3_notifier);
+
+#endif /* CONFIG_HOTPLUG_CPU */
+
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_KEXEC)
+
 /* To shutdown a core in Loongson 3, the target core should go to CKSEG1 and
  * flush all L1 entries at first. Then, another core (usually Core 0) can
  * safely disable the clock of the target core. loongson3_play_dead() is
@@ -674,7 +716,6 @@ void play_dead(void)
 	unsigned int cpu = smp_processor_id();
 	void (*play_dead_at_ckseg1)(int *);
 
-	idle_task_exit();
 	switch (read_c0_prid() & PRID_REV_MASK) {
 	case PRID_REV_LOONGSON3A_R1:
 	default:
@@ -697,44 +738,7 @@ void play_dead(void)
 	play_dead_at_ckseg1(state_addr);
 }
 
-static int loongson3_disable_clock(unsigned int cpu)
-{
-	uint64_t core_id = cpu_core(&cpu_data[cpu]);
-	uint64_t package_id = cpu_data[cpu].package;
-
-	if ((read_c0_prid() & PRID_REV_MASK) == PRID_REV_LOONGSON3A_R1) {
-		LOONGSON_CHIPCFG(package_id) &= ~(1 << (12 + core_id));
-	} else {
-		if (!(loongson_sysconf.workarounds & WORKAROUND_CPUHOTPLUG))
-			LOONGSON_FREQCTRL(package_id) &= ~(1 << (core_id * 4 + 3));
-	}
-	return 0;
-}
-
-static int loongson3_enable_clock(unsigned int cpu)
-{
-	uint64_t core_id = cpu_core(&cpu_data[cpu]);
-	uint64_t package_id = cpu_data[cpu].package;
-
-	if ((read_c0_prid() & PRID_REV_MASK) == PRID_REV_LOONGSON3A_R1) {
-		LOONGSON_CHIPCFG(package_id) |= 1 << (12 + core_id);
-	} else {
-		if (!(loongson_sysconf.workarounds & WORKAROUND_CPUHOTPLUG))
-			LOONGSON_FREQCTRL(package_id) |= 1 << (core_id * 4 + 3);
-	}
-	return 0;
-}
-
-static int register_loongson3_notifier(void)
-{
-	return cpuhp_setup_state_nocalls(CPUHP_MIPS_SOC_PREPARE,
-					 "mips/loongson:prepare",
-					 loongson3_enable_clock,
-					 loongson3_disable_clock);
-}
-early_initcall(register_loongson3_notifier);
-
-#endif
+#endif /* CONFIG_HOTPLUG_CPU || CONFIG_KEXEC */
 
 const struct plat_smp_ops loongson3_smp_ops = {
 	.send_ipi_single = loongson3_send_ipi_single,
-- 
2.17.1

[PATCH v4 2/6] MIPS: kexec: Let the new kernel handle all CPUs

[Dengcheng Zhu]

Use play_dead() to prepare an environment for the new kernel. Do not rely
on non-crashing CPUs jumping to kexec_start_address.

Tested-by: Rachel Mozes <rachel.mozes@intel.com>
Reported-by: Rachel Mozes <rachel.mozes@intel.com>
Signed-off-by: Dengcheng Zhu <dzhu@wavecomp.com>
---
Changes:

* Use play_dead() without an extra parameter.

 arch/mips/cavium-octeon/setup.c  |  2 +-
 arch/mips/include/asm/kexec.h    |  3 ++-
 arch/mips/kernel/crash.c         |  4 +++-
 arch/mips/kernel/machine_kexec.c | 28 ++++++++++++++++++++++++----
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c
index a8034d0dcade..3a1f7fa42413 100644
--- a/arch/mips/cavium-octeon/setup.c
+++ b/arch/mips/cavium-octeon/setup.c
@@ -95,7 +95,7 @@ static void octeon_kexec_smp_down(void *ignored)
 	"	sync						\n"
 	"	synci	($0)					\n");
 
-	relocated_kexec_smp_wait(NULL);
+	kexec_smp_reboot();
 }
 #endif
 
diff --git a/arch/mips/include/asm/kexec.h b/arch/mips/include/asm/kexec.h
index 493a3cc7c39a..a8e0bfc0da19 100644
--- a/arch/mips/include/asm/kexec.h
+++ b/arch/mips/include/asm/kexec.h
@@ -39,12 +39,13 @@ extern unsigned long kexec_args[4];
 extern int (*_machine_kexec_prepare)(struct kimage *);
 extern void (*_machine_kexec_shutdown)(void);
 extern void (*_machine_crash_shutdown)(struct pt_regs *regs);
-extern void default_machine_crash_shutdown(struct pt_regs *regs);
+void default_machine_crash_shutdown(struct pt_regs *regs);
 #ifdef CONFIG_SMP
 extern const unsigned char kexec_smp_wait[];
 extern unsigned long secondary_kexec_args[4];
 extern void (*relocated_kexec_smp_wait) (void *);
 extern atomic_t kexec_ready_to_reboot;
+void kexec_smp_reboot(void);
 extern void (*_crash_smp_send_stop)(void);
 #endif
 #endif
diff --git a/arch/mips/kernel/crash.c b/arch/mips/kernel/crash.c
index d455363d51c3..6d4320067cff 100644
--- a/arch/mips/kernel/crash.c
+++ b/arch/mips/kernel/crash.c
@@ -43,7 +43,9 @@ static void crash_shutdown_secondary(void *passed_regs)
 
 	while (!atomic_read(&kexec_ready_to_reboot))
 		cpu_relax();
-	relocated_kexec_smp_wait(NULL);
+
+	kexec_smp_reboot();
+
 	/* NOTREACHED */
 }
 
diff --git a/arch/mips/kernel/machine_kexec.c b/arch/mips/kernel/machine_kexec.c
index 8b574bcd39ba..900475ae256d 100644
--- a/arch/mips/kernel/machine_kexec.c
+++ b/arch/mips/kernel/machine_kexec.c
@@ -19,6 +19,10 @@ extern const size_t relocate_new_kernel_size;
 extern unsigned long kexec_start_address;
 extern unsigned long kexec_indirection_page;
 
+static unsigned long reboot_code_buffer;
+
+typedef void (*noretfun_t)(void) __noreturn;
+
 int (*_machine_kexec_prepare)(struct kimage *) = NULL;
 void (*_machine_kexec_shutdown)(void) = NULL;
 void (*_machine_crash_shutdown)(struct pt_regs *regs) = NULL;
@@ -26,6 +30,20 @@ void (*_machine_crash_shutdown)(struct pt_regs *regs) = NULL;
 void (*relocated_kexec_smp_wait) (void *);
 atomic_t kexec_ready_to_reboot = ATOMIC_INIT(0);
 void (*_crash_smp_send_stop)(void) = NULL;
+
+void kexec_smp_reboot(void)
+{
+	if (smp_processor_id() > 0) {
+		/*
+		 * Instead of cpu_relax() or wait, this is needed for kexec
+		 * smp reboot. Kdump usually doesn't require an smp new
+		 * kernel, but kexec may do.
+		 */
+		play_dead();
+	} else {
+		((noretfun_t)reboot_code_buffer)();
+	}
+}
 #endif
 
 static void kexec_image_info(const struct kimage *kimage)
@@ -79,12 +97,9 @@ machine_crash_shutdown(struct pt_regs *regs)
 		default_machine_crash_shutdown(regs);
 }
 
-typedef void (*noretfun_t)(void) __noreturn;
-
 void
 machine_kexec(struct kimage *image)
 {
-	unsigned long reboot_code_buffer;
 	unsigned long entry;
 	unsigned long *ptr;
 
@@ -132,6 +147,11 @@ machine_kexec(struct kimage *image)
 		(void *)(kexec_smp_wait - relocate_new_kernel);
 	smp_wmb();
 	atomic_set(&kexec_ready_to_reboot, 1);
-#endif
+
+	kexec_smp_reboot();
+
+	/* NOT REACHED */
+#else
 	((noretfun_t) reboot_code_buffer)();
+#endif
 }
-- 
2.17.1

[PATCH v4 3/6] MIPS: kexec: Deprecate (relocated_)kexec_smp_wait

[Dengcheng Zhu]

Since we've changed the SMP boot mechanism (by having the new kernel handle
all CPUs), now remove (relocated_)kexec_smp_wait.

Tested-by: Rachel Mozes <rachel.mozes@intel.com>
Reported-by: Rachel Mozes <rachel.mozes@intel.com>
Signed-off-by: Dengcheng Zhu <dzhu@wavecomp.com>
---
 arch/mips/include/asm/kexec.h      |  2 --
 arch/mips/kernel/machine_kexec.c   |  5 ----
 arch/mips/kernel/relocate_kernel.S | 39 ------------------------------
 3 files changed, 46 deletions(-)

diff --git a/arch/mips/include/asm/kexec.h b/arch/mips/include/asm/kexec.h
index a8e0bfc0da19..33f31a8bd8d3 100644
--- a/arch/mips/include/asm/kexec.h
+++ b/arch/mips/include/asm/kexec.h
@@ -41,9 +41,7 @@ extern void (*_machine_kexec_shutdown)(void);
 extern void (*_machine_crash_shutdown)(struct pt_regs *regs);
 void default_machine_crash_shutdown(struct pt_regs *regs);
 #ifdef CONFIG_SMP
-extern const unsigned char kexec_smp_wait[];
 extern unsigned long secondary_kexec_args[4];
-extern void (*relocated_kexec_smp_wait) (void *);
 extern atomic_t kexec_ready_to_reboot;
 void kexec_smp_reboot(void);
 extern void (*_crash_smp_send_stop)(void);
diff --git a/arch/mips/kernel/machine_kexec.c b/arch/mips/kernel/machine_kexec.c
index 900475ae256d..baffc7113204 100644
--- a/arch/mips/kernel/machine_kexec.c
+++ b/arch/mips/kernel/machine_kexec.c
@@ -27,7 +27,6 @@ int (*_machine_kexec_prepare)(struct kimage *) = NULL;
 void (*_machine_kexec_shutdown)(void) = NULL;
 void (*_machine_crash_shutdown)(struct pt_regs *regs) = NULL;
 #ifdef CONFIG_SMP
-void (*relocated_kexec_smp_wait) (void *);
 atomic_t kexec_ready_to_reboot = ATOMIC_INIT(0);
 void (*_crash_smp_send_stop)(void) = NULL;
 
@@ -142,10 +141,6 @@ machine_kexec(struct kimage *image)
 	printk("Bye ...\n");
 	__flush_cache_all();
 #ifdef CONFIG_SMP
-	/* All secondary cpus now may jump to kexec_wait cycle */
-	relocated_kexec_smp_wait = reboot_code_buffer +
-		(void *)(kexec_smp_wait - relocate_new_kernel);
-	smp_wmb();
 	atomic_set(&kexec_ready_to_reboot, 1);
 
 	kexec_smp_reboot();
diff --git a/arch/mips/kernel/relocate_kernel.S b/arch/mips/kernel/relocate_kernel.S
index c6bbf2165051..14e0eaf12306 100644
--- a/arch/mips/kernel/relocate_kernel.S
+++ b/arch/mips/kernel/relocate_kernel.S
@@ -100,45 +100,6 @@ done:
 	j		s1
 	END(relocate_new_kernel)
 
-#ifdef CONFIG_SMP
-/*
- * Other CPUs should wait until code is relocated and
- * then start at entry (?) point.
- */
-LEAF(kexec_smp_wait)
-	PTR_L		a0, s_arg0
-	PTR_L		a1, s_arg1
-	PTR_L		a2, s_arg2
-	PTR_L		a3, s_arg3
-	PTR_L		s1, kexec_start_address
-
-	/* Non-relocated address works for args and kexec_start_address ( old
-	 * kernel is not overwritten). But we need relocated address of
-	 * kexec_flag.
-	 */
-
-	bal		1f
-1:	move		t1,ra;
-	PTR_LA		t2,1b
-	PTR_LA		t0,kexec_flag
-	PTR_SUB		t0,t0,t2;
-	PTR_ADD		t0,t1,t0;
-
-1:	LONG_L		s0, (t0)
-	bne		s0, zero,1b
-
-#ifdef CONFIG_CPU_CAVIUM_OCTEON
-	.set push
-	.set noreorder
-	synci		0($0)
-	.set pop
-#else
-	sync
-#endif
-	j		s1
-	END(kexec_smp_wait)
-#endif
-
 #ifdef __mips64
        /* all PTR's must be aligned to 8 byte in 64-bit mode */
        .align  3
-- 
2.17.1

[PATCH v4 4/6] MIPS: kexec: Do not flush system wide caches in machine_kexec()

[Dengcheng Zhu]

Instead of __flush_cache_all(), simply flush local icache range. In systems
without IOCU, flushing system wide caches require sending IPIs. But other
CPUs have disabled local IRQs waiting for the reboot signal. It will then
cause system hang.

This patch fixes this problem.

Tested-by: Rachel Mozes <rachel.mozes@intel.com>
Reported-by: Rachel Mozes <rachel.mozes@intel.com>
Signed-off-by: Dengcheng Zhu <dzhu@wavecomp.com>
---
 arch/mips/kernel/machine_kexec.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/machine_kexec.c b/arch/mips/kernel/machine_kexec.c
index baffc7113204..c2119e448490 100644
--- a/arch/mips/kernel/machine_kexec.c
+++ b/arch/mips/kernel/machine_kexec.c
@@ -139,7 +139,16 @@ machine_kexec(struct kimage *image)
 
 	printk("Will call new kernel at %08lx\n", image->start);
 	printk("Bye ...\n");
-	__flush_cache_all();
+	/*
+	 * __flush_cache_all() is expensive but unnecessary. More
+	 * importantly, it could freeze the system as it may need to send
+	 * IPIs, whereas other CPUs have been waiting for the reboot signal
+	 * (kexec_ready_to_reboot) with local irqs disabled, because
+	 * machine_crash_shutdown() has been called prior to entering
+	 * this function - machine_kexec().
+	 */
+	local_flush_icache_range(reboot_code_buffer,
+				 reboot_code_buffer + relocate_new_kernel_size);
 #ifdef CONFIG_SMP
 	atomic_set(&kexec_ready_to_reboot, 1);
 
-- 
2.17.1

[PATCH v4 5/6] MIPS: kexec: Relax memory restriction

[Dengcheng Zhu]

We can rely on the system kernel and the dump capture kernel themselves in
memory usage.

Being restrictive with 512MB limit may cause kexec tool failure on some
platforms.

Tested-by: Rachel Mozes <rachel.mozes@intel.com>
Reported-by: Rachel Mozes <rachel.mozes@intel.com>
Signed-off-by: Dengcheng Zhu <dzhu@wavecomp.com>
---
 arch/mips/include/asm/kexec.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/kexec.h b/arch/mips/include/asm/kexec.h
index 33f31a8bd8d3..2583e6a201e3 100644
--- a/arch/mips/include/asm/kexec.h
+++ b/arch/mips/include/asm/kexec.h
@@ -12,11 +12,11 @@
 #include <asm/stacktrace.h>
 
 /* Maximum physical address we can use pages from */
-#define KEXEC_SOURCE_MEMORY_LIMIT (0x20000000)
+#define KEXEC_SOURCE_MEMORY_LIMIT (-1UL)
 /* Maximum address we can reach in physical address mode */
-#define KEXEC_DESTINATION_MEMORY_LIMIT (0x20000000)
+#define KEXEC_DESTINATION_MEMORY_LIMIT (-1UL)
  /* Maximum address we can use for the control code buffer */
-#define KEXEC_CONTROL_MEMORY_LIMIT (0x20000000)
+#define KEXEC_CONTROL_MEMORY_LIMIT (-1UL)
 /* Reserve 3*4096 bytes for board-specific info */
 #define KEXEC_CONTROL_PAGE_SIZE (4096 + 3*4096)
 
-- 
2.17.1

[PATCH v4 6/6] MIPS: kexec: Use prepare method from Generic for UHI platforms

[Dengcheng Zhu]

The kexec_prepare method for the Generic platform should be applicable to
others using UHI boot protocol. In arch/mips/Kconfig, "select UHI_BOOT" is
needed for such platforms.

If the platform has its own method for _machine_kexec_prepare(), that one
will be used.

Tested-by: Rachel Mozes <rachel.mozes@intel.com>
Reported-by: Rachel Mozes <rachel.mozes@intel.com>
Signed-off-by: Dengcheng Zhu <dzhu@wavecomp.com>
---
Changes:

* The kexec_prepare method for the Generic platform is defined as
  uhi_machine_kexec_prepare() for all platforms using UHI boot protocol.

 arch/mips/Kconfig                |  4 +++
 arch/mips/generic/Makefile       |  1 -
 arch/mips/generic/kexec.c        | 44 --------------------------------
 arch/mips/kernel/machine_kexec.c | 38 ++++++++++++++++++++++++++-
 4 files changed, 41 insertions(+), 46 deletions(-)
 delete mode 100644 arch/mips/generic/kexec.c

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 08c10c518f83..260f2ee083ac 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -129,6 +129,7 @@ config MIPS_GENERIC
 	select USB_UHCI_BIG_ENDIAN_DESC if CPU_BIG_ENDIAN
 	select USB_UHCI_BIG_ENDIAN_MMIO if CPU_BIG_ENDIAN
 	select USE_OF
+	select UHI_BOOT
 	help
 	  Select this to build a kernel which aims to support multiple boards,
 	  generally using a flattened device tree passed from the bootloader
@@ -2907,6 +2908,9 @@ config USE_OF
 	select OF_EARLY_FLATTREE
 	select IRQ_DOMAIN
 
+config UHI_BOOT
+	bool
+
 config BUILTIN_DTB
 	bool
 
diff --git a/arch/mips/generic/Makefile b/arch/mips/generic/Makefile
index d03a36f869a4..181aa1335419 100644
--- a/arch/mips/generic/Makefile
+++ b/arch/mips/generic/Makefile
@@ -15,5 +15,4 @@ obj-y += proc.o
 obj-$(CONFIG_YAMON_DT_SHIM)		+= yamon-dt.o
 obj-$(CONFIG_LEGACY_BOARD_SEAD3)	+= board-sead3.o
 obj-$(CONFIG_LEGACY_BOARD_OCELOT)	+= board-ocelot.o
-obj-$(CONFIG_KEXEC)			+= kexec.o
 obj-$(CONFIG_VIRT_BOARD_RANCHU)		+= board-ranchu.o
diff --git a/arch/mips/generic/kexec.c b/arch/mips/generic/kexec.c
deleted file mode 100644
index 1ca409f58929..000000000000
--- a/arch/mips/generic/kexec.c
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * Copyright (C) 2016 Imagination Technologies
- * Author: Marcin Nowakowski <marcin.nowakowski@mips.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- */
-
-#include <linux/kexec.h>
-#include <linux/libfdt.h>
-#include <linux/uaccess.h>
-
-static int generic_kexec_prepare(struct kimage *image)
-{
-	int i;
-
-	for (i = 0; i < image->nr_segments; i++) {
-		struct fdt_header fdt;
-
-		if (image->segment[i].memsz <= sizeof(fdt))
-			continue;
-
-		if (copy_from_user(&fdt, image->segment[i].buf, sizeof(fdt)))
-			continue;
-
-		if (fdt_check_header(&fdt))
-			continue;
-
-		kexec_args[0] = -2;
-		kexec_args[1] = (unsigned long)
-			phys_to_virt((unsigned long)image->segment[i].mem);
-		break;
-	}
-	return 0;
-}
-
-static int __init register_generic_kexec(void)
-{
-	_machine_kexec_prepare = generic_kexec_prepare;
-	return 0;
-}
-arch_initcall(register_generic_kexec);
diff --git a/arch/mips/kernel/machine_kexec.c b/arch/mips/kernel/machine_kexec.c
index c2119e448490..209196b195c5 100644
--- a/arch/mips/kernel/machine_kexec.c
+++ b/arch/mips/kernel/machine_kexec.c
@@ -9,6 +9,7 @@
 #include <linux/kexec.h>
 #include <linux/mm.h>
 #include <linux/delay.h>
+#include <linux/libfdt.h>
 
 #include <asm/cacheflush.h>
 #include <asm/page.h>
@@ -23,7 +24,6 @@ static unsigned long reboot_code_buffer;
 
 typedef void (*noretfun_t)(void) __noreturn;
 
-int (*_machine_kexec_prepare)(struct kimage *) = NULL;
 void (*_machine_kexec_shutdown)(void) = NULL;
 void (*_machine_crash_shutdown)(struct pt_regs *regs) = NULL;
 #ifdef CONFIG_SMP
@@ -65,6 +65,42 @@ static void kexec_image_info(const struct kimage *kimage)
 	}
 }
 
+#ifdef CONFIG_UHI_BOOT
+static int uhi_machine_kexec_prepare(struct kimage *kimage)
+{
+	int i;
+
+	/*
+	 * In case DTB file is not passed to the new kernel, a flat device
+	 * tree will be created by kexec tool. It holds modified command
+	 * line for the new kernel.
+	 */
+	for (i = 0; i < kimage->nr_segments; i++) {
+		struct fdt_header fdt;
+
+		if (kimage->segment[i].memsz <= sizeof(fdt))
+			continue;
+
+		if (copy_from_user(&fdt, kimage->segment[i].buf, sizeof(fdt)))
+			continue;
+
+		if (fdt_check_header(&fdt))
+			continue;
+
+		kexec_args[0] = -2;
+		kexec_args[1] = (unsigned long)
+			phys_to_virt((unsigned long)kimage->segment[i].mem);
+		break;
+	}
+
+	return 0;
+}
+
+int (*_machine_kexec_prepare)(struct kimage *) = uhi_machine_kexec_prepare;
+#else
+int (*_machine_kexec_prepare)(struct kimage *) = NULL;
+#endif /* CONFIG_UHI_BOOT */
+
 int
 machine_kexec_prepare(struct kimage *kimage)
 {
-- 
2.17.1

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Paul Burton]

Hi Dengcheng,

On Wed, Sep 05, 2018 at 08:59:03AM -0700, Dengcheng Zhu wrote:
> The issues are mentioned in patches 1/4/5/6. I will update kdump
> documentation for MIPS if the series gets accepted. Testing has been done
> on single core i6500/Boston with IOCU, dual core i6500 without IOCU, and
> dual core interAptiv without IOCU.
> 
> Changes:
> 
> v4 - v3:
> * In patch #1, idle_task_exit() is moved out from play_dead() to its sole
>   caller arch_cpu_idle_dead(). So no interface change of play_dead().
> * In patch #6, the kexec_prepare method for the Generic platform is defined
>   as uhi_machine_kexec_prepare() for all platforms using UHI boot protocol.

Thanks! I've applied patches 1,2,3,5 to a test branch with a few
changes:

    git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git test-kexec

    https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/log/?h=test-kexec

I didn't apply patch 4 because I'm not sure it's correct & I believe the
changes in the branch above should take care of it - CPUs that reach
kexec_this_cpu() are maked offline, so they shouldn't be IPI'd by
__flush_cache_all(). The CPU that runs machine_kexec() should still
flush its dcache (& the L2), and then CPU 0 invalidates its icache in
kexec_this_cpu() prior to jumping into reboot_code_buffer.

I'm also still not sure about patch 6 - since no platforms besides the
arch/mips/generic/ make use of the UHI boot code yet I think it'd be
best to leave as-is. If we do ever need to use it from another platform
then we can deal with the problem then. If an out of tree platform needs
to use it then for now it could copy generic_kexec_prepare() and deal
with removing the duplication when it heads upstream.

Could you take a look & let me know if you see any problems?

Thanks,
    Paul

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

Hi Paul,

On 09/05/2018 03:54 PM, Paul Burton wrote:
> Hi Dengcheng,
>
> On Wed, Sep 05, 2018 at 08:59:03AM -0700, Dengcheng Zhu wrote:
>> The issues are mentioned in patches 1/4/5/6. I will update kdump
>> documentation for MIPS if the series gets accepted. Testing has been done
>> on single core i6500/Boston with IOCU, dual core i6500 without IOCU, and
>> dual core interAptiv without IOCU.
>>
>> Changes:
>>
>> v4 - v3:
>> * In patch #1, idle_task_exit() is moved out from play_dead() to its sole
>>    caller arch_cpu_idle_dead(). So no interface change of play_dead().
>> * In patch #6, the kexec_prepare method for the Generic platform is defined
>>    as uhi_machine_kexec_prepare() for all platforms using UHI boot protocol.
> Thanks! I've applied patches 1,2,3,5 to a test branch with a few
> changes:
>
>      git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git test-kexec
>
>      https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/log/?h=test-kexec
>
> I didn't apply patch 4 because I'm not sure it's correct & I believe the
> changes in the branch above should take care of it - CPUs that reach
> kexec_this_cpu() are maked offline, so they shouldn't be IPI'd by
> __flush_cache_all().

I believe patch 4 is necessary. As mentioned in the code comment and patch
comment of that patch, machine_crash_shutdown() is called prior to
machine_kexec() in the kdump sequence. So other CPUs have disabled local
IRQs waiting for the reboot signal.

In fact, in kexec_this_cpu() [you renamed and modified kexec_smp_reboot()],
the added marking CPU offline will cause system hang (tested). This is
because it will change how play_dead() will work.

> The CPU that runs machine_kexec() should still
> flush its dcache (& the L2), and then CPU 0 invalidates its icache in
> kexec_this_cpu() prior to jumping into reboot_code_buffer.
>
> I'm also still not sure about patch 6 - since no platforms besides the
> arch/mips/generic/ make use of the UHI boot code yet I think it'd be
> best to leave as-is. If we do ever need to use it from another platform
> then we can deal with the problem then. If an out of tree platform needs
> to use it then for now it could copy generic_kexec_prepare() and deal
> with removing the duplication when it heads upstream.

Understood. It really depends on how this problem is viewed. If patch #6
is considered creating a framework for future UHI platforms, then it has
the following facts:

* It doesn't create code redundancy. I mean, it does not add unnecessary
   code to the kernel.

* Out of tree platforms will get access to this functionality by a simple
   "select UHI_BOOT". When the kernel developer of an out-of-tree platform
   wants to make kexec work, they will naturally look at machine_kexec.c,
   where they will find this UHI stuff, obviously telling them to do
   "select UHI_BOOT". Otherwise, unless they google onto this discussion
   thread, it's harder to know the solution to the kexec_args related
   problem hides in the code of another platform (Generic).

* It simplifies work if the out of tree platform wants to upstream.

>
> Could you take a look & let me know if you see any problems?
>
> Thanks,
>      Paul

Thanks,

Dengcheng

---------------------------------------------------------------------------

*From:* Paul Burton <mailto:paul.burton@mips.com>
*Sent:* Wednesday, September 05, 2018 3:54PM
*To:* Dengcheng Zhu <mailto:dzhu@wavecomp.com>
*Cc:* Pburton <mailto:pburton@wavecomp.com>, Ralf 
<mailto:ralf@linux-mips.org>, Linux-mips 
<mailto:linux-mips@linux-mips.org>, Rachel.mozes 
<mailto:rachel.mozes@intel.com>
*Subject:* Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other 
issues

Hi Dengcheng,

On Wed, Sep 05, 2018 at 08:59:03AM -0700, Dengcheng Zhu wrote:

> The issues are mentioned in patches 1/4/5/6. I will update kdump
> documentation for MIPS if the series gets accepted. Testing has been done
> on single core i6500/Boston with IOCU, dual core i6500 without IOCU, and
> dual core interAptiv without IOCU.
>
> Changes:
>
> v4 - v3:
> * In patch #1, idle_task_exit() is moved out from play_dead() to its sole
>    caller arch_cpu_idle_dead(). So no interface change of play_dead().
> * In patch #6, the kexec_prepare method for the Generic platform is defined
>    as uhi_machine_kexec_prepare() for all platforms using UHI boot protocol.

Thanks! I've applied patches 1,2,3,5 to a test branch with a few
changes:

     git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git test-kexec

     https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/log/?h=test-kexec

I didn't apply patch 4 because I'm not sure it's correct & I believe the
changes in the branch above should take care of it - CPUs that reach
kexec_this_cpu() are maked offline, so they shouldn't be IPI'd by
__flush_cache_all(). The CPU that runs machine_kexec() should still
flush its dcache (& the L2), and then CPU 0 invalidates its icache in
kexec_this_cpu() prior to jumping into reboot_code_buffer.

I'm also still not sure about patch 6 - since no platforms besides the
arch/mips/generic/ make use of the UHI boot code yet I think it'd be
best to leave as-is. If we do ever need to use it from another platform
then we can deal with the problem then. If an out of tree platform needs
to use it then for now it could copy generic_kexec_prepare() and deal
with removing the duplication when it heads upstream.

Could you take a look & let me know if you see any problems?

Thanks,
     Paul

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Paul Burton]

Hi Dengcheng,

On Thu, Sep 06, 2018 at 12:19:49PM -0700, Dengcheng Zhu wrote:
> > I didn't apply patch 4 because I'm not sure it's correct & I believe the
> > changes in the branch above should take care of it - CPUs that reach
> > kexec_this_cpu() are maked offline, so they shouldn't be IPI'd by
> > __flush_cache_all().
> 
> I believe patch 4 is necessary. As mentioned in the code comment and patch
> comment of that patch, machine_crash_shutdown() is called prior to
> machine_kexec() in the kdump sequence. So other CPUs have disabled local
> IRQs waiting for the reboot signal.
> 
> In fact, in kexec_this_cpu() [you renamed and modified kexec_smp_reboot()],
> the added marking CPU offline will cause system hang (tested). This is
> because it will change how play_dead() will work.

So the problem is that patch 4 isn't really right either, and I suspect
the hang you mention probably shows a problem with the whole play_dead()
approach from patches 1 & 2 too.

On the cache flushing, what we really need is for stores performed by
the CPU running machine_kexec() via its dcache to be observed by the
icache of the CPU that jumps into reboot_code_buffer, which is always
CPU 0 after patch 2. Using local_flush_icache_range() will only ensure
that the icache of the CPU running machine_kexec() observes the changes
made by that same CPU, and does nothing with CPU 0 unless they happen to
be the same CPU or siblings sharing caches.

Consider I6x00 CPUs for example - patch 4 may *almost* work OK because
both cpu_has_ic_fills_f_dc & cpu_icache_snoops_remote_store are true, so
when the machine_kexec() CPU writes to reboot_code_buffer CPU 0's icache
will observe that & will fill from dcache without needing it to be
written back to L2. It's still not quite right because if CPU 0's icache
already contained any valid lines within reboot_code_buffer (which could
happen speculatively) then it'll execute stale/garbage code.

The hang you mention is likely down to the fact that play_dead(), at
least the smp-cps.c implementation of it, is written for the CPU hotplug
infrastructure which will only operate on one CPU at a time. The loop
looking for a sibling CPU just isn't going to be safe to run on multiple
CPUs in parallel like patch 2 does. That's true of either your original
patch or my modification - it's just that my modified patch will cause
siblings to race towards powering off the core, whilst your original
will cause them to all halt themselves & never power off the core.

Which inclines me to return to my earlier thought that perhaps we
shouldn't use play_dead() for this at all.

> > The CPU that runs machine_kexec() should still
> > flush its dcache (& the L2), and then CPU 0 invalidates its icache in
> > kexec_this_cpu() prior to jumping into reboot_code_buffer.
> > 
> > I'm also still not sure about patch 6 - since no platforms besides the
> > arch/mips/generic/ make use of the UHI boot code yet I think it'd be
> > best to leave as-is. If we do ever need to use it from another platform
> > then we can deal with the problem then. If an out of tree platform needs
> > to use it then for now it could copy generic_kexec_prepare() and deal
> > with removing the duplication when it heads upstream.
> 
> Understood. It really depends on how this problem is viewed. If patch #6
> is considered creating a framework for future UHI platforms, then it has
> the following facts:
> 
> * It doesn't create code redundancy. I mean, it does not add unnecessary
>   code to the kernel.
> 
> * Out of tree platforms will get access to this functionality by a simple
>   "select UHI_BOOT". When the kernel developer of an out-of-tree platform
>   wants to make kexec work, they will naturally look at machine_kexec.c,
>   where they will find this UHI stuff, obviously telling them to do
>   "select UHI_BOOT". Otherwise, unless they google onto this discussion
>   thread, it's harder to know the solution to the kexec_args related
>   problem hides in the code of another platform (Generic).
> 
> * It simplifies work if the out of tree platform wants to upstream.

Well, ideally future platforms will just use arch/mips/generic rather
than adding more platform/board code at all. Right now the only reason
not to is if a system has a memory map that doesn't fit nicely, which
should be resolved at some point by mapping the kernel which will allow
the generic kernel to better support differing physical address maps.

So I hope we don't add more platform code anyway, which would make all
this moot. I certainly don't want to encourage adding more by explicitly
making it easier to do. And if there does come a point where we need to
add more platform code for some good reason then we can deal with this
at that point rather than speculating now.

For out of tree platforms, I don't think that copying the
generic_kexec_prepare() function is particularly onerous anyway, and
should be trivial to remove if such code is submitted upstream. Whilst I
wouldn't want to make submitting code upstream more difficult, it's
likely to need some amount of rework if submitted upstream anyway so
this doesn't seem like a big deal.

I don't think upstream should be particularly concerned with making life
easy for out of tree code - if one wants the benefit of their code being
taken into account upstream, then one should submit one's code upstream.
In the paraphrased words of my pastor at a marriage prep class - no
benefits (except those already conferred by free access to open source
software) without commitment.

Thanks,
    Paul

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

Hi Paul,


Regarding cache flushing in machine_kexec(), I recommend simply removing
__flush_cache_all() as CPU0 will do local icache flush before jumping to
reboot_code_buffer, and other CPUs don't jump at all.

Re: marking CPU offline in kexec_this_cpu(), it's probably good NOT doing
it either, as the system is going to reboot from CPU0 soon. HALT is good
enough, no need to gate core power. As to whether it's safe running
play_dead() in parallel, it shouldn't be a problem, as the loop is based
on cpu online mask (which we don't mark offline as mentioned above) -- The
CPU will simply halt itself. For non-MT CPUs, play_dead() does make sense
as well, as it's supposed to stop this CPU's execution, getting ready to
reboot from CPU0.

On UHI stuff, if it's true "future platforms will just use
arch/mips/generic", then certainly no need of getting code out of Generic.
My feeling is the customer is NOT doing so, according to "I fixed it by
moving the code to another file". Other MIPS systems may be like this as
well. I understand your "upstream-or-copy-code" point, but the only thing,
which IMO meaningfully justifies patch 6 *for_such_cases*, is already said:

-----------------

When the kernel developer of an out-of-tree platform
   wants to make kexec work, they will naturally look at machine_kexec.c,
   where they will find this UHI stuff, obviously telling them to do
   "select UHI_BOOT". Otherwise, unless they google onto this discussion
   thread, it's harder to know the solution to the kexec_args related
   problem hides in the code of another platform (Generic).

-----------------

In other words, it may take considerable efforts to realize "copying the
generic_kexec_prepare() function" can solve the problem. Copying code itself
is certainly not onerous.


Regards,

Dengcheng


On 09/06/2018 01:34 PM, Paul Burton wrote:
> Hi Dengcheng,
>
> On Thu, Sep 06, 2018 at 12:19:49PM -0700, Dengcheng Zhu wrote:
>>> I didn't apply patch 4 because I'm not sure it's correct & I believe the
>>> changes in the branch above should take care of it - CPUs that reach
>>> kexec_this_cpu() are maked offline, so they shouldn't be IPI'd by
>>> __flush_cache_all().
>> I believe patch 4 is necessary. As mentioned in the code comment and patch
>> comment of that patch, machine_crash_shutdown() is called prior to
>> machine_kexec() in the kdump sequence. So other CPUs have disabled local
>> IRQs waiting for the reboot signal.
>>
>> In fact, in kexec_this_cpu() [you renamed and modified kexec_smp_reboot()],
>> the added marking CPU offline will cause system hang (tested). This is
>> because it will change how play_dead() will work.
> So the problem is that patch 4 isn't really right either, and I suspect
> the hang you mention probably shows a problem with the whole play_dead()
> approach from patches 1 & 2 too.
>
> On the cache flushing, what we really need is for stores performed by
> the CPU running machine_kexec() via its dcache to be observed by the
> icache of the CPU that jumps into reboot_code_buffer, which is always
> CPU 0 after patch 2. Using local_flush_icache_range() will only ensure
> that the icache of the CPU running machine_kexec() observes the changes
> made by that same CPU, and does nothing with CPU 0 unless they happen to
> be the same CPU or siblings sharing caches.
>
> Consider I6x00 CPUs for example - patch 4 may *almost* work OK because
> both cpu_has_ic_fills_f_dc & cpu_icache_snoops_remote_store are true, so
> when the machine_kexec() CPU writes to reboot_code_buffer CPU 0's icache
> will observe that & will fill from dcache without needing it to be
> written back to L2. It's still not quite right because if CPU 0's icache
> already contained any valid lines within reboot_code_buffer (which could
> happen speculatively) then it'll execute stale/garbage code.
>
> The hang you mention is likely down to the fact that play_dead(), at
> least the smp-cps.c implementation of it, is written for the CPU hotplug
> infrastructure which will only operate on one CPU at a time. The loop
> looking for a sibling CPU just isn't going to be safe to run on multiple
> CPUs in parallel like patch 2 does. That's true of either your original
> patch or my modification - it's just that my modified patch will cause
> siblings to race towards powering off the core, whilst your original
> will cause them to all halt themselves & never power off the core.
>
> Which inclines me to return to my earlier thought that perhaps we
> shouldn't use play_dead() for this at all.
>
>>> The CPU that runs machine_kexec() should still
>>> flush its dcache (& the L2), and then CPU 0 invalidates its icache in
>>> kexec_this_cpu() prior to jumping into reboot_code_buffer.
>>>
>>> I'm also still not sure about patch 6 - since no platforms besides the
>>> arch/mips/generic/ make use of the UHI boot code yet I think it'd be
>>> best to leave as-is. If we do ever need to use it from another platform
>>> then we can deal with the problem then. If an out of tree platform needs
>>> to use it then for now it could copy generic_kexec_prepare() and deal
>>> with removing the duplication when it heads upstream.
>> Understood. It really depends on how this problem is viewed. If patch #6
>> is considered creating a framework for future UHI platforms, then it has
>> the following facts:
>>
>> * It doesn't create code redundancy. I mean, it does not add unnecessary
>>    code to the kernel.
>>
>> * Out of tree platforms will get access to this functionality by a simple
>>    "select UHI_BOOT". When the kernel developer of an out-of-tree platform
>>    wants to make kexec work, they will naturally look at machine_kexec.c,
>>    where they will find this UHI stuff, obviously telling them to do
>>    "select UHI_BOOT". Otherwise, unless they google onto this discussion
>>    thread, it's harder to know the solution to the kexec_args related
>>    problem hides in the code of another platform (Generic).
>>
>> * It simplifies work if the out of tree platform wants to upstream.
> Well, ideally future platforms will just use arch/mips/generic rather
> than adding more platform/board code at all. Right now the only reason
> not to is if a system has a memory map that doesn't fit nicely, which
> should be resolved at some point by mapping the kernel which will allow
> the generic kernel to better support differing physical address maps.
>
> So I hope we don't add more platform code anyway, which would make all
> this moot. I certainly don't want to encourage adding more by explicitly
> making it easier to do. And if there does come a point where we need to
> add more platform code for some good reason then we can deal with this
> at that point rather than speculating now.
>
> For out of tree platforms, I don't think that copying the
> generic_kexec_prepare() function is particularly onerous anyway, and
> should be trivial to remove if such code is submitted upstream. Whilst I
> wouldn't want to make submitting code upstream more difficult, it's
> likely to need some amount of rework if submitted upstream anyway so
> this doesn't seem like a big deal.
>
> I don't think upstream should be particularly concerned with making life
> easy for out of tree code - if one wants the benefit of their code being
> taken into account upstream, then one should submit one's code upstream.
> In the paraphrased words of my pastor at a marriage prep class - no
> benefits (except those already conferred by free access to open source
> software) without commitment.
>
> Thanks,
>      Paul




Dengcheng

---------------------------------------------------------------------------

*From:* Paul Burton <mailto:paul.burton@mips.com>
*Sent:* Thursday, September 06, 2018 1:34PM
*To:* Dengcheng Zhu <mailto:dzhu@wavecomp.com>
*Cc:* Pburton <mailto:pburton@wavecomp.com>, Ralf 
<mailto:ralf@linux-mips.org>, Linux-mips 
<mailto:linux-mips@linux-mips.org>, Rachel.mozes 
<mailto:rachel.mozes@intel.com>
*Subject:* Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other 
issues

Hi Dengcheng,

On Thu, Sep 06, 2018 at 12:19:49PM -0700, Dengcheng Zhu wrote:

>> I didn't apply patch 4 because I'm not sure it's correct & I believe the
>> changes in the branch above should take care of it - CPUs that reach
>> kexec_this_cpu() are maked offline, so they shouldn't be IPI'd by
>> __flush_cache_all().
> I believe patch 4 is necessary. As mentioned in the code comment and patch
> comment of that patch, machine_crash_shutdown() is called prior to
> machine_kexec() in the kdump sequence. So other CPUs have disabled local
> IRQs waiting for the reboot signal.
>
> In fact, in kexec_this_cpu() [you renamed and modified kexec_smp_reboot()],
> the added marking CPU offline will cause system hang (tested). This is
> because it will change how play_dead() will work.

So the problem is that patch 4 isn't really right either, and I suspect
the hang you mention probably shows a problem with the whole play_dead()
approach from patches 1 & 2 too.

On the cache flushing, what we really need is for stores performed by
the CPU running machine_kexec() via its dcache to be observed by the
icache of the CPU that jumps into reboot_code_buffer, which is always
CPU 0 after patch 2. Using local_flush_icache_range() will only ensure
that the icache of the CPU running machine_kexec() observes the changes
made by that same CPU, and does nothing with CPU 0 unless they happen to
be the same CPU or siblings sharing caches.

Consider I6x00 CPUs for example - patch 4 may *almost* work OK because
both cpu_has_ic_fills_f_dc & cpu_icache_snoops_remote_store are true, so
when the machine_kexec() CPU writes to reboot_code_buffer CPU 0's icache
will observe that & will fill from dcache without needing it to be
written back to L2. It's still not quite right because if CPU 0's icache
already contained any valid lines within reboot_code_buffer (which could
happen speculatively) then it'll execute stale/garbage code.

The hang you mention is likely down to the fact that play_dead(), at
least the smp-cps.c implementation of it, is written for the CPU hotplug
infrastructure which will only operate on one CPU at a time. The loop
looking for a sibling CPU just isn't going to be safe to run on multiple
CPUs in parallel like patch 2 does. That's true of either your original
patch or my modification - it's just that my modified patch will cause
siblings to race towards powering off the core, whilst your original
will cause them to all halt themselves & never power off the core.

Which inclines me to return to my earlier thought that perhaps we
shouldn't use play_dead() for this at all.

>> The CPU that runs machine_kexec() should still
>> flush its dcache (& the L2), and then CPU 0 invalidates its icache in
>> kexec_this_cpu() prior to jumping into reboot_code_buffer.
>>
>> I'm also still not sure about patch 6 - since no platforms besides the
>> arch/mips/generic/ make use of the UHI boot code yet I think it'd be
>> best to leave as-is. If we do ever need to use it from another platform
>> then we can deal with the problem then. If an out of tree platform needs
>> to use it then for now it could copy generic_kexec_prepare() and deal
>> with removing the duplication when it heads upstream.
> Understood. It really depends on how this problem is viewed. If patch #6
> is considered creating a framework for future UHI platforms, then it has
> the following facts:
>
> * It doesn't create code redundancy. I mean, it does not add unnecessary
>    code to the kernel.
>
> * Out of tree platforms will get access to this functionality by a simple
>    "select UHI_BOOT". When the kernel developer of an out-of-tree platform
>    wants to make kexec work, they will naturally look at machine_kexec.c,
>    where they will find this UHI stuff, obviously telling them to do
>    "select UHI_BOOT". Otherwise, unless they google onto this discussion
>    thread, it's harder to know the solution to the kexec_args related
>    problem hides in the code of another platform (Generic).
>
> * It simplifies work if the out of tree platform wants to upstream.

Well, ideally future platforms will just use arch/mips/generic rather
than adding more platform/board code at all. Right now the only reason
not to is if a system has a memory map that doesn't fit nicely, which
should be resolved at some point by mapping the kernel which will allow
the generic kernel to better support differing physical address maps.

So I hope we don't add more platform code anyway, which would make all
this moot. I certainly don't want to encourage adding more by explicitly
making it easier to do. And if there does come a point where we need to
add more platform code for some good reason then we can deal with this
at that point rather than speculating now.

For out of tree platforms, I don't think that copying the
generic_kexec_prepare() function is particularly onerous anyway, and
should be trivial to remove if such code is submitted upstream. Whilst I
wouldn't want to make submitting code upstream more difficult, it's
likely to need some amount of rework if submitted upstream anyway so
this doesn't seem like a big deal.

I don't think upstream should be particularly concerned with making life
easy for out of tree code - if one wants the benefit of their code being
taken into account upstream, then one should submit one's code upstream.
In the paraphrased words of my pastor at a marriage prep class - no
benefits (except those already conferred by free access to open source
software) without commitment.

Thanks,
     Paul

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Paul Burton]

Hi Dengcheng,

On Thu, Sep 06, 2018 at 03:23:13PM -0700, Dengcheng Zhu wrote:
> Regarding cache flushing in machine_kexec(), I recommend simply removing
> __flush_cache_all() as CPU0 will do local icache flush before jumping to
> reboot_code_buffer, and other CPUs don't jump at all.

Unfortunately that doesn't work - what if CPU 1 runs machine_kexec() &
writes to reboot_code_buffer, then CPU 0 fetches stale/garbage code from
L2 into its icache?

The icache flush I added in kexec_this_cpu() isn't enough in all systems
unless the CPU that wrote the code writes it back far enough for a
remote CPU to observe. The existing __flush_cache_all() is one
heavy-handed way to achieve that.

> Re: marking CPU offline in kexec_this_cpu(), it's probably good NOT doing
> it either, as the system is going to reboot from CPU0 soon.

...but the new kernel will have no knowledge of whether the old kernel
had CPUs marked online or offline, so I don't follow the argument?

Marking CPUs offline does have the advantage that we won't try to IPI
them. This just makes perfect sense to me, and note that both arch/arm &
arch/x86 offline CPUs during kexec too (I *really* like the way arch/arm
just uses disable_nonboot_cpus() to go through the usual hotplug
sequence in the non-crash case).

> HALT is good enough, no need to gate core power.
> As to whether it's safe running play_dead() in parallel, it shouldn't
> be a problem, as the loop is based
> on cpu online mask (which we don't mark offline as mentioned above) -- The
> CPU will simply halt itself. For non-MT CPUs, play_dead() does make sense
> as well, as it's supposed to stop this CPU's execution, getting ready to
> reboot from CPU0.

That feels like relying on play_dead() accidentally working rather than
doing something it's designed for though, and it would come at the
expense of not marking CPUs offline & the associated fragility of
needing to avoid anything that might IPI them (like some cache flush
functions, as we've seen). I'd much rather we figure out a way to do
this without all that.

One option might be to add something like arch/x86's stop_other_cpus &
crash_stop_other_cpus callbacks in struct plat_smp_ops, which we can
then implement as appropriate. We'd hopefully still reuse some of the
code from play_dead() implementations, but have the separation to allow
them to function differently where needed (eg. the new callbacks could
halt all threads in MT systems without caring about cpu_online_mask).

This would also give us a natural way to check whether a system actually
supports kexec properly, as we could just return with an error from
machine_kexec_prepare() if the stop_other_cpus callback isn't
implemented for the system (ie. is NULL).

Thanks,
    Paul

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

Hi Paul,


On cache issue, what if kexec_this_cpu() does local i$ flush and scache
invalidation? This is done only on CPU0, no IPI needed. And only CUP0 will use
reboot_code_buffer. In this way, marking other CPUs offline to prevent being
sent IPI is not needed.

As to "the associated fragility of needing to avoid anything that might IPI
them", I hope that's not a problem as the kexec sequence is determined: The last
IPI received is to ask them to stand by on reboot signal, because other than
kexec there is no other kernel code beyond our control, which might send IPIs.

> ...but the new kernel will have no knowledge of whether the old kernel
had CPUs marked online or offline, so I don't follow the argument?

[Dengcheng] I meant the new kernel shouldn't need CPU online info from the old
kernel. The new one is doing a "fresh" boot, provided CPUs 1+ have been halted.

Regarding the use of play_dead(), it's simple and generic - MIPS CPUs have
implemented their own play_dead(), which we can use to stop execution. Other
options may still come down to play_dead() to handle MT/MC details. The concern
of running it in parallel is really about the online status, which has been
explained above and in my last email.

A 'nicer' implementation stopping non-boot CPUs from one CPU might be available,
but handling code duplication of play_dead() and saving CPU states could be a
problem.

For now, no __flush_cache_all() in machine_kexec() + no marking offline in
kexec_this_cpu() + scache invalidation in kexec_this_cpu() might be a way to go.


Thanks,

Dengcheng

---

From: Paul Burton
Sent: Thursday, September 6, 2018 4:21 PM
To: Dengcheng Zhu
Cc: Paul Burton; ralf@linux-mips.org; linux-mips@linux-mips.org; rachel.mozes@intel.com
Subject: Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues
  

Hi Dengcheng,

On Thu, Sep 06, 2018 at 03:23:13PM -0700, Dengcheng Zhu wrote:
> Regarding cache flushing in machine_kexec(), I recommend simply removing
> __flush_cache_all() as CPU0 will do local icache flush before jumping to
> reboot_code_buffer, and other CPUs don't jump at all.

Unfortunately that doesn't work - what if CPU 1 runs machine_kexec() &
writes to reboot_code_buffer, then CPU 0 fetches stale/garbage code from
L2 into its icache?

The icache flush I added in kexec_this_cpu() isn't enough in all systems
unless the CPU that wrote the code writes it back far enough for a
remote CPU to observe. The existing __flush_cache_all() is one
heavy-handed way to achieve that.

> Re: marking CPU offline in kexec_this_cpu(), it's probably good NOT doing
> it either, as the system is going to reboot from CPU0 soon.

...but the new kernel will have no knowledge of whether the old kernel
had CPUs marked online or offline, so I don't follow the argument?

Marking CPUs offline does have the advantage that we won't try to IPI
them. This just makes perfect sense to me, and note that both arch/arm &
arch/x86 offline CPUs during kexec too (I *really* like the way arch/arm
just uses disable_nonboot_cpus() to go through the usual hotplug
sequence in the non-crash case).

> HALT is good enough, no need to gate core power.
> As to whether it's safe running play_dead() in parallel, it shouldn't
> be a problem, as the loop is based
> on cpu online mask (which we don't mark offline as mentioned above) -- The
> CPU will simply halt itself. For non-MT CPUs, play_dead() does make sense
> as well, as it's supposed to stop this CPU's execution, getting ready to
> reboot from CPU0.

That feels like relying on play_dead() accidentally working rather than
doing something it's designed for though, and it would come at the
expense of not marking CPUs offline & the associated fragility of
needing to avoid anything that might IPI them (like some cache flush
functions, as we've seen). I'd much rather we figure out a way to do
this without all that.

One option might be to add something like arch/x86's stop_other_cpus &
crash_stop_other_cpus callbacks in struct plat_smp_ops, which we can
then implement as appropriate. We'd hopefully still reuse some of the
code from play_dead() implementations, but have the separation to allow
them to function differently where needed (eg. the new callbacks could
halt all threads in MT systems without caring about cpu_online_mask).

This would also give us a natural way to check whether a system actually
supports kexec properly, as we could just return with an error from
machine_kexec_prepare() if the stop_other_cpus callback isn't
implemented for the system (ie. is NULL).

Thanks,
    Paul
    
From robh+dt@kernel.org Fri Sep  7 22:29:23 2018
Received: with ECARTIS (v1.0.0; list linux-mips); Fri, 07 Sep 2018 22:29:30 +0200 (CEST)
Received: from mail.kernel.org ([198.145.29.99]:53582 "EHLO mail.kernel.org"
        rhost-flags-OK-OK-OK-OK) by eddie.linux-mips.org with ESMTP
        id S23994660AbeIGU3W4apiN (ORCPT <rfc822;linux-mips@linux-mips.org>);
        Fri, 7 Sep 2018 22:29:22 +0200
Received: from mail-qt0-f176.google.com (mail-qt0-f176.google.com [209.85.216.176])
        (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
        (No client certificate requested)
        by mail.kernel.org (Postfix) with ESMTPSA id F21042087A
        for <linux-mips@linux-mips.org>; Fri,  7 Sep 2018 20:29:15 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;
        s=default; t=1536352156;
        bh=qaTN1sLAoeF2hUo9r8Pc8zfcetm74aPR5ccLPi03ij0=;
        h=References:In-Reply-To:From:Date:Subject:To:Cc:From;
        b=0SUTw9D9BeFNMT5o0w5IOq9Qq4GPH8asGBAGCSdLD+82/wgI21oSiel29VgRO7o9r
         TFsXjXPpeSVNZrPUwwtffzR4uEBF6N0e3nhzggKjy3fb8AcNX6hRW8HnqH2TTyaKpG
         TuG9BaZggjEc1PCQ62qDD99KtyTZXb6C7kT18FJg=
Received: by mail-qt0-f176.google.com with SMTP id h4-v6so17605621qtj.7
        for <linux-mips@linux-mips.org>; Fri, 07 Sep 2018 13:29:15 -0700 (PDT)
X-Gm-Message-State: APzg51A5FB3qFRoe7k9J/C3J43UR8F96WmERZydouBPdTeXBxrJB+jh7
        je6rvBEhKoEa/OHnmmdJAPahp3G+96wzh9N09g==
X-Google-Smtp-Source: ANB0Vdab3hT61u1M8M/lHOL0mWGQ/USdGai/hWOrTMd+yeGvnL96ZvpFeSxppjPt10TdKLI1ogg9PQzqaJMzE9dHMxc=
X-Received: by 2002:a0c:db87:: with SMTP id m7-v6mr7110948qvk.90.1536352155200;
 Fri, 07 Sep 2018 13:29:15 -0700 (PDT)
MIME-Version: 1.0
References: <20180907185414.2630-1-paul.burton@mips.com>
In-Reply-To: <20180907185414.2630-1-paul.burton@mips.com>
From:   Rob Herring <robh+dt@kernel.org>
Date:   Fri, 7 Sep 2018 15:29:03 -0500
X-Gmail-Original-Message-ID: <CAL_Jsq+n4e5_ptuh89CJibViGS_bgHz0A+Ki-uwtcGU38+mHXQ@mail.gmail.com>
Message-ID: <CAL_Jsq+n4e5_ptuh89CJibViGS_bgHz0A+Ki-uwtcGU38+mHXQ@mail.gmail.com>
Subject: Re: [PATCH 1/2] of/fdt: Allow architectures to override
 CONFIG_CMDLINE logic
To:     Paul Burton <paul.burton@mips.com>
Cc:     Linux-MIPS <linux-mips@linux-mips.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        devicetree@vger.kernel.org, Frank Rowand <frowand.list@gmail.com>,
        Jaedon Shin <jaedon.shin@gmail.com>,
        Mathieu Malaterre <malat@debian.org>,
        stable <stable@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Return-Path: <robh+dt@kernel.org>
X-Envelope-To: <"|/home/ecartis/ecartis -s linux-mips"> (uid 0)
X-Orcpt: rfc822;linux-mips@linux-mips.org
Original-Recipient: rfc822;linux-mips@linux-mips.org
X-archive-position: 66148
X-ecartis-version: Ecartis v1.0.0
Sender: linux-mips-bounce@linux-mips.org
Errors-to: linux-mips-bounce@linux-mips.org
X-original-sender: robh+dt@kernel.org
Precedence: bulk
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
List-software: Ecartis version 1.0.0
List-Id: linux-mips <linux-mips.eddie.linux-mips.org>
X-List-ID: linux-mips <linux-mips.eddie.linux-mips.org>
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
X-list: linux-mips
Content-Length: 2303
Lines: 46

On Fri, Sep 7, 2018 at 1:55 PM Paul Burton <paul.burton@mips.com> wrote:
>
> The CONFIG_CMDLINE-related logic in early_init_dt_scan_chosen() falls
> back to copying CONFIG_CMDLINE into boot_command_line/data if the DT has
> a /chosen node but that node has no bootargs property or a bootargs
> property of length zero.

The Risc-V guys found a similar issue if chosen is missing[1]. I
started a patch[2] to address that, but then looking at the different
arches wasn't sure if I'd break something. I don't recall for sure,
but it may have been MIPS that worried me.

> This is problematic for the MIPS architecture because we support
> concatenating arguments from either the DT or the bootloader with those
> from CONFIG_CMDLINE, but the behaviour of early_init_dt_scan_chosen()
> gives us no way of knowing whether boot_command_line contains arguments
> from DT or already contains CONFIG_CMDLINE. This can lead to us
> concatenating CONFIG_CMDLINE with itself, duplicating command line
> arguments which can be problematic (eg. for earlycon which will attempt
> to register the same console twice & warn about it).

If CONFIG_CMDLINE_EXTEND is set, you know it contains CONFIG_CMDLINE.
But I guess part of the problem is MIPS using its own kconfig options.

> Move the CONFIG_CMDLINE-related logic to a weak function that
> architectures can provide their own version of, such that we continue to
> use the existing logic for architectures where it's suitable but also
> allow MIPS to override this behaviour such that the architecture code
> knows when CONFIG_CMDLINE is used.

More arch specific functions is not what I want. Really, all the
cmdline manipulating logic doesn't belong in DT code, but it shouldn't
be in the arch specific code either IMO. Really it should be some
common kernel function which calls into the DT code to retrieve the DT
bootargs and that's it. Then you can skip calling that kernel function
if you really need non-standard handling.

Perhaps you should consider filling DT bootargs with the cmdline from
bootloader. IOW, make the legacy case look like the non-legacy case
early, and then the kernel doesn't have to deal with both cases later
on.

Rob

[1] https://lkml.org/lkml/2018/3/14/701
[2] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt/cmdline

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

Hi Paul,


On cache issue, what if kexec_this_cpu() does local i$ flush and scache
invalidation? This is done only on CPU0, no IPI needed. And only CUP0 will use
reboot_code_buffer. In this way, marking other CPUs offline to prevent being
sent IPI is not needed.

As to "the associated fragility of needing to avoid anything that might IPI
them", I hope that's not a problem as the kexec sequence is determined: The last
IPI received is to ask them to stand by on reboot signal, because other than
kexec there is no other kernel code beyond our control, which might send IPIs.

> ...but the new kernel will have no knowledge of whether the old kernel
had CPUs marked online or offline, so I don't follow the argument?

[Dengcheng] I meant the new kernel shouldn't need CPU online info from the old
kernel. The new one is doing a "fresh" boot, provided CPUs 1+ have been halted.

Regarding the use of play_dead(), it's simple and generic - MIPS CPUs have
implemented their own play_dead(), which we can use to stop execution. Other
options may still come down to play_dead() to handle MT/MC details. The concern
of running it in parallel is really about the online status, which has been
explained above and in my last email.

A 'nicer' implementation stopping non-boot CPUs from one CPU might be available,
but handling code duplication of play_dead() and saving CPU states could be a
problem.

For now, no __flush_cache_all() in machine_kexec() + no marking offline in
kexec_this_cpu() + scache invalidation in kexec_this_cpu() might be a way to go.


Thanks,

Dengcheng

---

From: Paul Burton
Sent: Thursday, September 6, 2018 4:21 PM
To: Dengcheng Zhu
Cc: Paul Burton; ralf@linux-mips.org; linux-mips@linux-mips.org; rachel.mozes@intel.com
Subject: Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues
  

Hi Dengcheng,

On Thu, Sep 06, 2018 at 03:23:13PM -0700, Dengcheng Zhu wrote:
> Regarding cache flushing in machine_kexec(), I recommend simply removing
> __flush_cache_all() as CPU0 will do local icache flush before jumping to
> reboot_code_buffer, and other CPUs don't jump at all.

Unfortunately that doesn't work - what if CPU 1 runs machine_kexec() &
writes to reboot_code_buffer, then CPU 0 fetches stale/garbage code from
L2 into its icache?

The icache flush I added in kexec_this_cpu() isn't enough in all systems
unless the CPU that wrote the code writes it back far enough for a
remote CPU to observe. The existing __flush_cache_all() is one
heavy-handed way to achieve that.

> Re: marking CPU offline in kexec_this_cpu(), it's probably good NOT doing
> it either, as the system is going to reboot from CPU0 soon.

...but the new kernel will have no knowledge of whether the old kernel
had CPUs marked online or offline, so I don't follow the argument?

Marking CPUs offline does have the advantage that we won't try to IPI
them. This just makes perfect sense to me, and note that both arch/arm &
arch/x86 offline CPUs during kexec too (I *really* like the way arch/arm
just uses disable_nonboot_cpus() to go through the usual hotplug
sequence in the non-crash case).

> HALT is good enough, no need to gate core power.
> As to whether it's safe running play_dead() in parallel, it shouldn't
> be a problem, as the loop is based
> on cpu online mask (which we don't mark offline as mentioned above) -- The
> CPU will simply halt itself. For non-MT CPUs, play_dead() does make sense
> as well, as it's supposed to stop this CPU's execution, getting ready to
> reboot from CPU0.

That feels like relying on play_dead() accidentally working rather than
doing something it's designed for though, and it would come at the
expense of not marking CPUs offline & the associated fragility of
needing to avoid anything that might IPI them (like some cache flush
functions, as we've seen). I'd much rather we figure out a way to do
this without all that.

One option might be to add something like arch/x86's stop_other_cpus &
crash_stop_other_cpus callbacks in struct plat_smp_ops, which we can
then implement as appropriate. We'd hopefully still reuse some of the
code from play_dead() implementations, but have the separation to allow
them to function differently where needed (eg. the new callbacks could
halt all threads in MT systems without caring about cpu_online_mask).

This would also give us a natural way to check whether a system actually
supports kexec properly, as we could just return with an error from
machine_kexec_prepare() if the stop_other_cpus callback isn't
implemented for the system (ie. is NULL).

Thanks,
    Paul
    

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Paul Burton]

Hi Dengcheng,

On Fri, Sep 07, 2018 at 12:47:45PM -0700, Dengcheng Zhu wrote:
> On cache issue, what if kexec_this_cpu() does local i$ flush and scache
> invalidation? This is done only on CPU0, no IPI needed. And only CUP0 will use
> reboot_code_buffer. In this way, marking other CPUs offline to prevent being
> sent IPI is not needed.

The problem is that the "only CPU0 will use reboot_code_buffer" part
isn't true because CPU0 might not be the one that writes to it. Though
perhaps if we moved the memcpy() from machine_kexec() to
kexec_this_cpu() that could work.

It still doesn't seem ideal to me that we'd have to even consider IPIs &
the fact that we might silently hang if we accidentally send one. I
really think it would be cleaner to mark CPUs offline & the problem is
solved naturally - we shouldn't IPI these CPUs, and marking them offline
means that we won't. Problem solved, and consistently with other
architectures.

> As to "the associated fragility of needing to avoid anything that might IPI
> them", I hope that's not a problem as the kexec sequence is determined: The last
> IPI received is to ask them to stand by on reboot signal, because other than
> kexec there is no other kernel code beyond our control, which might send IPIs.

But it does mean we can't call certain things, like __flush_cache_all(),
which means that developers need to 1) know that and 2) remember to
avoid any functions like this that perform an IPI behind the scenes. It
just feels like loading a gun & leaving it pointed at our feet - by
itself it's not necessarily going to cause a problem, but it would be oh
so easy to trigger problems later.

> > ...but the new kernel will have no knowledge of whether the old kernel
> had CPUs marked online or offline, so I don't follow the argument?
> 
> [Dengcheng] I meant the new kernel shouldn't need CPU online info from the old
> kernel. The new one is doing a "fresh" boot, provided CPUs 1+ have been halted.
> 
> Regarding the use of play_dead(), it's simple and generic - MIPS CPUs have
> implemented their own play_dead(), which we can use to stop execution. Other
> options may still come down to play_dead() to handle MT/MC details. The concern
> of running it in parallel is really about the online status, which has been
> explained above and in my last email.

But play_dead() really doesn't mean what you're suggesting it means.

What we want for kexec is that the CPU is stopped entirely - either
halted or powered down. Failing that it needs to be running a loop from
somewhere that we know isn't going to be overwritten, and the code it's
running needs to be able to hand over to the new kernel later (eg. the
existing kexec_smp_wait loop implements this latter approach).

Whilst the smp-cps.c implementation of play_dead() will currently halt
or power down the CPU, that's not universally true & doesn't necessarily
even need to remain true for smp-cps.c in the future (eg. we could
decide the halt/power down step would be cleaner to do in cps_cpu_die).

A quick look at the other play_dead() implementations we have shows that
both the cavium-octeon & loongson3 versions leave the CPU running a
loop (within the current/old kernel), whilst the bmips version just sits
at a wait instruction before jumping back to the fixed location of
bmips_secondary_reentry (in the current/old kernel) once interrupted.
None of those are suitable for kexec.

In general the CPU hotplug code just needs that when play_dead() runs on
the CPU going offline, and the struct plat_smp_ops cpu_die() callback
runs on some other CPU, the CPU being offlined will be quiesced to a
degree that it won't interfere with Linux. This is different to kexec
where we need the CPU to stop in such a way that it's fine that we might
overwrite the old kernel, and then be able to start it running the new
kernel later. As described before, that means it either needs to halt or
power down until the new kernel takes control of it or it needs to run
some loop from some location that we know won't be overwritten.

play_dead() is simple & already exists, it just doesn't do quite what we
need kexec to do outside of the one system you're looking at.

> A 'nicer' implementation stopping non-boot CPUs from one CPU might be available,
> but handling code duplication of play_dead() and saving CPU states could be a
> problem.

Code that can be shared between play_dead() & kexec can still be shared
if we have a different callback - the particular implementation of the
two would just have both call a common function for the part that's
common.

> For now, no __flush_cache_all() in machine_kexec() + no marking offline in
> kexec_this_cpu() + scache invalidation in kexec_this_cpu() might be a way to go.

For the system you're interested in, sure that works. But as I've
pointed out in my last few emails it does not work in general for other
systems that we also need to support in the kernel.

Thanks,
    Paul

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

Hi Paul,


> The problem is that the "only CPU0 will use reboot_code_buffer" part
isn't true because CPU0 might not be the one that writes to it.

[Dengcheng]: I didn't say CPU0 is always the one running machine_kexec().
Actually, in my testing, I intentionally did something like:

taskset -c 3 echo c > /proc/sysrq-trigger

And for "either halted or powered down", do you have any idea/plan for
Octeon, Loognson and bmips?


Thanks,

Dengcheng


From: Paul Burton
Sent: Friday, September 7, 2018 2:42 PM
To: Dengcheng Zhu
Cc: Paul Burton; ralf@linux-mips.org; linux-mips@linux-mips.org; rachel.mozes@intel.com
Subject: Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues
  

Hi Dengcheng,

On Fri, Sep 07, 2018 at 12:47:45PM -0700, Dengcheng Zhu wrote:
> On cache issue, what if kexec_this_cpu() does local i$ flush and scache
> invalidation? This is done only on CPU0, no IPI needed. And only CUP0 will use
> reboot_code_buffer. In this way, marking other CPUs offline to prevent being
> sent IPI is not needed.

The problem is that the "only CPU0 will use reboot_code_buffer" part
isn't true because CPU0 might not be the one that writes to it. Though
perhaps if we moved the memcpy() from machine_kexec() to
kexec_this_cpu() that could work.

It still doesn't seem ideal to me that we'd have to even consider IPIs &
the fact that we might silently hang if we accidentally send one. I
really think it would be cleaner to mark CPUs offline & the problem is
solved naturally - we shouldn't IPI these CPUs, and marking them offline
means that we won't. Problem solved, and consistently with other
architectures.

> As to "the associated fragility of needing to avoid anything that might IPI
> them", I hope that's not a problem as the kexec sequence is determined: The last
> IPI received is to ask them to stand by on reboot signal, because other than
> kexec there is no other kernel code beyond our control, which might send IPIs.

But it does mean we can't call certain things, like __flush_cache_all(),
which means that developers need to 1) know that and 2) remember to
avoid any functions like this that perform an IPI behind the scenes. It
just feels like loading a gun & leaving it pointed at our feet - by
itself it's not necessarily going to cause a problem, but it would be oh
so easy to trigger problems later.

> > ...but the new kernel will have no knowledge of whether the old kernel
> had CPUs marked online or offline, so I don't follow the argument?
> 
> [Dengcheng] I meant the new kernel shouldn't need CPU online info from the old
> kernel. The new one is doing a "fresh" boot, provided CPUs 1+ have been halted.
> 
> Regarding the use of play_dead(), it's simple and generic - MIPS CPUs have
> implemented their own play_dead(), which we can use to stop execution. Other
> options may still come down to play_dead() to handle MT/MC details. The concern
> of running it in parallel is really about the online status, which has been
> explained above and in my last email.

But play_dead() really doesn't mean what you're suggesting it means.

What we want for kexec is that the CPU is stopped entirely - either
halted or powered down. Failing that it needs to be running a loop from
somewhere that we know isn't going to be overwritten, and the code it's
running needs to be able to hand over to the new kernel later (eg. the
existing kexec_smp_wait loop implements this latter approach).

Whilst the smp-cps.c implementation of play_dead() will currently halt
or power down the CPU, that's not universally true & doesn't necessarily
even need to remain true for smp-cps.c in the future (eg. we could
decide the halt/power down step would be cleaner to do in cps_cpu_die).

A quick look at the other play_dead() implementations we have shows that
both the cavium-octeon & loongson3 versions leave the CPU running a
loop (within the current/old kernel), whilst the bmips version just sits
at a wait instruction before jumping back to the fixed location of
bmips_secondary_reentry (in the current/old kernel) once interrupted.
None of those are suitable for kexec.

In general the CPU hotplug code just needs that when play_dead() runs on
the CPU going offline, and the struct plat_smp_ops cpu_die() callback
runs on some other CPU, the CPU being offlined will be quiesced to a
degree that it won't interfere with Linux. This is different to kexec
where we need the CPU to stop in such a way that it's fine that we might
overwrite the old kernel, and then be able to start it running the new
kernel later. As described before, that means it either needs to halt or
power down until the new kernel takes control of it or it needs to run
some loop from some location that we know won't be overwritten.

play_dead() is simple & already exists, it just doesn't do quite what we
need kexec to do outside of the one system you're looking at.

> A 'nicer' implementation stopping non-boot CPUs from one CPU might be available,
> but handling code duplication of play_dead() and saving CPU states could be a
> problem.

Code that can be shared between play_dead() & kexec can still be shared
if we have a different callback - the particular implementation of the
two would just have both call a common function for the part that's
common.

> For now, no __flush_cache_all() in machine_kexec() + no marking offline in
> kexec_this_cpu() + scache invalidation in kexec_this_cpu() might be a way to go.

For the system you're interested in, sure that works. But as I've
pointed out in my last few emails it does not work in general for other
systems that we also need to support in the kernel.

Thanks,
    Paul
    
From pburton@wavecomp.com Sat Sep  8 01:12:37 2018
Received: with ECARTIS (v1.0.0; list linux-mips); Sat, 08 Sep 2018 01:12:43 +0200 (CEST)
Received: from mail-by2nam05on0718.outbound.protection.outlook.com ([IPv6:2a01:111:f400:fe52::718]:6445
        "EHLO NAM05-BY2-obe.outbound.protection.outlook.com"
        rhost-flags-OK-OK-OK-FAIL) by eddie.linux-mips.org with ESMTP
        id S23994663AbeIGXMhoRaf9 (ORCPT <rfc822;linux-mips@linux-mips.org>);
        Sat, 8 Sep 2018 01:12:37 +0200
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=wavesemi.onmicrosoft.com; s=selector1-wavecomp-com;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=w6TFXe7QB5xWbTn3QyaLYFooZxOGSPku7nnweuowODY=;
 b=YyTTZyZ1UiH1WoxJo6orsvGXD1p1m3pPtJCHFkCIeyVAZxxeSQvXatUZnp38QU4V9sHYL2jgu2DE6QFJlaLquRr4d02wurjVamDfCrcfXP54zhYsGpk+4PTB3rb5nnbcRtP95DYYGOjC4jp+sBJVavzDx/GsFeG9vzXIBqa8lHQ=
Authentication-Results: spf=none (sender IP is )
 smtp.mailfrom=pburton@wavecomp.com; 
Received: from localhost (4.16.204.77) by
 DM6PR08MB4940.namprd08.prod.outlook.com (2603:10b6:5:4b::21) with Microsoft
 SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.1080.17; Fri, 7 Sep 2018 23:11:56 +0000
Date:   Fri, 7 Sep 2018 16:11:54 -0700
From:   Paul Burton <paul.burton@mips.com>
To:     Dengcheng Zhu <dzhu@wavecomp.com>
Cc:     Paul Burton <pburton@wavecomp.com>,
        "ralf@linux-mips.org" <ralf@linux-mips.org>,
        "linux-mips@linux-mips.org" <linux-mips@linux-mips.org>,
        "rachel.mozes@intel.com" <rachel.mozes@intel.com>
Subject: Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues
Message-ID: <20180907231154.hnnhi2kns77hdnr2@pburton-laptop>
References: <20180905155909.30454-1-dzhu@wavecomp.com>
 <20180905225455.luh32536uei5je6m@pburton-laptop>
 <5B917DD5.6020009@wavecomp.com>
 <20180906203455.pap7lh5itrbp7ed2@pburton-laptop>
 <5B91A8D1.4060206@wavecomp.com>
 <20180906232122.wp72jwfnsbb2ps7b@pburton-laptop>
 <CO2PR0801MB21512A2FA3A3A718725DC035A2000@CO2PR0801MB2151.namprd08.prod.outlook.com>
 <20180907214232.du2fh422uwzhdusm@pburton-laptop>
 <CO2PR0801MB2151111E37EDDB45D93F0F64A2000@CO2PR0801MB2151.namprd08.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CO2PR0801MB2151111E37EDDB45D93F0F64A2000@CO2PR0801MB2151.namprd08.prod.outlook.com>
User-Agent: NeoMutt/20180716
X-Originating-IP: [4.16.204.77]
X-ClientProxiedBy: BYAPR02CA0024.namprd02.prod.outlook.com
 (2603:10b6:a02:ee::37) To DM6PR08MB4940.namprd08.prod.outlook.com
 (2603:10b6:5:4b::21)
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 7dec7c0c-21da-418b-3f4c-08d615174e94
X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:(7020095)(4652040)(8989137)(5600074)(711020)(2017052603328)(7153060)(7193020);SRVR:DM6PR08MB4940;
X-Microsoft-Exchange-Diagnostics: 1;DM6PR08MB4940;3:nGqBDRfqr+lUWfFGqTWPFI4QJuiHoEzXbYsfAKC3vulaQpEgHRRcEhYKW9P9p5dDOEPBz0fzhDqIrAliCD/ywV8WY8P7wztIKff8KmL6RDkgJB/ZCmXEye4XQ2Z5UBvTskp2LYvQx25Ck3bdV2sSohfe5CE85UPxZ2LYSnTr9ey6Kr4DmBblB/6SyWI9ynGnZ2xe66dzuAm9wDG9+YwQhmk9E3vbkPLRGmqnawJVnnCiAYgkBNQ7zrcvZ5MyR2lL;25:Lu1KIHbbPU0j5mBJUiNxD3bo3xzZ6VJI9MX9QZqNRWQB4Omi4VLLNqLquSJtUGfSI8tpLcFfowvMYMnWk/pr+fQhCfItmREHpsT9F9UCeTmUql09vQ2Na9TRdvTAOd49zOandiifXB3iDTKMvKLGzHT0/ldqEW7cZychIO2BNHCXtXyw9VXDp69GZZkOctR4JD5NnxovSDKqYi1C+554rYTWULWcviimYKTiXoNqwmG6yAT13gSCx7xq2kwiI2YVSmF/6r9E2a72D6oYxUyKzG83/ctbEEy3IAZLkh3ksLo2cjIzA2Sq8pSlONNlVYcoBSPQ84yyqeiIb9CpikClTw==;31:fqCc49N6dNaGYShN+WZNxlqAJj6/1XsJJy6+M0+kvJkLsoSKEs+qldLqJax/bgyJN77iLf3mbuuZ3N52KSHPna0HjrRRtWLmyLbClCrwmyad61lpCDPg6aaokY9khreHRBojg0xXTw4T636J3OyJzrGDDiPG2AEPaIzC8Ieb8DpGrcbeC/qjk8WeFV978Kz8ThFhkJbBpgAlZP46Ov6+4a33og7/sxY/+j5vrXaWW14=
X-MS-TrafficTypeDiagnostic: DM6PR08MB4940:
X-Microsoft-Exchange-Diagnostics: 1;DM6PR08MB4940;20:Y/ownOQxx4M0IFGye/vI7Hc7xxdTVzrzb1fII8JakQJ+FaF7IaxuA2NRCKOYAesIZAA93yndZ4MQY+pljJiMz+28Dmsvlregmr6ViW3a+3TFCu+AfXS3mnHfxai8dl3LzQnLgGhoYZ5fkXJOHYFGkZTetoQX8Ti/tMH3J3Fi1L016AcMB7ZNnNaTK4ppEYOQbVKBszVqTDV5XxqnqEYB8RdBg4qJ4b3hERjZPJDLbNfHRI4/REc6MReiyR6f027v;4:4BVGr4wnz5a6bqLbvwEwyCVZGemUAE6D8jpUbsxw/Nm36xZAmzYDrjqjCcYLFf8QehdF4m0p9xCmPEFjz5tbMUV95Urc/UA7B/V0yV2I9CrOCcb/sQOo78AiXSZugdhhCsmfCBR1ph6yMY4+MxhtbHNu7DVRXoeYPfRS7vy44rxoaL/aD/ShrVNz1EbkSZqpIujIdR1OhG+ST/c5EQbu0pTRoKcPnm29WDw11PiO0prqAvfuEf6AQ4jcTgMwAAUo/TgWDfoh6n4CnDRN4Lg8C4SMThlkTro6EXJ8MAHNwg7JXU6cGJDRc+wdMAmCi23a
X-Microsoft-Antispam-PRVS: <DM6PR08MB49401ACF4BDBE61B4267DF6CC1000@DM6PR08MB4940.namprd08.prod.outlook.com>
X-Exchange-Antispam-Report-Test: UriScan:(190756311086443);
X-MS-Exchange-SenderADCheck: 1
X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040522)(2401047)(8121501046)(5005006)(10201501046)(93006095)(3002001)(3231311)(944501410)(52105095)(149027)(150027)(6041310)(20161123558120)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123562045)(20161123560045)(201708071742011)(7699050);SRVR:DM6PR08MB4940;BCL:0;PCL:0;RULEID:;SRVR:DM6PR08MB4940;
X-Forefront-PRVS: 07880C4932
X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(6069001)(7916004)(136003)(396003)(376002)(366004)(346002)(39840400004)(199004)(189003)(76506005)(476003)(106356001)(105586002)(11346002)(33716001)(446003)(956004)(5660300001)(26005)(33896004)(16526019)(386003)(486006)(44832011)(76176011)(478600001)(14444005)(3846002)(1076002)(6116002)(966005)(50466002)(47776003)(42882007)(6246003)(93886005)(9686003)(229853002)(8936002)(7736002)(4326008)(54906003)(305945005)(6862004)(68736007)(53936002)(2906002)(316002)(97736004)(6496006)(81156014)(16586007)(66066001)(8676002)(6306002)(52116002)(25786009)(23726003)(6486002)(58126008)(81166006);DIR:OUT;SFP:1102;SCL:1;SRVR:DM6PR08MB4940;H:localhost;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1;
Received-SPF: None (protection.outlook.com: wavecomp.com does not designate
 permitted sender hosts)
X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;DM6PR08MB4940;23:ecAUOOsDOfG9DOLs7BMz022V/aNs7uL5L+n+0K8Gp?=
 =?us-ascii?Q?Xt/SPqJHvcx+PjaZxpPbdKYz1EwJpDcI9s4zEjrdFLy6yenYPqML49W6LFMx?=
 =?us-ascii?Q?Ug6+pQ0G5lPvVNLLlT57O9gKpyTZIbfg4Zy7x9HOCXHgRwsWGXfP2Ql2tKae?=
 =?us-ascii?Q?+CZS/hTUXXz+mUODDWEjDuN33R1nIcy1jtUTY3V4KV2ivrmhE4lHqEyPcIlo?=
 =?us-ascii?Q?Gs1GWM74SkN1jdKbZijGDRtpwtRzmP5LPv0gijZdAuI7twzjUS3b8YQWscWg?=
 =?us-ascii?Q?7mVciLnAwAdv8QvXUvTW3z+tikL56nYK9KNYfuVNLtKfJngMTKVmODFltFQc?=
 =?us-ascii?Q?zC7nwRCk0zF58OWI7V2JQEEQGZL76YtfiZG9bMGJDOlBUk+C93RGWaF6DeW/?=
 =?us-ascii?Q?f66d7ZijoWArp90zv7bEF6Stw9mGR/JCKHsRyfEAQo76oJ6/s7GsRnTdMuOs?=
 =?us-ascii?Q?ozRBSI9yfKggnibCYMYAGVeMO42mdXLRMZs5rgpkjAw1clmLa+hdFrjn44Ti?=
 =?us-ascii?Q?ndoHvdSVtThWbP7plM03akF5kXcSnflEaEnQx7dx3QuvTC3XiWm9J8aA/lNT?=
 =?us-ascii?Q?o+3ArQJ+SloGoFubvlarKVVDBYDPc/Csj/sD7zP6HRV+sgm0sgpBxjubUpfp?=
 =?us-ascii?Q?TSFCgvUiVZX2CC8/d8uE3QbHv3zUaod6zk/xvotX7et/WOYbH1EvJcljkxqL?=
 =?us-ascii?Q?wCpez+2QdNw4xYb3nGpCwQhexcFhcqJVCHAUGBbh9/y+cOuak5nEmsnfYBEh?=
 =?us-ascii?Q?OaYFFS0DqFAn4z2zHzcZN8JLndIZEZ6WVFgnAsm7DbCTaCzB+D2jdwLiZRIi?=
 =?us-ascii?Q?OKZP+796ULwvVcDqozgeHmfCYL/2Xh0XAcap6L2PRngmap2vd498Z5yQMuto?=
 =?us-ascii?Q?VcwWlLiqbadDT8lUadPYnFYY7XXQcBtG99YuX8EUIMz3fxqZslm8O14AJ3uQ?=
 =?us-ascii?Q?2RfBWGLiJjpQtvq1Vm+jW/mqvcIBlHUxc+R1skrPncJevFOUYTltP4nSiGMQ?=
 =?us-ascii?Q?4PcLZk9sCfa6GHxaAszLhYFuDOHzltxyqyEs0qTXk6I2S70h+jtH0SUYcgH5?=
 =?us-ascii?Q?LMNfWKLBROw/d72YZNxiaecCNJsbzkQUOVzS0/bdKTkVtYJRDqxDYEfMnvQ2?=
 =?us-ascii?Q?mKlNhOsuFjam3BZ89f+1yNhRsdqrM1K4aT+D+mgHEG6otlUVfzuCAALgkfdK?=
 =?us-ascii?Q?RbD6qOHisTFcdKsyoGPyOjxvYmd6tSXWwn3mp4yyfGbDp9BteR9xYqAE6Q3a?=
 =?us-ascii?Q?CQJeNezLGXl8m+tCKP35ZybdpmdFfoxsVf8Id4U05bS3JIq2Va+jqAo6jsNf?=
 =?us-ascii?Q?cWpxYfg2LVfzlOCmct0+/sv0/NJKUou/H7RM/TUXk2Bp2NN8hLxwNLosEL6I?=
 =?us-ascii?Q?Ck91g=3D=3D?=
X-Microsoft-Antispam-Message-Info: zuVj3KfFLn5y2754iJKB85zgikNpbXcrbPBj12l13pTFRw1qfrwwgEbqVP1qXMDoGeFLKvg/u32EjoiWbJR0nGo28a2mPcfY/gtC+C3OaBp6GM5T/xWaLHwIERBoPQhJXv5dZmW0szHmvrtGUbPuNdEqcTFJuXb4PA+gxOldBQALpJ8+oZ7PS29+017Hrtm8N47MpiMPxfKz00jVmeQQrEY/NPbJ6ufSa2nozKGWDrb15AmppXc0lwoRE8q7nMIG+E6o4e6WYnAtur8/1otB0lmxYBfCTDEcuJhekuGpz93QW/L810ZFN08Ai/M0XNiFkDyaxJAvFXsLKYeF+M9YAZykIUAKtggBlxBsDHferpY=
X-Microsoft-Exchange-Diagnostics: 1;DM6PR08MB4940;6:51T67EKHqwEkBB4CkmwaoJKWlZVOgW3to/QzH26BWVDz+bzNAK+ZoU7B/lVa7AIDYgNtR6WMmVTpQoaPP0reRVhBrI2+2rAz+sPaans/VyxwqtKoroqrYhuxkQ0/kXwOCdxyo+PrFSis/Ldei8+eFKpij0/a5ugXRoPUnAlzjSHsTcG23hvjNpUY3C24GJqKY2b9oi8qCHCEs0AabLBvg+GRGZMBthgGP2MUBnyYJWgLqu7qfOrSqxX897ZaEbP7BHb31cFStFjB+K9gC23fBe+Veq7oAOWujC9ev5IOto2TtAZeCZDw61g9grl5HCTYx5WIO/fgenk49ZPZjkj9Kpuiwl+hzCxQzM/WAzJt8VAaNb6fuQse1uINBo7gF7wz49NWHRk1NxH7f6T232GXxUXZ6FLnBqSy3PwOHP3zg+lrwHmuoJSNR0TC4LN9J3kMGNhm/+DBo4Jx5EOailkTHA==;5:Wli04Zphm9rm7LFcG35vTU2Ny+Pe7jvOtaDPt5+is02HCobdnsmzXksItIKaln0tkWIo/RJNejjaaL0JxS2Xg6/B0P6VDTd9EbOFq7i9D8/ZEZWfyHFwm7ijchGZzJN/vPYzgg/6qDSf0P7H3JZCOE2O36FDLuM2zRX2yBSSMdY=;7:r1auQhJ/s0MYuWQlbDhEK8IYu96nqMjBFZQ8tP0kt52klKdn4KrFHfc6lBHUisozXVV38/lcZnYfewXX1zerMYCrtS6IF6Sb1ZRjSSj27kNAytlClAEfkCOWUEWM59utd8nZi3RE/Tu24q+gom73otaaLPM+MTdEugGnCYe4T1dWtq7IyG67eekz+8sg6ovcdTHlE8GSb9Csbz9M9Ii6IZ2FipAO6DUGTm7+zadhv1LKhdphzmGxOtQ+ZeVYuCUb
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-OriginatorOrg: mips.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Sep 2018 23:11:56.5102 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 7dec7c0c-21da-418b-3f4c-08d615174e94
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 463607d3-1db3-40a0-8a29-970c56230104
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR08MB4940
Return-Path: <pburton@wavecomp.com>
X-Envelope-To: <"|/home/ecartis/ecartis -s linux-mips"> (uid 0)
X-Orcpt: rfc822;linux-mips@linux-mips.org
Original-Recipient: rfc822;linux-mips@linux-mips.org
X-archive-position: 66154
X-ecartis-version: Ecartis v1.0.0
Sender: linux-mips-bounce@linux-mips.org
Errors-to: linux-mips-bounce@linux-mips.org
X-original-sender: paul.burton@mips.com
Precedence: bulk
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
List-software: Ecartis version 1.0.0
List-Id: linux-mips <linux-mips.eddie.linux-mips.org>
X-List-ID: linux-mips <linux-mips.eddie.linux-mips.org>
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
X-list: linux-mips
Content-Length: 1837
Lines: 44

On Fri, Sep 07, 2018 at 03:21:10PM -0700, Dengcheng Zhu wrote:
> Hi Paul,
> 
> 
> > The problem is that the "only CPU0 will use reboot_code_buffer" part
> isn't true because CPU0 might not be the one that writes to it.
> 
> [Dengcheng]: I didn't say CPU0 is always the one running machine_kexec().
> Actually, in my testing, I intentionally did something like:
> 
> taskset -c 3 echo c > /proc/sysrq-trigger

Well, if CPU 0 isn't running machine_kexec() then some other CPU is
doing the write to reboot_code_buffer and therefore CPU 0 isn't the only
one accessing/using it.

As I walked through in an earlier email, the safe way for this to happen
is for the machine_kexec() CPU to write back its dcache as far as needed
for a remote CPU's icache to observe the stores. Having CPU 0 be the
only one to do any cache maintenance will not achieve that.

As I mentioned before, if you're testing on I6x00 then you probably got
away with it because the icache fills from the dcache. You'd probably
also get away with having CPU 0 be the only one to do cache maintenance
since the CM globalizes hit cache ops. But not all MIPS CPUs would be so
lucky.

> And for "either halted or powered down", do you have any idea/plan for
> Octeon, Loognson and bmips?

Well like I said before if we add a callback to struct plat_smp_ops to
stop other CPUs, at least in the crash case, then we can check that &
refuse to kexec() on systems that haven't implemented it.

Octeon right now has kexec, so any changes like this should bring Octeon
along for the ride by implementing the new callback there. Loongson has
a patch pending [1] so that should probably be taken into account too.
bmips would just be left alone, and wouldn't support kexec until someone
implements the new callback.

[1] https://patchwork.linux-mips.org/patch/20432/

Thanks,
    Paul

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

Hi Paul,


> The problem is that the "only CPU0 will use reboot_code_buffer" part
isn't true because CPU0 might not be the one that writes to it.

[Dengcheng]: I didn't say CPU0 is always the one running machine_kexec().
Actually, in my testing, I intentionally did something like:

taskset -c 3 echo c > /proc/sysrq-trigger

And for "either halted or powered down", do you have any idea/plan for
Octeon, Loognson and bmips?


Thanks,

Dengcheng


From: Paul Burton
Sent: Friday, September 7, 2018 2:42 PM
To: Dengcheng Zhu
Cc: Paul Burton; ralf@linux-mips.org; linux-mips@linux-mips.org; rachel.mozes@intel.com
Subject: Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues
  

Hi Dengcheng,

On Fri, Sep 07, 2018 at 12:47:45PM -0700, Dengcheng Zhu wrote:
> On cache issue, what if kexec_this_cpu() does local i$ flush and scache
> invalidation? This is done only on CPU0, no IPI needed. And only CUP0 will use
> reboot_code_buffer. In this way, marking other CPUs offline to prevent being
> sent IPI is not needed.

The problem is that the "only CPU0 will use reboot_code_buffer" part
isn't true because CPU0 might not be the one that writes to it. Though
perhaps if we moved the memcpy() from machine_kexec() to
kexec_this_cpu() that could work.

It still doesn't seem ideal to me that we'd have to even consider IPIs &
the fact that we might silently hang if we accidentally send one. I
really think it would be cleaner to mark CPUs offline & the problem is
solved naturally - we shouldn't IPI these CPUs, and marking them offline
means that we won't. Problem solved, and consistently with other
architectures.

> As to "the associated fragility of needing to avoid anything that might IPI
> them", I hope that's not a problem as the kexec sequence is determined: The last
> IPI received is to ask them to stand by on reboot signal, because other than
> kexec there is no other kernel code beyond our control, which might send IPIs.

But it does mean we can't call certain things, like __flush_cache_all(),
which means that developers need to 1) know that and 2) remember to
avoid any functions like this that perform an IPI behind the scenes. It
just feels like loading a gun & leaving it pointed at our feet - by
itself it's not necessarily going to cause a problem, but it would be oh
so easy to trigger problems later.

> > ...but the new kernel will have no knowledge of whether the old kernel
> had CPUs marked online or offline, so I don't follow the argument?
> 
> [Dengcheng] I meant the new kernel shouldn't need CPU online info from the old
> kernel. The new one is doing a "fresh" boot, provided CPUs 1+ have been halted.
> 
> Regarding the use of play_dead(), it's simple and generic - MIPS CPUs have
> implemented their own play_dead(), which we can use to stop execution. Other
> options may still come down to play_dead() to handle MT/MC details. The concern
> of running it in parallel is really about the online status, which has been
> explained above and in my last email.

But play_dead() really doesn't mean what you're suggesting it means.

What we want for kexec is that the CPU is stopped entirely - either
halted or powered down. Failing that it needs to be running a loop from
somewhere that we know isn't going to be overwritten, and the code it's
running needs to be able to hand over to the new kernel later (eg. the
existing kexec_smp_wait loop implements this latter approach).

Whilst the smp-cps.c implementation of play_dead() will currently halt
or power down the CPU, that's not universally true & doesn't necessarily
even need to remain true for smp-cps.c in the future (eg. we could
decide the halt/power down step would be cleaner to do in cps_cpu_die).

A quick look at the other play_dead() implementations we have shows that
both the cavium-octeon & loongson3 versions leave the CPU running a
loop (within the current/old kernel), whilst the bmips version just sits
at a wait instruction before jumping back to the fixed location of
bmips_secondary_reentry (in the current/old kernel) once interrupted.
None of those are suitable for kexec.

In general the CPU hotplug code just needs that when play_dead() runs on
the CPU going offline, and the struct plat_smp_ops cpu_die() callback
runs on some other CPU, the CPU being offlined will be quiesced to a
degree that it won't interfere with Linux. This is different to kexec
where we need the CPU to stop in such a way that it's fine that we might
overwrite the old kernel, and then be able to start it running the new
kernel later. As described before, that means it either needs to halt or
power down until the new kernel takes control of it or it needs to run
some loop from some location that we know won't be overwritten.

play_dead() is simple & already exists, it just doesn't do quite what we
need kexec to do outside of the one system you're looking at.

> A 'nicer' implementation stopping non-boot CPUs from one CPU might be available,
> but handling code duplication of play_dead() and saving CPU states could be a
> problem.

Code that can be shared between play_dead() & kexec can still be shared
if we have a different callback - the particular implementation of the
two would just have both call a common function for the part that's
common.

> For now, no __flush_cache_all() in machine_kexec() + no marking offline in
> kexec_this_cpu() + scache invalidation in kexec_this_cpu() might be a way to go.

For the system you're interested in, sure that works. But as I've
pointed out in my last few emails it does not work in general for other
systems that we also need to support in the kernel.

Thanks,
    Paul
    

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Paul Burton]

On Fri, Sep 07, 2018 at 03:21:10PM -0700, Dengcheng Zhu wrote:
> Hi Paul,
> 
> 
> > The problem is that the "only CPU0 will use reboot_code_buffer" part
> isn't true because CPU0 might not be the one that writes to it.
> 
> [Dengcheng]: I didn't say CPU0 is always the one running machine_kexec().
> Actually, in my testing, I intentionally did something like:
> 
> taskset -c 3 echo c > /proc/sysrq-trigger

Well, if CPU 0 isn't running machine_kexec() then some other CPU is
doing the write to reboot_code_buffer and therefore CPU 0 isn't the only
one accessing/using it.

As I walked through in an earlier email, the safe way for this to happen
is for the machine_kexec() CPU to write back its dcache as far as needed
for a remote CPU's icache to observe the stores. Having CPU 0 be the
only one to do any cache maintenance will not achieve that.

As I mentioned before, if you're testing on I6x00 then you probably got
away with it because the icache fills from the dcache. You'd probably
also get away with having CPU 0 be the only one to do cache maintenance
since the CM globalizes hit cache ops. But not all MIPS CPUs would be so
lucky.

> And for "either halted or powered down", do you have any idea/plan for
> Octeon, Loognson and bmips?

Well like I said before if we add a callback to struct plat_smp_ops to
stop other CPUs, at least in the crash case, then we can check that &
refuse to kexec() on systems that haven't implemented it.

Octeon right now has kexec, so any changes like this should bring Octeon
along for the ride by implementing the new callback there. Loongson has
a patch pending [1] so that should probably be taken into account too.
bmips would just be left alone, and wouldn't support kexec until someone
implements the new callback.

[1] https://patchwork.linux-mips.org/patch/20432/

Thanks,
    Paul

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

> Well, if CPU 0 isn't running machine_kexec() then some other CPU is
doing the write to reboot_code_buffer and therefore CPU 0 isn't the only
one accessing/using it.

[Dengcheng]: I meant only CPU0 will jump to reboot_code_buffer. This buffer is
surely possibly written by another CPU. Any issue with the machine_kexec() CPU
flush "local" dache, and CPU0 invalidate "local" icache and scahe before jumping?


Thanks,

Dengcheng


From: Paul Burton
Sent: Friday, September 7, 2018 4:11 PM
To: Dengcheng Zhu
Cc: Paul Burton; ralf@linux-mips.org; linux-mips@linux-mips.org; rachel.mozes@intel.com
Subject: Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues
  

On Fri, Sep 07, 2018 at 03:21:10PM -0700, Dengcheng Zhu wrote:
> Hi Paul,
> 
> 
> > The problem is that the "only CPU0 will use reboot_code_buffer" part
> isn't true because CPU0 might not be the one that writes to it.
> 
> [Dengcheng]: I didn't say CPU0 is always the one running machine_kexec().
> Actually, in my testing, I intentionally did something like:
> 
> taskset -c 3 echo c > /proc/sysrq-trigger

Well, if CPU 0 isn't running machine_kexec() then some other CPU is
doing the write to reboot_code_buffer and therefore CPU 0 isn't the only
one accessing/using it.

As I walked through in an earlier email, the safe way for this to happen
is for the machine_kexec() CPU to write back its dcache as far as needed
for a remote CPU's icache to observe the stores. Having CPU 0 be the
only one to do any cache maintenance will not achieve that.

As I mentioned before, if you're testing on I6x00 then you probably got
away with it because the icache fills from the dcache. You'd probably
also get away with having CPU 0 be the only one to do cache maintenance
since the CM globalizes hit cache ops. But not all MIPS CPUs would be so
lucky.

> And for "either halted or powered down", do you have any idea/plan for
> Octeon, Loognson and bmips?

Well like I said before if we add a callback to struct plat_smp_ops to
stop other CPUs, at least in the crash case, then we can check that &
refuse to kexec() on systems that haven't implemented it.

Octeon right now has kexec, so any changes like this should bring Octeon
along for the ride by implementing the new callback there. Loongson has
a patch pending [1] so that should probably be taken into account too.
bmips would just be left alone, and wouldn't support kexec until someone
implements the new callback.

[1] https://patchwork.linux-mips.org/patch/20432/

Thanks,
    Paul
    
From alexandre.belloni@bootlin.com Sat Sep  8 10:17:39 2018
Received: with ECARTIS (v1.0.0; list linux-mips); Sat, 08 Sep 2018 10:17:42 +0200 (CEST)
Received: from mail.bootlin.com ([62.4.15.54]:48469 "EHLO mail.bootlin.com"
        rhost-flags-OK-OK-OK-OK) by eddie.linux-mips.org with ESMTP
        id S23994059AbeIHIRjBHzYG (ORCPT <rfc822;linux-mips@linux-mips.org>);
        Sat, 8 Sep 2018 10:17:39 +0200
Received: by mail.bootlin.com (Postfix, from userid 110)
        id EBADC207B0; Sat,  8 Sep 2018 10:17:33 +0200 (CEST)
Received: from localhost (unknown [88.191.26.124])
        by mail.bootlin.com (Postfix) with ESMTPSA id BF3B6203DC;
        Sat,  8 Sep 2018 10:17:23 +0200 (CEST)
Date:   Sat, 8 Sep 2018 10:17:23 +0200
From:   Alexandre Belloni <alexandre.belloni@bootlin.com>
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     linux-rtc@vger.kernel.org, a.zummo@towertech.it,
        keguang.zhang@gmail.com, y2038@lists.linaro.org,
        Ralf Baechle <ralf@linux-mips.org>,
        Paul Burton <paul.burton@mips.com>,
        James Hogan <jhogan@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Matt Redfearn <matt.redfearn@mips.com>,
        Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>,
        linux-mips@linux-mips.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] rtc: mips: default to rtc-cmos on mips
Message-ID: <20180908081723.GB2598@piout.net>
References: <20180828142724.4067857-1-arnd@arndb.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180828142724.4067857-1-arnd@arndb.de>
User-Agent: Mutt/1.10.1 (2018-07-13)
Return-Path: <alexandre.belloni@bootlin.com>
X-Envelope-To: <"|/home/ecartis/ecartis -s linux-mips"> (uid 0)
X-Orcpt: rfc822;linux-mips@linux-mips.org
Original-Recipient: rfc822;linux-mips@linux-mips.org
X-archive-position: 66156
X-ecartis-version: Ecartis v1.0.0
Sender: linux-mips-bounce@linux-mips.org
Errors-to: linux-mips-bounce@linux-mips.org
X-original-sender: alexandre.belloni@bootlin.com
Precedence: bulk
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
List-software: Ecartis version 1.0.0
List-Id: linux-mips <linux-mips.eddie.linux-mips.org>
X-List-ID: linux-mips <linux-mips.eddie.linux-mips.org>
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
X-list: linux-mips
Content-Length: 587
Lines: 18

On 28/08/2018 16:26:30+0200, Arnd Bergmann wrote:
> The old rtc driver is getting in the way of some compat_ioctl
> simplification. Looking up the loongson64 git history, it seems
> that everyone uses the more modern but compatible RTC_CMOS driver
> anyway, so let's remove the special case for loongson64.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/mips/Kconfig    | 2 +-
>  drivers/char/Kconfig | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues

[Dengcheng Zhu]

> Well, if CPU 0 isn't running machine_kexec() then some other CPU is
doing the write to reboot_code_buffer and therefore CPU 0 isn't the only
one accessing/using it.

[Dengcheng]: I meant only CPU0 will jump to reboot_code_buffer. This buffer is
surely possibly written by another CPU. Any issue with the machine_kexec() CPU
flush "local" dache, and CPU0 invalidate "local" icache and scahe before jumping?


Thanks,

Dengcheng


From: Paul Burton
Sent: Friday, September 7, 2018 4:11 PM
To: Dengcheng Zhu
Cc: Paul Burton; ralf@linux-mips.org; linux-mips@linux-mips.org; rachel.mozes@intel.com
Subject: Re: [PATCH v4 0/6] MIPS: kexec/kdump: Fix smp reboot and other issues
  

On Fri, Sep 07, 2018 at 03:21:10PM -0700, Dengcheng Zhu wrote:
> Hi Paul,
> 
> 
> > The problem is that the "only CPU0 will use reboot_code_buffer" part
> isn't true because CPU0 might not be the one that writes to it.
> 
> [Dengcheng]: I didn't say CPU0 is always the one running machine_kexec().
> Actually, in my testing, I intentionally did something like:
> 
> taskset -c 3 echo c > /proc/sysrq-trigger

Well, if CPU 0 isn't running machine_kexec() then some other CPU is
doing the write to reboot_code_buffer and therefore CPU 0 isn't the only
one accessing/using it.

As I walked through in an earlier email, the safe way for this to happen
is for the machine_kexec() CPU to write back its dcache as far as needed
for a remote CPU's icache to observe the stores. Having CPU 0 be the
only one to do any cache maintenance will not achieve that.

As I mentioned before, if you're testing on I6x00 then you probably got
away with it because the icache fills from the dcache. You'd probably
also get away with having CPU 0 be the only one to do cache maintenance
since the CM globalizes hit cache ops. But not all MIPS CPUs would be so
lucky.

> And for "either halted or powered down", do you have any idea/plan for
> Octeon, Loognson and bmips?

Well like I said before if we add a callback to struct plat_smp_ops to
stop other CPUs, at least in the crash case, then we can check that &
refuse to kexec() on systems that haven't implemented it.

Octeon right now has kexec, so any changes like this should bring Octeon
along for the ride by implementing the new callback there. Loongson has
a patch pending [1] so that should probably be taken into account too.
bmips would just be left alone, and wouldn't support kexec until someone
implements the new callback.

[1] https://patchwork.linux-mips.org/patch/20432/

Thanks,
    Paul