Skip to contentSkip to conversation

ARM: Disable outer cache before kexec call -V2

PATCH New Thomas Gleixner 2 participants 9 messages Jul 5, 2010

lore
Share & tools
lore

Thomas Gleixner

author
#0 lore raw

cover ARM: Disable outer cache before kexec call -V2

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

#2 lore raw

PATCH 1/2 arm: Disable outer (L2) cache in kexec

Quoted 16 lines · Thomas Gleixner

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 <[email protected]>

Index: linux-2.6/arch/arm/include/asm/outercache.h
===================================================================

arch/arm/include/asm/outercache.h struct outer_cache_fns { · line 30

Quoted 6 lines
arch/arm/include/asm/outercache.h +3 -0
@@ -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.

arch/arm/include/asm/outercache.h struct outer_cache_fns {

Quoted 5 lines
arch/arm/include/asm/outercache.h +5 -0
+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_*().

arch/arm/kernel/machine_kexec.c void machine_kexec(struct kimage *image) · line 80

Quoted 10 lines
arch/arm/kernel/machine_kexec.c +3 -0
@@ -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.

Signature · 1 line
Catalin
author
#3 lore raw

PATCH 1/2 arm: Disable outer (L2) cache in kexec

Quoted 11 lines · Catalin Marinas
Quoted 10 lines · Thomas Gleixner

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.

Quoted 15 lines

arch/arm/include/asm/outercache.h struct outer_cache_fns { · line 30

Quoted 12 lines
Signed-off-by Thomas Gleixner <[email protected]>

Index: linux-2.6/arch/arm/include/asm/outercache.h
===================================================================

arch/arm/include/asm/outercache.h +3 -0
@@ -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

#4 lore raw

PATCH 1/2 arm: Disable outer (L2) cache in kexec

Quoted 29 lines · Thomas Gleixner
Quoted 11 lines · Catalin Marinas
Quoted 10 lines · Thomas Gleixner

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.

Quoted 15 lines

arch/arm/include/asm/outercache.h struct outer_cache_fns { · line 30

Quoted 12 lines
Signed-off-by Thomas Gleixner <[email protected]>

Index: linux-2.6/arch/arm/include/asm/outercache.h
===================================================================

arch/arm/include/asm/outercache.h +3 -0
@@ -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

Quoted 4 lines

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.

Signature · 1 line
Catalin
#6 lore raw

PATCH 2/2 arm: Implement l2x0 cache disable functions

Quoted 8 lines · Thomas Gleixner
arch/arm/mm/cache-l2x0.c +1 -1
@@ -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)

arch/arm/mm/cache-l2x0.c static void l2x0_cache_sync(void) · line 116

Quoted 10 lines
arch/arm/mm/cache-l2x0.c +10 -0
+{
+       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.

Signature · 1 line
Catalin
author
#7 lore raw

PATCH 2/2 arm: Implement l2x0 cache disable functions

Quoted 7 lines · Catalin Marinas
Quoted 6 lines · Thomas Gleixner
arch/arm/mm/cache-l2x0.c +0 -0
@@ -103,7 +103,19 @@ static void l2x0_cache_sync(void)
        spin_unlock_irqrestore(&l2x0_lock, flags);
 }
Quoted 16 lines

arch/arm/mm/cache-l2x0.c static void l2x0_cache_sync(void) · line 116

Quoted 12 lines
arch/arm/mm/cache-l2x0.c +11 -1
-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

#8 lore raw

PATCH 2/2 arm: Implement l2x0 cache disable functions

Quoted 27 lines · Thomas Gleixner
Quoted 7 lines · Catalin Marinas
Quoted 6 lines · Thomas Gleixner
arch/arm/mm/cache-l2x0.c +0 -0
@@ -103,7 +103,19 @@ static void l2x0_cache_sync(void)
        spin_unlock_irqrestore(&l2x0_lock, flags);
 }
Quoted 16 lines

arch/arm/mm/cache-l2x0.c static void l2x0_cache_sync(void) · line 116

Quoted 12 lines
arch/arm/mm/cache-l2x0.c +11 -1
-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.

Quoted 1 line

So CLEAN_INV_WAY is the right opcode ?

Yes.

Signature · 1 line
Catalin