[patch 0/2] [patch 0/2] ARM: Disable outer cache before kexec call -V2

[patch 0/2] [patch 0/2] ARM: Disable outer cache before kexec call -V2

[Thomas Gleixner]

The following patch series addresses the problem, that the kexec code
does not disable the outer cache before disabling the inner cache and
jumping into the new kernel. This results in random crashes of the new
kernel.

Changes since version 1:

 - added flush_all() and inv_all() functions to outer_cache_fns
   so OMAP can override the disable() function easily.

 - changed the implementation from inv_all() -> disable() to
   flush_all() -> disable() -> inv_all() according to feedback from
   Russell and Catalin.

Thanks,

	tglx

[patch 1/2] arm: Disable outer (L2) cache in kexec

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-outer-cache-disable-for-kexec.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100705/76e5d455/attachment.ksh>

[patch 1/2] arm: Disable outer (L2) cache in kexec

[Catalin Marinas]

On Mon, 2010-07-05 at 10:25 +0100, Thomas Gleixner wrote:
> kexec does not disable the outer cache before disabling the inner
> caches in cpu_proc_fin(). So L2 is enabled across the kexec jump. When
> the new kernel enables chaches again, it randomly crashes.
> 
> Disabling L2 before calling cpu_proc_fin() cures the problem.
> 
> Disabling L2 requires the following new functions: flush_all(),
> inv_all() and disable(). Add them to outer_cache_fns and call them
> from the kexec code.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Index: linux-2.6/arch/arm/include/asm/outercache.h
> ===================================================================
> --- linux-2.6.orig/arch/arm/include/asm/outercache.h
> +++ linux-2.6/arch/arm/include/asm/outercache.h
> @@ -25,6 +25,9 @@ struct outer_cache_fns {
>         void (*inv_range)(unsigned long, unsigned long);
>         void (*clean_range)(unsigned long, unsigned long);
>         void (*flush_range)(unsigned long, unsigned long);
> +       void (*flush_all)(void);
> +       void (*inv_all)(void);
> +       void (*disable)(void);

Why don't we handle the flush_all/inv_all in the disable function and
avoid exporting them? Once exported, people may start using them in
drivers and they should use the DMA API for this. The inv_all is also
dangerous if called when the L2 is enabled.

> +static inline void outer_cache_disable(void)
> +{
> +       if (outer_cache.disable)
> +               outer_cache.disable();
> +}

Personal preference - "outer_disable()" as we don't call any of the
other functions outer_cache_*().

> --- linux-2.6.orig/arch/arm/kernel/machine_kexec.c
> +++ linux-2.6/arch/arm/kernel/machine_kexec.c
> @@ -74,7 +74,10 @@ void machine_kexec(struct kimage *image)
>                            (unsigned long) reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE);
>         printk(KERN_INFO "Bye!\n");
> 
> +       outer_flush_all();
> +       outer_cache_disable();
>         cpu_proc_fin();
> +       outer_inv_all();

As I said above, we could have just outer_disable() here which does
outer_flush_all, disabling and outer_inv_all. I don't think the
invalidation needs to be done after cup_proc_fin().

Thanks.

-- 
Catalin

[patch 1/2] arm: Disable outer (L2) cache in kexec

[Thomas Gleixner]

On Mon, 5 Jul 2010, Catalin Marinas wrote:

