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@xxxxxxxx>
*Sent:* Thursday, August 30, 2018 4:32PM
*To:* Dengcheng Zhu <mailto:dzhu@xxxxxxxxxxxx>
*Cc:* Pburton <mailto:pburton@xxxxxxxxxxxx>, Ralf
<mailto:ralf@xxxxxxxxxxxxxx>, Linux-mips
<mailto:linux-mips@xxxxxxxxxxxxxx>, Rachel.mozes
<mailto:rachel.mozes@xxxxxxxxx>
*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
