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

[PATCH v3 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:

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 platform as default
    option

 arch/mips/cavium-octeon/setup.c       |  2 +-
 arch/mips/cavium-octeon/smp.c         | 36 +++++++++------
 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      | 78 ++++++++++++++++++++++++++-----
 arch/mips/kernel/process.c            |  2 +-
 arch/mips/kernel/relocate_kernel.S    | 39 ----------------
 arch/mips/kernel/smp-bmips.c          | 11 +++--
 arch/mips/kernel/smp-cps.c            | 60 ++++++++++++++----------
 arch/mips/loongson64/loongson-3/smp.c | 86 +++++++++++++++++++----------------
 13 files changed, 191 insertions(+), 187 deletions(-)
 delete mode 100644 arch/mips/generic/kexec.c

-- 
2.7.4

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

[Dengcheng Zhu]

Extract play_dead() from CONFIG_HOTPLUG_CPU and share with CONFIG_KEXEC.
Also, add one parameter to it to avoid doing unnecessary things in the case
of 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:

* Code style adjustments.

 arch/mips/cavium-octeon/smp.c         | 36 +++++++++------
 arch/mips/include/asm/smp.h           |  4 +-
 arch/mips/kernel/process.c            |  2 +-
 arch/mips/kernel/smp-bmips.c          | 11 +++--
 arch/mips/kernel/smp-cps.c            | 60 ++++++++++++++----------
 arch/mips/loongson64/loongson-3/smp.c | 86 +++++++++++++++++++----------------
 6 files changed, 115 insertions(+), 84 deletions(-)

diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index 75e7c86..6aaf4de 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -280,11 +280,31 @@ 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(bool kexec)
+{
+	int cpu = cpu_number_map(cvmx_get_core_num());
+
+	if (!kexec)
+		idle_task_exit();
+	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 +363,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 88ebd83..c8035e5 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(bool kexec);
 #endif
 
 /*
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 8d85046..1aa24bb 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -53,7 +53,7 @@
 #ifdef CONFIG_HOTPLUG_CPU
 void arch_cpu_idle_dead(void)
 {
-	play_dead();
+	play_dead(false);
 }
 #endif
 
diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c
index 159e83a..e1c11bd 100644
--- a/arch/mips/kernel/smp-bmips.c
+++ b/arch/mips/kernel/smp-bmips.c
@@ -381,9 +381,14 @@ static void bmips_cpu_die(unsigned int cpu)
 {
 }
 
-void __ref play_dead(void)
+#endif /* CONFIG_HOTPLUG_CPU */
+
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_KEXEC)
+
+void __ref play_dead(bool kexec)
 {
-	idle_task_exit();
+	if (!kexec)
+		idle_task_exit();
 
 	/* flush data cache */
 	_dma_cache_wback_inv(0, ~0);
@@ -409,7 +414,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 03f1026..aeade5b 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 {
@@ -426,12 +406,18 @@ static enum {
 	CPU_DEATH_POWER,
 } cpu_death;
 
-void play_dead(void)
+void play_dead(bool kexec)
 {
 	unsigned int cpu, core, vpe_id;
 
 	local_irq_disable();
-	idle_task_exit();
+	/*
+	 * Don't bother dealing with idle task's mm as we are executing the
+	 * new kernel.
+	 */
+	if (!kexec)
+		idle_task_exit();
+
 	cpu = smp_processor_id();
 	core = cpu_core(&cpu_data[cpu]);
 	cpu_death = CPU_DEATH_POWER;
@@ -454,7 +440,8 @@ void play_dead(void)
 	}
 
 	/* This CPU has chosen its way out */
-	(void)cpu_report_death();
+	if (!kexec)
+		(void)cpu_report_death();
 
 	if (cpu_death == CPU_DEATH_HALT) {
 		vpe_id = cpu_vpe_id(&cpu_data[cpu]);
@@ -480,6 +467,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 8501109..5fcf0f8 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
@@ -668,13 +710,14 @@ static void loongson3b_play_dead(int *state_addr)
 		: "a1");
 }
 
-void play_dead(void)
+void play_dead(bool kexec)
 {
 	int *state_addr;
 	unsigned int cpu = smp_processor_id();
 	void (*play_dead_at_ckseg1)(int *);
 
-	idle_task_exit();
+	if (!kexec)
+		idle_task_exit();
 	switch (read_c0_prid() & PRID_REV_MASK) {
 	case PRID_REV_LOONGSON3A_R1:
 	default:
@@ -697,44 +740,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.7.4

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

[Paul Burton]

Hi Dengcheng,

On Mon, Jul 23, 2018 at 07:48:14AM -0700, Dengcheng Zhu wrote:
> Extract play_dead() from CONFIG_HOTPLUG_CPU and share with CONFIG_KEXEC.
> Also, add one parameter to it to avoid doing unnecessary things in the case
> of kexec.

I'd prefer that we use a separate function to play_dead() for this, for
example we could provide an implementation of crash_smp_send_stop() much
like ARM's which invokes a machine_crash_nonpanic_core() function on all
CPUs other than the crash CPU.

This would prevent the kexec/kdump functionality from depending on the
board/platform specific play_dead(), and wouldn't need these changes to
all of the implementations of play_dead().

We should also be calling crash_save_cpu() on each CPU, which is a
further difference from play_dead().

> 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,

Do you know how that happens? I'd expect detecting topology not to
depend upon what state CPUs are in. That should certainly be the case
for smp-cps/CONFIG_MIPS_CPS which detects topology just by reading
CM/CPC/GIC registers.

> sluggish performance in MT environment during and after reboot,

The function running on non-crash CPUs would just need to execute a loop
of wait instructions to avoid this.

> new SMP kernel not able to bring up secondary CPU etc.

If the SMP implementation can reset CPUs then that ought not to happen,
since no matter what the CPU was doing Linux should be able to cause it
to reset & run some known piece of code. I'm not sure the current Octeon
SMP code can do that, but there are patches in patchwork that look like
they might (& patches to remove Octeon's current kexec/kdump code which
suggests nobody cares much about it).

I'd suggest we could perhaps add a boolean to struct plat_smp_ops to
indicate whether kexec is supported, and start by setting it to true for
cps_smp_ops. Then we can have machine_kexec_prepare() return an error if
it finds !mp_ops->kexec_supported, and deal with enabling kexec per
platform. I think this would be better than Kconfig because there are
systems where we may use one of multiple SMP implementations - for
example Malta might use smp-cps (which would be OK for kexec) or smp-cmp
(which wouldn't). If we get to a point where all our SMP implementations
can deal with kexec we could remove the field later.

Thanks,
    Paul

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

[Dengcheng Zhu]

Hi Paul,


Thanks for reviewing. Please see my comments below.


On 07/24/2018 04:23 PM, Paul Burton wrote:
> Hi Dengcheng,
>
> On Mon, Jul 23, 2018 at 07:48:14AM -0700, Dengcheng Zhu wrote:
>> Extract play_dead() from CONFIG_HOTPLUG_CPU and share with CONFIG_KEXEC.
>> Also, add one parameter to it to avoid doing unnecessary things in the case
>> of kexec.
> I'd prefer that we use a separate function to play_dead() for this, for
> example we could provide an implementation of crash_smp_send_stop() much
> like ARM's which invokes a machine_crash_nonpanic_core() function on all
> CPUs other than the crash CPU.

ARM's crash_smp_send_stop() that calls machine_crash_nonpanic_core() is called
in machine_crash_shutdown(), not in machine_kexec(). It's similar in the MIPS
case - default_machine_crash_shutdown().

>
> This would prevent the kexec/kdump functionality from depending on the
> board/platform specific play_dead(), and wouldn't need these changes to
> all of the implementations of play_dead().

The revised play_dead() is JUST to make sure we are turning off CPUs cleanly.
This function itself already hides board/platform details. So it seems a good
candidate for turning off CPUs for the target platform.

This function is called only in the newly created kexec_smp_reboot(), before
which cpu states have been saved.

>
> We should also be calling crash_save_cpu() on each CPU, which is a
> further difference from play_dead().

crash_save_cpu() is already called in machine_crash_shutdown(), which is
prior to machine_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,
> Do you know how that happens? I'd expect detecting topology not to
> depend upon what state CPUs are in. That should certainly be the case
> for smp-cps/CONFIG_MIPS_CPS which detects topology just by reading
> CM/CPC/GIC registers.

I didn't debug the topology issue. But it's related to the attempt of *all*
CPUs using the same code from kexec_start_address, thinking they are
dominating the system.

The cleanest way IMO is simple and what this patch is trying to do - turn off
CPUs and do a fresh SMP boot from c0v0.


>
>> sluggish performance in MT environment during and after reboot,
> The function running on non-crash CPUs would just need to execute a loop
> of wait instructions to avoid this.

Same as the topology problem, I didn't look into this performance issue. But
I did try using 'wait' on non-crash CPUs - it didn't work well. In the clean
way, this problem disappears as expected.

>
>> new SMP kernel not able to bring up secondary CPU etc.
> If the SMP implementation can reset CPUs then that ought not to happen,
> since no matter what the CPU was doing Linux should be able to cause it
> to reset & run some known piece of code. I'm not sure the current Octeon
> SMP code can do that, but there are patches in patchwork that look like
> they might (& patches to remove Octeon's current kexec/kdump code which
> suggests nobody cares much about it).
>
> I'd suggest we could perhaps add a boolean to struct plat_smp_ops to
> indicate whether kexec is supported, and start by setting it to true for
> cps_smp_ops. Then we can have machine_kexec_prepare() return an error if
> it finds !mp_ops->kexec_supported, and deal with enabling kexec per
> platform. I think this would be better than Kconfig because there are
> systems where we may use one of multiple SMP implementations - for
> example Malta might use smp-cps (which would be OK for kexec) or smp-cmp
> (which wouldn't). If we get to a point where all our SMP implementations
> can deal with kexec we could remove the field later.

This problem is also related to the original kexec boot mechanism. No
matter what SMP implementation it is, it should be good if we correctly turn
off CPUs and reboot the (SMP) system from the 1st CPU in its own
implementation.


Regards,

Dengcheng

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

*From:* Paul Burton <mailto:paul.burton@mips.com>
*Sent:* Tuesday, July 24, 2018 4:23PM
*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 v3 1/6] MIPS: Make play_dead() work for kexec

Hi Dengcheng,

On Mon, Jul 23, 2018 at 07:48:14AM -0700, Dengcheng Zhu wrote:

> Extract play_dead() from CONFIG_HOTPLUG_CPU and share with CONFIG_KEXEC.
> Also, add one parameter to it to avoid doing unnecessary things in the case
> of kexec.

I'd prefer that we use a separate function to play_dead() for this, for
example we could provide an implementation of crash_smp_send_stop() much
like ARM's which invokes a machine_crash_nonpanic_core() function on all
CPUs other than the crash CPU.

This would prevent the kexec/kdump functionality from depending on the
board/platform specific play_dead(), and wouldn't need these changes to
all of the implementations of play_dead().

We should also be calling crash_save_cpu() on each CPU, which is a
further difference from play_dead().

> 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,

Do you know how that happens? I'd expect detecting topology not to
depend upon what state CPUs are in. That should certainly be the case
for smp-cps/CONFIG_MIPS_CPS which detects topology just by reading
CM/CPC/GIC registers.

> sluggish performance in MT environment during and after reboot,

The function running on non-crash CPUs would just need to execute a loop
of wait instructions to avoid this.

> new SMP kernel not able to bring up secondary CPU etc.

If the SMP implementation can reset CPUs then that ought not to happen,
since no matter what the CPU was doing Linux should be able to cause it
to reset & run some known piece of code. I'm not sure the current Octeon
SMP code can do that, but there are patches in patchwork that look like
they might (& patches to remove Octeon's current kexec/kdump code which
suggests nobody cares much about it).

I'd suggest we could perhaps add a boolean to struct plat_smp_ops to
indicate whether kexec is supported, and start by setting it to true for
cps_smp_ops. Then we can have machine_kexec_prepare() return an error if
it finds !mp_ops->kexec_supported, and deal with enabling kexec per
platform. I think this would be better than Kconfig because there are
systems where we may use one of multiple SMP implementations - for
example Malta might use smp-cps (which would be OK for kexec) or smp-cmp
(which wouldn't). If we get to a point where all our SMP implementations
can deal with kexec we could remove the field later.

Thanks,
     Paul

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

[Paul Burton]

Hi Dengcheng,

On Mon, Jul 30, 2018 at 11:50:38AM -0700, Dengcheng Zhu wrote:
> On 07/24/2018 04:23 PM, Paul Burton wrote:
> > On Mon, Jul 23, 2018 at 07:48:14AM -0700, Dengcheng Zhu wrote:
> > > Extract play_dead() from CONFIG_HOTPLUG_CPU and share with CONFIG_KEXEC.
> > > Also, add one parameter to it to avoid doing unnecessary things in the case
> > > of kexec.
> > I'd prefer that we use a separate function to play_dead() for this, for
> > example we could provide an implementation of crash_smp_send_stop() much
> > like ARM's which invokes a machine_crash_nonpanic_core() function on all
> > CPUs other than the crash CPU.
>%
> > This would prevent the kexec/kdump functionality from depending on the
> > board/platform specific play_dead(), and wouldn't need these changes to
> > all of the implementations of play_dead().
> 
> The revised play_dead() is JUST to make sure we are turning off CPUs cleanly.
> This function itself already hides board/platform details. So it seems a good
> candidate for turning off CPUs for the target platform.
> 
> This function is called only in the newly created kexec_smp_reboot(), before
> which cpu states have been saved.

I can see the appeal, but please see my reply from just now (which is
accidentally in response to v2, but still applies to v3) about cleaning
up the changes to play_dead() a little. I think that would help it feel
"nicer" to me.

> > We should also be calling crash_save_cpu() on each CPU, which is a
> > further difference from play_dead().
> 
> crash_save_cpu() is already called in machine_crash_shutdown(), which is
> prior to machine_kexec().

But that only happens on one CPU, right? See the comment in
crash_kexec(). So aren't we missing the register state for all the other
CPUs?

Thanks,
    Paul

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

[Dengcheng Zhu]

Hi Paul,


On 08/30/2018 04:32 PM, Paul Burton wrote:
> Hi Dengcheng,
>
> On Mon, Jul 30, 2018 at 11:50:38AM -0700, Dengcheng Zhu wrote:
>> On 07/24/2018 04:23 PM, Paul Burton wrote:
>>> On Mon, Jul 23, 2018 at 07:48:14AM -0700, Dengcheng Zhu wrote:
>>>> Extract play_dead() from CONFIG_HOTPLUG_CPU and share with CONFIG_KEXEC.
>>>> Also, add one parameter to it to avoid doing unnecessary things in the case
>>>> of kexec.
>>> I'd prefer that we use a separate function to play_dead() for this, for
>>> example we could provide an implementation of crash_smp_send_stop() much
>>> like ARM's which invokes a machine_crash_nonpanic_core() function on all
>>> CPUs other than the crash CPU.
>> %
>>> This would prevent the kexec/kdump functionality from depending on the
>>> board/platform specific play_dead(), and wouldn't need these changes to
>>> all of the implementations of play_dead().
>> The revised play_dead() is JUST to make sure we are turning off CPUs cleanly.
>> This function itself already hides board/platform details. So it seems a good
>> candidate for turning off CPUs for the target platform.
>>
>> This function is called only in the newly created kexec_smp_reboot(), before
>> which cpu states have been saved.
> I can see the appeal, but please see my reply from just now (which is
> accidentally in response to v2, but still applies to v3) about cleaning
> up the changes to play_dead() a little. I think that would help it feel
> "nicer" to me.
>
>>> We should also be calling crash_save_cpu() on each CPU, which is a
>>> further difference from play_dead().
>> crash_save_cpu() is already called in machine_crash_shutdown(), which is
>> prior to machine_kexec().
> But that only happens on one CPU, right? See the comment in
> crash_kexec(). So aren't we missing the register state for all the other
> CPUs?

No, we are not missing the state for other CPUs. They do crash_save_cpu() in
crash_shutdown_secondary().


Thanks,

Dengcheng

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

*From:* Paul Burton <mailto:paul.burton@mips.com>
*Sent:* Thursday, August 30, 2018 4:32PM
*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 v3 1/6] MIPS: Make play_dead() work for kexec

Hi Dengcheng,

On Mon, Jul 30, 2018 at 11:50:38AM -0700, Dengcheng Zhu wrote:

> On 07/24/2018 04:23 PM, Paul Burton wrote:
>> On Mon, Jul 23, 2018 at 07:48:14AM -0700, Dengcheng Zhu wrote:
>>> Extract play_dead() from CONFIG_HOTPLUG_CPU and share with CONFIG_KEXEC.
>>> Also, add one parameter to it to avoid doing unnecessary things in the case
>>> of kexec.
>> I'd prefer that we use a separate function to play_dead() for this, for
>> example we could provide an implementation of crash_smp_send_stop() much
>> like ARM's which invokes a machine_crash_nonpanic_core() function on all
>> CPUs other than the crash CPU.
> %
>> This would prevent the kexec/kdump functionality from depending on the
>> board/platform specific play_dead(), and wouldn't need these changes to
>> all of the implementations of play_dead().
> The revised play_dead() is JUST to make sure we are turning off CPUs cleanly.
> This function itself already hides board/platform details. So it seems a good
> candidate for turning off CPUs for the target platform.
>
> This function is called only in the newly created kexec_smp_reboot(), before
> which cpu states have been saved.

I can see the appeal, but please see my reply from just now (which is
accidentally in response to v2, but still applies to v3) about cleaning
up the changes to play_dead() a little. I think that would help it feel
"nicer" to me.

>> We should also be calling crash_save_cpu() on each CPU, which is a
>> further difference from play_dead().
> crash_save_cpu() is already called in machine_crash_shutdown(), which is
> prior to machine_kexec().

But that only happens on one CPU, right? See the comment in
crash_kexec(). So aren't we missing the register state for all the other
CPUs?

Thanks,
     Paul

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

[Dengcheng Zhu]

Use the new play_dead() interface 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:

* Code style adjustments.

 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 a8034d0..3a1f7fa 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 493a3cc..a8e0bfc 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 d455363..6d43200 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 8b574bc..b3674f7 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(true);
+	} 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.7.4

[PATCH v3 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 a8e0bfc..33f31a8 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 b3674f7..1679408 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 c6bbf21..14e0eaf 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.7.4

[PATCH v3 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>
---
Changes:

* Code style adjustments.

 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 1679408..6e3f7c8 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.7.4

[PATCH v3 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 33f31a8..2583e6a 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.7.4

[PATCH v3 6/6] MIPS: kexec: Use prepare method from generic platform as default option

[Dengcheng Zhu]

The kexec_prepare method for the generic platform should be applicable to
other platforms. For those otherwise, like Octeon, they will use their own
_machine_kexec_prepare().

Without the default prepare work, platforms other than the generic one will
not be able to automatically set up command line correctly 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:

* Add LIBFDT to CPU_LOONGSON3 for default_machine_kexec_prepare().

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

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 08c10c5..df62764 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1374,6 +1374,7 @@ config CPU_LOONGSON3
 	select MIPS_L1_CACHE_SHIFT_6
 	select GPIOLIB
 	select SWIOTLB
+	select LIBFDT
 	help
 		The Loongson 3 processor implements the MIPS64R2 instruction
 		set with many extensions.
diff --git a/arch/mips/generic/Makefile b/arch/mips/generic/Makefile
index d03a36f..181aa13 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 1ca409f..0000000
--- 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 6e3f7c8..e3efb64 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>
@@ -65,6 +66,36 @@ static void kexec_image_info(const struct kimage *kimage)
 	}
 }
 
+static int default_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 *kimage)
 {
@@ -72,7 +103,8 @@ machine_kexec_prepare(struct kimage *kimage)
 
 	if (_machine_kexec_prepare)
 		return _machine_kexec_prepare(kimage);
-	return 0;
+	else
+		return default_machine_kexec_prepare(kimage);
 }
 
 void
-- 
2.7.4

Re: [PATCH v3 6/6] MIPS: kexec: Use prepare method from generic platform as default option

[Paul Burton]

Hi Dengcheng,

On Mon, Jul 23, 2018 at 07:48:19AM -0700, Dengcheng Zhu wrote:
> The kexec_prepare method for the generic platform should be applicable to
> other platforms.

...only for platforms which use the UHI boot protocol, which is what
generic_kexec_prepare() sets up kexec_args for. On any platforms which
don't use this boot protocol the new kernel wound find unexpected
arguments & in the worst case do something crazy with them.

> For those otherwise, like Octeon, they will use their own
> _machine_kexec_prepare().
> 
> Without the default prepare work, platforms other than the generic one will
> not be able to automatically set up command line correctly for the new
> kernel.

So even with this patch they still can't unless they happen to use the
UHI boot protocol.

If we ever have multiple in-tree platforms which make use of the UHI
boot protocol & want to use kexec then I'd be fine with us moving
generic_kexec_prepare() & renaming it something like uhi_kexec_prepare()
but I prefer that we don't just presume the boot protocol for any
platform that doesn't set _machine_kexec_prepare.

Hopefully anyone using the UHI boot protocol is doing so because they
want to fit in with the generic kernel platform so this won't happen
anyway (just need to get EVA supported by the generic platform & then
there'd be no reason at all not to use it).

Interestingly there's a patch in patchwork from last year which aimed at
making the default to pass arguments the current kernel was given to the
new kernel [1]. That would seem like a saner default since we'd be
making no presumptions about what those arguments should actually be,
but I don't think that's safe either because if any of those arguments
are pointers we have no guarantees that we haven't overwritten the
memory they're pointing at.

Thanks,
    Paul

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

Re: [PATCH v3 6/6] MIPS: kexec: Use prepare method from generic platform as default option

[Dengcheng Zhu]

Hi Paul,


On 07/24/2018 04:36 PM, Paul Burton wrote:
> Hi Dengcheng,
>
> On Mon, Jul 23, 2018 at 07:48:19AM -0700, Dengcheng Zhu wrote:
>> The kexec_prepare method for the generic platform should be applicable to
>> other platforms.
> ...only for platforms which use the UHI boot protocol, which is what
> generic_kexec_prepare() sets up kexec_args for. On any platforms which
> don't use this boot protocol the new kernel wound find unexpected
> arguments & in the worst case do something crazy with them.
>
>> For those otherwise, like Octeon, they will use their own
>> _machine_kexec_prepare().
>>
>> Without the default prepare work, platforms other than the generic one will
>> not be able to automatically set up command line correctly for the new
>> kernel.
> So even with this patch they still can't unless they happen to use the
> UHI boot protocol.
>
> If we ever have multiple in-tree platforms which make use of the UHI
> boot protocol & want to use kexec then I'd be fine with us moving
> generic_kexec_prepare() & renaming it something like uhi_kexec_prepare()
> but I prefer that we don't just presume the boot protocol for any
> platform that doesn't set _machine_kexec_prepare.

Thanks for pointing this out. How about something like the following in
kernel/machine_kexec.c?

--------------------------
#ifdef CONFIG_UHI_BOOT
static int uhi_machine_kexec_prepare(struct kimage *kimage)
{
     <the code of generic_kexec_prepare()>
}

int (*_machine_kexec_prepare)(struct kimage *) = uhi_machine_kexec_prepare;
#else
int (*_machine_kexec_prepare)(struct kimage *) = NULL;
#endif
--------------------------

In this way, any platform (config MIPS_GENERIC at this moment) can do
"select UHI_BOOT" if applicable. So this will be a one-line change to
UHI platforms that have not been upstreamed.



Regards,

Dengcheng

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

*From:* Paul Burton <mailto:paul.burton@mips.com>
*Sent:* Tuesday, July 24, 2018 4:36PM
*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 v3 6/6] MIPS: kexec: Use prepare method from generic 
platform as default option

Hi Dengcheng,

On Mon, Jul 23, 2018 at 07:48:19AM -0700, Dengcheng Zhu wrote:

> The kexec_prepare method for the generic platform should be applicable to
> other platforms.

...only for platforms which use the UHI boot protocol, which is what
generic_kexec_prepare() sets up kexec_args for. On any platforms which
don't use this boot protocol the new kernel wound find unexpected
arguments & in the worst case do something crazy with them.

> For those otherwise, like Octeon, they will use their own
> _machine_kexec_prepare().
>
> Without the default prepare work, platforms other than the generic one will
> not be able to automatically set up command line correctly for the new
> kernel.

So even with this patch they still can't unless they happen to use the
UHI boot protocol.

If we ever have multiple in-tree platforms which make use of the UHI
boot protocol & want to use kexec then I'd be fine with us moving
generic_kexec_prepare() & renaming it something like uhi_kexec_prepare()
but I prefer that we don't just presume the boot protocol for any
platform that doesn't set _machine_kexec_prepare.

Hopefully anyone using the UHI boot protocol is doing so because they
want to fit in with the generic kernel platform so this won't happen
anyway (just need to get EVA supported by the generic platform & then
there'd be no reason at all not to use it).

Interestingly there's a patch in patchwork from last year which aimed at
making the default to pass arguments the current kernel was given to the
new kernel [1]. That would seem like a saner default since we'd be
making no presumptions about what those arguments should actually be,
but I don't think that's safe either because if any of those arguments
are pointers we have no guarantees that we haven't overwritten the
memory they're pointing at.

Thanks,
     Paul

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