[PATCH v15 17/20] arm64: kdump: implement machine_crash_shutdown()

[takahiro.akashi@linaro.org (AKASHI Takahiro)]

Mark,

On Fri, Apr 01, 2016 at 10:36:35AM +0100, Mark Rutland wrote:
> On Fri, Apr 01, 2016 at 05:45:09PM +0900, AKASHI Takahiro wrote:
> > On Thu, Mar 31, 2016 at 11:10:38AM +0100, Mark Rutland wrote:
> > > On Thu, Mar 31, 2016 at 09:12:32AM +0100, Marc Zyngier wrote:
> > > > On 31/03/16 08:57, AKASHI Takahiro wrote:
> > > > > On Mon, Mar 21, 2016 at 01:29:28PM +0000, James Morse wrote:
> > > > >> On 18/03/16 18:08, James Morse wrote:
> > > > >>> On 14/03/16 17:48, Geoff Levand wrote:
> > > > >>>> +	/* just in case */
> > > > >>>> +	while (1)
> > > > >>>> +		wfi();
> > > > >>
> > > > >> Having thought about this some more: I don't think spinning like this is safe.
> > > > >> We need to spin with the MMU turned off, otherwise this core will pollute the
> > > > >> kdump kernel with TLB entries from the old page tables.
> > > > > 
> > > > > I think that wfi() will never wake up since local interrupts are disabled
> > > > > here. So how can it pollute the kdump kernel?
> > > > 
> > > > Having interrupts disabled doesn't prevent an exit from WFI. Quite the
> > > > opposite, actually. It is designed to wake-up the core when something
> > > > happens on the external interface.
> > > 
> > > Further, WFI is a hint, and may simply act as a NOP. 
> > 
> > Ah, OK. But even so, none of interrupt handlers (nor other code) will
> > be executed after cpu wakes up, and the memory won't be polluted.
> 
> The code comprising the while(1) loop will be executed, and TLB walks,
> speculative fetches, etc may occur regardless.
> 
> We don't share TLB entries between cores (we don't have support for
> ARMv8.2s CnP), and the kdump kernel should be running in a carveout from
> main memory (which IIUC is not mapped by the original kernel). So
> normally, this would not be a problem.

In fact, the memory region for crash kernel will be just memblock_reserve()'d.
We can't do memblock_remove() here because that region is expected to
exist as part of system memory.
(For details, see kimage_load_crash_segment() in kexec_core.c.)
Should we remove it from kernel mapping?
 
> However, if there is a problem with the page tables (e.g. entries
> erroneously point into a PA range the kdump kernel is using), then
> unavoidable background memory traffic from the CPU may cause problems
> for the kdump kernel.

But the traffic generated by the cpu will concentrate on the area around
the while loop, ipi_cpu_crash_stop(), in the *crashed* kernel's memory.
(I'm not sure about speculative behaviors.)

So I don't think it will hurt kdump kernel.

Thanks,
-Takahiro AKASHI

 
> Thanks,
> Mark.

-- 
Thanks,
-Takahiro AKASHI