Re: [PATCH v4 15/13] firmware: arm_sdei: be more robust against cpu-hotplug

Will Deacon
2017-11-13T11:01:10Z

On Wed, Nov 08, 2017 at 04:06:24PM +0000, James Morse wrote:
> dpm_suspend() calls the freeze/thaw callbacks for hibernate before
> disable_non_bootcpus() takes down secondaries.
>
> This leads to a fun race where the freeze/thaw callbacks reset the
> SDEI interface (as we may be restoring a kernel with a different
> layout due to KASLR), then the cpu-hotplug callbacks come in to
> save the current state, which has already been reset.
>
> I tried to solve this with a 'frozen' flag that stops the hotplug
> callback from overwriting the saved values. Instead this just
> moves the race around and makes it even harder to think about.
>
> Instead, make it look like the secondaries have gone offline.
> Call cpuhp_remove_state() in the freeze callback, this will call the
> teardown hook on all online CPUs, then remove the state. This saves
> all private events and makes future CPU up/down events invisible.
>
> Change sdei_event_unregister_all()/sdei_reregister_events() to
> only save/restore shared events, which are all that is left. With
> this we can remove the frozen flag. We can remove the device
> suspend/resume calls too as cpuhotplug's teardown call has masked
> the CPUs.
>
> All that is left is the reboot notifier, (which was abusing the
> frozen flag). Call cpuhp_remove_state() to make it look like
> secondary CPUs have gone offline.
>
> Suggested-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
> ---
> drivers/firmware/arm_sdei.c | 60 +++++++++++++++++++++++----------------------
> 1 file changed, 31 insertions(+), 29 deletions(-)

Thanks, this appears to address my concerns. It's too late for 4.15 now,
but please resend for 4.16 and Catalin can pick this series up.

Will