* [RFC PATCH] ARM: Flush L2 cache on soft_restart
@ 2013-10-02 11:34 Taras Kondratiuk
2013-10-02 12:49 ` Will Deacon
0 siblings, 1 reply; 6+ messages in thread
From: Taras Kondratiuk @ 2013-10-02 11:34 UTC (permalink / raw)
To: linux-arm-kernel
Kexec disables L2 cache before jumping to reboot code,
but it doesn't flush it. So often just copied reboot code
gets corrupted, because part of it is stored in L2 cache
and have not reached memory.
Flushing cache prevents this corruption.
I'm facing this issue on Pandaboard ES, but it looks like
similar issue is observed on TC2 board [1].
[1] http://www.spinics.net/lists/arm-kernel/msg264339.html
Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
---
arch/arm/kernel/process.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 94f6b05..e359b62 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -103,9 +103,11 @@ void soft_restart(unsigned long addr)
local_irq_disable();
local_fiq_disable();
- /* Disable the L2 if we're the last man standing. */
- if (num_online_cpus() == 1)
+ /* Flush and disable the L2 if we're the last man standing. */
+ if (num_online_cpus() == 1) {
+ outer_flush_all();
outer_disable();
+ }
/* Change to the new stack and continue with the reset. */
call_with_stack(__soft_restart, (void *)addr, (void *)stack);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH] ARM: Flush L2 cache on soft_restart
2013-10-02 11:34 [RFC PATCH] ARM: Flush L2 cache on soft_restart Taras Kondratiuk
@ 2013-10-02 12:49 ` Will Deacon
2013-10-02 17:19 ` Taras Kondratiuk
2013-10-02 21:01 ` Russell King - ARM Linux
0 siblings, 2 replies; 6+ messages in thread
From: Will Deacon @ 2013-10-02 12:49 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 02, 2013 at 12:34:16PM +0100, Taras Kondratiuk wrote:
> Kexec disables L2 cache before jumping to reboot code,
> but it doesn't flush it. So often just copied reboot code
> gets corrupted, because part of it is stored in L2 cache
> and have not reached memory.
>
> Flushing cache prevents this corruption.
>
> I'm facing this issue on Pandaboard ES, but it looks like
> similar issue is observed on TC2 board [1].
TC2 doesn't have an outer cache, so that report is not relevant to this
patch.
> [1] http://www.spinics.net/lists/arm-kernel/msg264339.html
>
> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
> ---
> arch/arm/kernel/process.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 94f6b05..e359b62 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -103,9 +103,11 @@ void soft_restart(unsigned long addr)
> local_irq_disable();
> local_fiq_disable();
>
> - /* Disable the L2 if we're the last man standing. */
> - if (num_online_cpus() == 1)
> + /* Flush and disable the L2 if we're the last man standing. */
> + if (num_online_cpus() == 1) {
> + outer_flush_all();
> outer_disable();
l2x0_disable already contains a flush, so this doesn't change anything.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH] ARM: Flush L2 cache on soft_restart
2013-10-02 12:49 ` Will Deacon
@ 2013-10-02 17:19 ` Taras Kondratiuk
2013-10-02 17:31 ` Will Deacon
2013-10-02 21:01 ` Russell King - ARM Linux
1 sibling, 1 reply; 6+ messages in thread
From: Taras Kondratiuk @ 2013-10-02 17:19 UTC (permalink / raw)
To: linux-arm-kernel
On 2 October 2013 15:49, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Oct 02, 2013 at 12:34:16PM +0100, Taras Kondratiuk wrote:
>> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
>> index 94f6b05..e359b62 100644
>> --- a/arch/arm/kernel/process.c
>> +++ b/arch/arm/kernel/process.c
>> @@ -103,9 +103,11 @@ void soft_restart(unsigned long addr)
>> local_irq_disable();
>> local_fiq_disable();
>>
>> - /* Disable the L2 if we're the last man standing. */
>> - if (num_online_cpus() == 1)
>> + /* Flush and disable the L2 if we're the last man standing. */
>> + if (num_online_cpus() == 1) {
>> + outer_flush_all();
>> outer_disable();
>
> l2x0_disable already contains a flush, so this doesn't change anything.
Unfortunately not everybody uses l2x0_disable().
SoC's that use SMC calls for L2 cache maintenance have its own implementation
of outer_cache.disable which usually doesn't flush cache implicitly.
--
Regards,
Taras Kondratiuk
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH] ARM: Flush L2 cache on soft_restart
2013-10-02 17:19 ` Taras Kondratiuk
@ 2013-10-02 17:31 ` Will Deacon
2013-10-03 22:32 ` Taras Kondratiuk
0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2013-10-02 17:31 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 02, 2013 at 06:19:30PM +0100, Taras Kondratiuk wrote:
> On 2 October 2013 15:49, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Oct 02, 2013 at 12:34:16PM +0100, Taras Kondratiuk wrote:
> >> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> >> index 94f6b05..e359b62 100644
> >> --- a/arch/arm/kernel/process.c
> >> +++ b/arch/arm/kernel/process.c
> >> @@ -103,9 +103,11 @@ void soft_restart(unsigned long addr)
> >> local_irq_disable();
> >> local_fiq_disable();
> >>
> >> - /* Disable the L2 if we're the last man standing. */
> >> - if (num_online_cpus() == 1)
> >> + /* Flush and disable the L2 if we're the last man standing. */
> >> + if (num_online_cpus() == 1) {
> >> + outer_flush_all();
> >> outer_disable();
> >
> > l2x0_disable already contains a flush, so this doesn't change anything.
>
> Unfortunately not everybody uses l2x0_disable().
> SoC's that use SMC calls for L2 cache maintenance have its own implementation
> of outer_cache.disable which usually doesn't flush cache implicitly.
In which case, we should probably fix the disabling code to make a flush
then update callers not to bother with redundant flushing. The flushing
during the disable code is likely required anyway if there's any
synchronisation going on.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH] ARM: Flush L2 cache on soft_restart
2013-10-02 12:49 ` Will Deacon
2013-10-02 17:19 ` Taras Kondratiuk
@ 2013-10-02 21:01 ` Russell King - ARM Linux
1 sibling, 0 replies; 6+ messages in thread
From: Russell King - ARM Linux @ 2013-10-02 21:01 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 02, 2013 at 01:49:03PM +0100, Will Deacon wrote:
> On Wed, Oct 02, 2013 at 12:34:16PM +0100, Taras Kondratiuk wrote:
> > Kexec disables L2 cache before jumping to reboot code,
> > but it doesn't flush it. So often just copied reboot code
> > gets corrupted, because part of it is stored in L2 cache
> > and have not reached memory.
> >
> > Flushing cache prevents this corruption.
> >
> > I'm facing this issue on Pandaboard ES, but it looks like
> > similar issue is observed on TC2 board [1].
>
> TC2 doesn't have an outer cache, so that report is not relevant to this
> patch.
>
> > [1] http://www.spinics.net/lists/arm-kernel/msg264339.html
> >
> > Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
> > ---
> > arch/arm/kernel/process.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> > index 94f6b05..e359b62 100644
> > --- a/arch/arm/kernel/process.c
> > +++ b/arch/arm/kernel/process.c
> > @@ -103,9 +103,11 @@ void soft_restart(unsigned long addr)
> > local_irq_disable();
> > local_fiq_disable();
> >
> > - /* Disable the L2 if we're the last man standing. */
> > - if (num_online_cpus() == 1)
> > + /* Flush and disable the L2 if we're the last man standing. */
> > + if (num_online_cpus() == 1) {
> > + outer_flush_all();
> > outer_disable();
>
> l2x0_disable already contains a flush, so this doesn't change anything.
The disable call _has_ to contain the flush as that's the only way it
can be disabled to avoid any loss of data when the outer cache stops
being searched for cache hits.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH] ARM: Flush L2 cache on soft_restart
2013-10-02 17:31 ` Will Deacon
@ 2013-10-03 22:32 ` Taras Kondratiuk
0 siblings, 0 replies; 6+ messages in thread
From: Taras Kondratiuk @ 2013-10-03 22:32 UTC (permalink / raw)
To: linux-arm-kernel
On 2 October 2013 20:31, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Oct 02, 2013 at 06:19:30PM +0100, Taras Kondratiuk wrote:
>> On 2 October 2013 15:49, Will Deacon <will.deacon@arm.com> wrote:
>> > On Wed, Oct 02, 2013 at 12:34:16PM +0100, Taras Kondratiuk wrote:
>> >> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
>> >> index 94f6b05..e359b62 100644
>> >> --- a/arch/arm/kernel/process.c
>> >> +++ b/arch/arm/kernel/process.c
>> >> @@ -103,9 +103,11 @@ void soft_restart(unsigned long addr)
>> >> local_irq_disable();
>> >> local_fiq_disable();
>> >>
>> >> - /* Disable the L2 if we're the last man standing. */
>> >> - if (num_online_cpus() == 1)
>> >> + /* Flush and disable the L2 if we're the last man standing. */
>> >> + if (num_online_cpus() == 1) {
>> >> + outer_flush_all();
>> >> outer_disable();
>> >
>> > l2x0_disable already contains a flush, so this doesn't change anything.
>>
>> Unfortunately not everybody uses l2x0_disable().
>> SoC's that use SMC calls for L2 cache maintenance have its own implementation
>> of outer_cache.disable which usually doesn't flush cache implicitly.
>
> In which case, we should probably fix the disabling code to make a flush
> then update callers not to bother with redundant flushing. The flushing
> during the disable code is likely required anyway if there's any
> synchronisation going on.
It makes sense, but a history of the current code looks a bit confusing.
Implicit flush was added in l2x0_disable() as a "side effect" of
commit 38a8914f9ac2379293944f613e6ca24b61373de8
"ARM: 6987/1: l2x0: fix disabling function to avoid deadlock",
while initially disable was just a disable.
Maybe it worth to explicitly document that .disable callback must flush cache?
--
Regards,
Taras Kondratiuk
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-03 22:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-02 11:34 [RFC PATCH] ARM: Flush L2 cache on soft_restart Taras Kondratiuk
2013-10-02 12:49 ` Will Deacon
2013-10-02 17:19 ` Taras Kondratiuk
2013-10-02 17:31 ` Will Deacon
2013-10-03 22:32 ` Taras Kondratiuk
2013-10-02 21:01 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).