[PATCH 0/1] arm64: for-next/core: Fix hang in machine_kexec

[PATCH 0/1] arm64: for-next/core: Fix hang in machine_kexec

[Dave Kleikamp]

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(-)

[PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Dave Kleikamp]

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 <dave.kleikamp@oracle.com>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 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

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Catalin Marinas]

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 <dave.kleikamp@oracle.com>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  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

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Will Deacon]

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

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Catalin Marinas]

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

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Dave Kleikamp]

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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Will Deacon]

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

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Dave Kleikamp]

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
> 

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Dave Kleikamp]

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
> 

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[AKASHI Takahiro]

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

Re: [PATCH 1/1] arm64: kexec: machine_kexec should call __flush_icache_range

[Dave Kleikamp]

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