> On Mon, 2010-07-05 at 10:25 +0100, Thomas Gleixner wrote:
> > kexec does not disable the outer cache before disabling the inner
> > caches in cpu_proc_fin(). So L2 is enabled across the kexec jump. When
> > the new kernel enables chaches again, it randomly crashes.
> > 
> > Disabling L2 before calling cpu_proc_fin() cures the problem.
> > 
> > Disabling L2 requires the following new functions: flush_all(),
> > inv_all() and disable(). Add them to outer_cache_fns and call them
> > from the kexec code.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Index: linux-2.6/arch/arm/include/asm/outercache.h
> > ===================================================================
> > --- linux-2.6.orig/arch/arm/include/asm/outercache.h
> > +++ linux-2.6/arch/arm/include/asm/outercache.h
> > @@ -25,6 +25,9 @@ struct outer_cache_fns {
> >         void (*inv_range)(unsigned long, unsigned long);
> >         void (*clean_range)(unsigned long, unsigned long);
> >         void (*flush_range)(unsigned long, unsigned long);
> > +       void (*flush_all)(void);
> > +       void (*inv_all)(void);
> > +       void (*disable)(void);
> 
> Why don't we handle the flush_all/inv_all in the disable function and
> avoid exporting them? Once exported, people may start using them in

Because OMAP needs them to avoid reimplementing everything in the OMAP
part. So OMAP only needs to override the disable function. And if
Russell wants to add his cleanup patch which he posted last week, then
the split and the export needs to be done anyway.

Thanks,

	tglx

[patch 1/2] arm: Disable outer (L2) cache in kexec

[Catalin Marinas]

On Mon, 2010-07-05 at 12:13 +0100, Thomas Gleixner wrote:
> On Mon, 5 Jul 2010, Catalin Marinas wrote:
> 
> > On Mon, 2010-07-05 at 10:25 +0100, Thomas Gleixner wrote:
> > > kexec does not disable the outer cache before disabling the inner
> > > caches in cpu_proc_fin(). So L2 is enabled across the kexec jump. When
> > > the new kernel enables chaches again, it randomly crashes.
> > >
> > > Disabling L2 before calling cpu_proc_fin() cures the problem.
> > >
> > > Disabling L2 requires the following new functions: flush_all(),
> > > inv_all() and disable(). Add them to outer_cache_fns and call them
> > > from the kexec code.
> > >
> > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > > Index: linux-2.6/arch/arm/include/asm/outercache.h
> > > ===================================================================
> > > --- linux-2.6.orig/arch/arm/include/asm/outercache.h
> > > +++ linux-2.6/arch/arm/include/asm/outercache.h
> > > @@ -25,6 +25,9 @@ struct outer_cache_fns {
> > >         void (*inv_range)(unsigned long, unsigned long);
> > >         void (*clean_range)(unsigned long, unsigned long);
> > >         void (*flush_range)(unsigned long, unsigned long);
> > > +       void (*flush_all)(void);
> > > +       void (*inv_all)(void);
> > > +       void (*disable)(void);
> >
> > Why don't we handle the flush_all/inv_all in the disable function and
> > avoid exporting them? Once exported, people may start using them in
> 
> Because OMAP needs them to avoid reimplementing everything in the OMAP
> part. So OMAP only needs to override the disable function. And if
> Russell wants to add his cleanup patch which he posted last week, then
> the split and the export needs to be done anyway.

OK, good point.

Thanks.

-- 
Catalin

[patch 2/2] arm: Implement l2x0 cache disable functions

[Thomas Gleixner]

An embedded and charset-unspecified text was scrubbed...
Name: arm-l2x0-cache-disable.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100705/e4216032/attachment.ksh>

[patch 2/2] arm: Implement l2x0 cache disable functions

[Catalin Marinas]

On Mon, 2010-07-05 at 10:25 +0100, Thomas Gleixner wrote:
> --- linux-2.6.orig/arch/arm/mm/cache-l2x0.c
> +++ linux-2.6/arch/arm/mm/cache-l2x0.c
> @@ -103,7 +103,19 @@ static void l2x0_cache_sync(void)
>         spin_unlock_irqrestore(&l2x0_lock, flags);
>  }
> 
> -static inline void l2x0_inv_all(void)
> +static void l2x0_flush_all(void)
> +{
> +       unsigned long flags;
> +
> +       /* clean all ways */
> +       spin_lock_irqsave(&l2x0_lock, flags);
> +       writel(l2x0_way_mask, l2x0_base + L2X0_CLEAN_WAY);
> +       cache_wait(l2x0_base + L2X0_CLEAN_WAY, l2x0_way_mask);
> +       cache_sync();
> +       spin_unlock_irqrestore(&l2x0_lock, flags);
> +}

Since it only does cleaning, we should probably call it l2x0_clean_all
(the same for the outer_cache_fns entry). I think on ARM we
traditionally referred to flush as clean+invalidate.

-- 
Catalin

[patch 2/2] arm: Implement l2x0 cache disable functions

[Thomas Gleixner]

On Mon, 5 Jul 2010, Catalin Marinas wrote:

> On Mon, 2010-07-05 at 10:25 +0100, Thomas Gleixner wrote:
> > --- linux-2.6.orig/arch/arm/mm/cache-l2x0.c
> > +++ linux-2.6/arch/arm/mm/cache-l2x0.c
> > @@ -103,7 +103,19 @@ static void l2x0_cache_sync(void)
> >         spin_unlock_irqrestore(&l2x0_lock, flags);
> >  }
> > 
> > -static inline void l2x0_inv_all(void)
> > +static void l2x0_flush_all(void)
> > +{
> > +       unsigned long flags;
> > +
> > +       /* clean all ways */
> > +       spin_lock_irqsave(&l2x0_lock, flags);
> > +       writel(l2x0_way_mask, l2x0_base + L2X0_CLEAN_WAY);
> > +       cache_wait(l2x0_base + L2X0_CLEAN_WAY, l2x0_way_mask);
> > +       cache_sync();
> > +       spin_unlock_irqrestore(&l2x0_lock, flags);
> > +}
> 
> Since it only does cleaning, we should probably call it l2x0_clean_all
> (the same for the outer_cache_fns entry). I think on ARM we
> traditionally referred to flush as clean+invalidate.

So CLEAN_INV_WAY is the right opcode ?

Thanks,

	tglx

[patch 2/2] arm: Implement l2x0 cache disable functions

[Catalin Marinas]

On Mon, 2010-07-05 at 12:10 +0100, Thomas Gleixner wrote:
> 
> On Mon, 5 Jul 2010, Catalin Marinas wrote:
> 
> > On Mon, 2010-07-05 at 10:25 +0100, Thomas Gleixner wrote:
> > > --- linux-2.6.orig/arch/arm/mm/cache-l2x0.c
> > > +++ linux-2.6/arch/arm/mm/cache-l2x0.c
> > > @@ -103,7 +103,19 @@ static void l2x0_cache_sync(void)
> > >         spin_unlock_irqrestore(&l2x0_lock, flags);
> > >  }
> > >
> > > -static inline void l2x0_inv_all(void)
> > > +static void l2x0_flush_all(void)
> > > +{
> > > +       unsigned long flags;
> > > +
> > > +       /* clean all ways */
> > > +       spin_lock_irqsave(&l2x0_lock, flags);
> > > +       writel(l2x0_way_mask, l2x0_base + L2X0_CLEAN_WAY);
> > > +       cache_wait(l2x0_base + L2X0_CLEAN_WAY, l2x0_way_mask);
> > > +       cache_sync();
> > > +       spin_unlock_irqrestore(&l2x0_lock, flags);
> > > +}
> >
> > Since it only does cleaning, we should probably call it l2x0_clean_all
> > (the same for the outer_cache_fns entry). I think on ARM we
> > traditionally referred to flush as clean+invalidate.
> 
> So CLEAN_INV_WAY is the right opcode ?

Yes.

-- 
Catalin