This fixes a regression in the for-next/core branch
Dave Kleikamp (1):
arm64: kexec: machine_kexec should call __flush_icache_range
arch/arm64/kernel/machine_kexec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
machine_kexec flushes the reboot_code_buffer from the icache
after stopping the other cpus.
Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
for kernel mappings") added an IPI call to flush_icache_range, which
causes a hang here, so replace the call with __flush_icache_range
Signed-off-by: Dave Kleikamp <[email protected]>
Cc: AKASHI Takahiro <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/kernel/machine_kexec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
index f62effc..e8c0283 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -184,7 +184,7 @@ void machine_kexec(struct kimage *kimage)
/* Flush the reboot_code_buffer in preparation for its execution. */
__flush_dcache_area(reboot_code_buffer, arm64_relocate_new_kernel_size);
- flush_icache_range((uintptr_t)reboot_code_buffer,
+ __flush_icache_range((uintptr_t)reboot_code_buffer,
arm64_relocate_new_kernel_size);
/* Flush the kimage list and its buffers. */
--
1.8.3.1
On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
> machine_kexec flushes the reboot_code_buffer from the icache
> after stopping the other cpus.
>
> Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
> for kernel mappings") added an IPI call to flush_icache_range, which
> causes a hang here, so replace the call with __flush_icache_range
While machine_kexec() may be called with interrupts disabled (IIUC) and
we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
there any other CPUs online at this point?
> Signed-off-by: Dave Kleikamp <[email protected]>
> Cc: AKASHI Takahiro <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> arch/arm64/kernel/machine_kexec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
> index f62effc..e8c0283 100644
> --- a/arch/arm64/kernel/machine_kexec.c
> +++ b/arch/arm64/kernel/machine_kexec.c
> @@ -184,7 +184,7 @@ void machine_kexec(struct kimage *kimage)
>
> /* Flush the reboot_code_buffer in preparation for its execution. */
> __flush_dcache_area(reboot_code_buffer, arm64_relocate_new_kernel_size);
> - flush_icache_range((uintptr_t)reboot_code_buffer,
> + __flush_icache_range((uintptr_t)reboot_code_buffer,
> arm64_relocate_new_kernel_size);
That's probably needed, at least to avoid a WARN_ON(irqs_disabled()) via
smp_call_function_many().
--
Catalin
On Mon, Jul 30, 2018 at 05:16:42PM +0100, Catalin Marinas wrote:
> On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
> > machine_kexec flushes the reboot_code_buffer from the icache
> > after stopping the other cpus.
> >
> > Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
> > for kernel mappings") added an IPI call to flush_icache_range, which
> > causes a hang here, so replace the call with __flush_icache_range
>
> While machine_kexec() may be called with interrupts disabled (IIUC) and
> we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
> there any other CPUs online at this point?
The BUG_ON and WARN_ON at the start of machine_kexec() suggest to me that
this should only happen if we're kexec'ing a crash kernel and
smp_crash_stop_failed(). Is that something we need to care about?
Will
On Mon, Jul 30, 2018 at 05:22:35PM +0100, Will Deacon wrote:
> On Mon, Jul 30, 2018 at 05:16:42PM +0100, Catalin Marinas wrote:
> > On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
> > > machine_kexec flushes the reboot_code_buffer from the icache
> > > after stopping the other cpus.
> > >
> > > Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
> > > for kernel mappings") added an IPI call to flush_icache_range, which
> > > causes a hang here, so replace the call with __flush_icache_range
> >
> > While machine_kexec() may be called with interrupts disabled (IIUC) and
> > we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
> > there any other CPUs online at this point?
>
> The BUG_ON and WARN_ON at the start of machine_kexec() suggest to me that
> this should only happen if we're kexec'ing a crash kernel and
> smp_crash_stop_failed(). Is that something we need to care about?
I guess we still need to be able to kexec the crash kernel to get as
much information as we can about the failure.
--
Catalin
On 07/30/2018 11:22 AM, Will Deacon wrote:
> On Mon, Jul 30, 2018 at 05:16:42PM +0100, Catalin Marinas wrote:
>> On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
>>> machine_kexec flushes the reboot_code_buffer from the icache
>>> after stopping the other cpus.
>>>
>>> Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
>>> for kernel mappings") added an IPI call to flush_icache_range, which
>>> causes a hang here, so replace the call with __flush_icache_range
>>
>> While machine_kexec() may be called with interrupts disabled (IIUC) and
>> we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
>> there any other CPUs online at this point?
>
> The BUG_ON and WARN_ON at the start of machine_kexec() suggest to me that
> this should only happen if we're kexec'ing a crash kernel and
> smp_crash_stop_failed(). Is that something we need to care about?
I observed the hang trying to kexec a crash kernel and I did not see the
warning that smp_crash_stop_failed(). I'm not exactly sure why
flush_icache_range() hung (but it did), but I think that
__flush_icache_range() makes more sense here anyway.
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
On Mon, Jul 30, 2018 at 11:46:24AM -0500, Dave Kleikamp wrote:
> On 07/30/2018 11:22 AM, Will Deacon wrote:
> > On Mon, Jul 30, 2018 at 05:16:42PM +0100, Catalin Marinas wrote:
> >> On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
> >>> machine_kexec flushes the reboot_code_buffer from the icache
> >>> after stopping the other cpus.
> >>>
> >>> Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
> >>> for kernel mappings") added an IPI call to flush_icache_range, which
> >>> causes a hang here, so replace the call with __flush_icache_range
> >>
> >> While machine_kexec() may be called with interrupts disabled (IIUC) and
> >> we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
> >> there any other CPUs online at this point?
> >
> > The BUG_ON and WARN_ON at the start of machine_kexec() suggest to me that
> > this should only happen if we're kexec'ing a crash kernel and
> > smp_crash_stop_failed(). Is that something we need to care about?
>
> I observed the hang trying to kexec a crash kernel and I did not see the
> warning that smp_crash_stop_failed(). I'm not exactly sure why
> flush_icache_range() hung (but it did), but I think that
> __flush_icache_range() makes more sense here anyway.
Yeah, I'll pick the patch up, but it would be nice to understand the
failure case you observed.
Will
On 07/30/2018 11:57 AM, Will Deacon wrote:
> On Mon, Jul 30, 2018 at 11:46:24AM -0500, Dave Kleikamp wrote:
>> On 07/30/2018 11:22 AM, Will Deacon wrote:
>>> On Mon, Jul 30, 2018 at 05:16:42PM +0100, Catalin Marinas wrote:
>>>> On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
>>>>> machine_kexec flushes the reboot_code_buffer from the icache
>>>>> after stopping the other cpus.
>>>>>
>>>>> Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
>>>>> for kernel mappings") added an IPI call to flush_icache_range, which
>>>>> causes a hang here, so replace the call with __flush_icache_range
>>>>
>>>> While machine_kexec() may be called with interrupts disabled (IIUC) and
>>>> we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
>>>> there any other CPUs online at this point?
>>>
>>> The BUG_ON and WARN_ON at the start of machine_kexec() suggest to me that
>>> this should only happen if we're kexec'ing a crash kernel and
>>> smp_crash_stop_failed(). Is that something we need to care about?
>>
>> I observed the hang trying to kexec a crash kernel and I did not see the
>> warning that smp_crash_stop_failed(). I'm not exactly sure why
>> flush_icache_range() hung (but it did), but I think that
>> __flush_icache_range() makes more sense here anyway.
>
> Yeah, I'll pick the patch up, but it would be nice to understand the
> failure case you observed.
I'll dig a little deeper.
Thanks,
Dave
>
> Will
>
On 07/30/2018 11:57 AM, Will Deacon wrote:
> On Mon, Jul 30, 2018 at 11:46:24AM -0500, Dave Kleikamp wrote:
>> On 07/30/2018 11:22 AM, Will Deacon wrote:
>>> On Mon, Jul 30, 2018 at 05:16:42PM +0100, Catalin Marinas wrote:
>>>> On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
>>>>> machine_kexec flushes the reboot_code_buffer from the icache
>>>>> after stopping the other cpus.
>>>>>
>>>>> Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
>>>>> for kernel mappings") added an IPI call to flush_icache_range, which
>>>>> causes a hang here, so replace the call with __flush_icache_range
>>>>
>>>> While machine_kexec() may be called with interrupts disabled (IIUC) and
>>>> we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
>>>> there any other CPUs online at this point?
>>>
>>> The BUG_ON and WARN_ON at the start of machine_kexec() suggest to me that
>>> this should only happen if we're kexec'ing a crash kernel and
>>> smp_crash_stop_failed(). Is that something we need to care about?
>>
>> I observed the hang trying to kexec a crash kernel and I did not see the
>> warning that smp_crash_stop_failed(). I'm not exactly sure why
>> flush_icache_range() hung (but it did), but I think that
>> __flush_icache_range() makes more sense here anyway.
>
> Yeah, I'll pick the patch up, but it would be nice to understand the
> failure case you observed.
I see why it failed. ipi_cpu_crash_stop() does not call
set_cpu_online(cpu, false) the way ipi_cpu_stop() does. So
cpu_online_mask is still populated with the stopped cpus.
Any reason why it isn't called there?
Thanks,
Dave
>
> Will
>
On Mon, Jul 30, 2018 at 04:36:28PM -0500, Dave Kleikamp wrote:
> On 07/30/2018 11:57 AM, Will Deacon wrote:
> > On Mon, Jul 30, 2018 at 11:46:24AM -0500, Dave Kleikamp wrote:
> >> On 07/30/2018 11:22 AM, Will Deacon wrote:
> >>> On Mon, Jul 30, 2018 at 05:16:42PM +0100, Catalin Marinas wrote:
> >>>> On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
> >>>>> machine_kexec flushes the reboot_code_buffer from the icache
> >>>>> after stopping the other cpus.
> >>>>>
> >>>>> Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
> >>>>> for kernel mappings") added an IPI call to flush_icache_range, which
> >>>>> causes a hang here, so replace the call with __flush_icache_range
> >>>>
> >>>> While machine_kexec() may be called with interrupts disabled (IIUC) and
> >>>> we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
> >>>> there any other CPUs online at this point?
> >>>
> >>> The BUG_ON and WARN_ON at the start of machine_kexec() suggest to me that
> >>> this should only happen if we're kexec'ing a crash kernel and
> >>> smp_crash_stop_failed(). Is that something we need to care about?
> >>
> >> I observed the hang trying to kexec a crash kernel and I did not see the
> >> warning that smp_crash_stop_failed(). I'm not exactly sure why
> >> flush_icache_range() hung (but it did), but I think that
> >> __flush_icache_range() makes more sense here anyway.
> >
> > Yeah, I'll pick the patch up, but it would be nice to understand the
> > failure case you observed.
>
> I see why it failed. ipi_cpu_crash_stop() does not call
> set_cpu_online(cpu, false) the way ipi_cpu_stop() does. So
> cpu_online_mask is still populated with the stopped cpus.
>
> Any reason why it isn't called there?
Because I wanted that saved cpu-related state be as close to as it was
at panic.
If cpus go offline, the core dump would show that all the cores but
a panicked one be offline whether or not they actually were.
Thanks,
-Takahiro AKASHI
> Thanks,
> Dave
>
> >
> > Will
> >
On 07/30/2018 07:28 PM, AKASHI Takahiro wrote:
> On Mon, Jul 30, 2018 at 04:36:28PM -0500, Dave Kleikamp wrote:
>> On 07/30/2018 11:57 AM, Will Deacon wrote:
>>> On Mon, Jul 30, 2018 at 11:46:24AM -0500, Dave Kleikamp wrote:
>>>> On 07/30/2018 11:22 AM, Will Deacon wrote:
>>>>> On Mon, Jul 30, 2018 at 05:16:42PM +0100, Catalin Marinas wrote:
>>>>>> On Mon, Jul 30, 2018 at 10:29:21AM -0500, Dave Kleikamp wrote:
>>>>>>> machine_kexec flushes the reboot_code_buffer from the icache
>>>>>>> after stopping the other cpus.
>>>>>>>
>>>>>>> Commit 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache
>>>>>>> for kernel mappings") added an IPI call to flush_icache_range, which
>>>>>>> causes a hang here, so replace the call with __flush_icache_range
>>>>>>
>>>>>> While machine_kexec() may be called with interrupts disabled (IIUC) and
>>>>>> we shouldn't IPI other CPUs, I don't understand why it hangs here. Are
>>>>>> there any other CPUs online at this point?
>>>>>
>>>>> The BUG_ON and WARN_ON at the start of machine_kexec() suggest to me that
>>>>> this should only happen if we're kexec'ing a crash kernel and
>>>>> smp_crash_stop_failed(). Is that something we need to care about?
>>>>
>>>> I observed the hang trying to kexec a crash kernel and I did not see the
>>>> warning that smp_crash_stop_failed(). I'm not exactly sure why
>>>> flush_icache_range() hung (but it did), but I think that
>>>> __flush_icache_range() makes more sense here anyway.
>>>
>>> Yeah, I'll pick the patch up, but it would be nice to understand the
>>> failure case you observed.
>>
>> I see why it failed. ipi_cpu_crash_stop() does not call
>> set_cpu_online(cpu, false) the way ipi_cpu_stop() does. So
>> cpu_online_mask is still populated with the stopped cpus.
>>
>> Any reason why it isn't called there?
>
> Because I wanted that saved cpu-related state be as close to as it was
> at panic.
> If cpus go offline, the core dump would show that all the cores but
> a panicked one be offline whether or not they actually were.
That makes sense.
Thanks,
Dave
>
> Thanks,
> -Takahiro AKASHI
>
>> Thanks,
>> Dave
>>
>>>
>>> Will
>>>