powerpc: Fix bogus cache flushing on all 40x and BookE processors

PATCH In review Benjamin Herrenschmidt 4 participants 6 messages Dec 1, 2008 – Dec 4, 2008 PowerPC

Share & tools

Benjamin Herrenschmidt

author

2008-12-01 06:01 UTC #0 lore raw

PATCH 1/1 powerpc: Fix bogus cache flushing on all 40x and BookE processors

We were missing the CPU_FTR_NOEXECUTE bit in our cputable for all these processors. The result is that update_mmu_cache() would flush the cache for all pages mapped to userspace which is totally unnecessary on those processors since we already handle flushing on execute in the page fault path.

This should provide a nice speed up ;-)

Signed-off-by Benjamin Herrenschmidt <[email protected]>

---

 arch/powerpc/include/asm/cputable.h |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

arch/powerpc/include/asm/cputable.h +7 -6

@@ -366,19 +366,20 @@ extern const char *powerpc_base_platform
 #define CPU_FTRS_CLASSIC32	(CPU_FTR_COMMON | \
 	    CPU_FTR_USE_TB | CPU_FTR_HPTE_TABLE)
 #define CPU_FTRS_8XX	(CPU_FTR_USE_TB)
-#define CPU_FTRS_40X	(CPU_FTR_USE_TB | CPU_FTR_NODSISRALIGN)
-#define CPU_FTRS_44X	(CPU_FTR_USE_TB | CPU_FTR_NODSISRALIGN)
+#define CPU_FTRS_40X	(CPU_FTR_USE_TB | CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
+#define CPU_FTRS_44X	(CPU_FTR_USE_TB | CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_E200	(CPU_FTR_USE_TB | CPU_FTR_SPE_COMP | \
 	    CPU_FTR_NODSISRALIGN | CPU_FTR_COHERENT_ICACHE | \
-	    CPU_FTR_UNIFIED_ID_CACHE)
+	    CPU_FTR_UNIFIED_ID_CACHE | CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_E500	(CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \
-	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN)
+	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN \
+	    CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_E500_2	(CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \
 	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_BIG_PHYS | \
-	    CPU_FTR_NODSISRALIGN)
+	    CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_E500MC	(CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \
 	    CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_BIG_PHYS | CPU_FTR_NODSISRALIGN | \
-	    CPU_FTR_L2CSR | CPU_FTR_LWSYNC)
+	    CPU_FTR_L2CSR | CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_GENERIC_32	(CPU_FTR_COMMON | CPU_FTR_NODSISRALIGN)
 
 /* 64-bit CPUs */

Kumar Gala

2008-12-02 07:36 UTC #1 lore raw

Quoted 26 lines · Benjamin Herrenschmidt

We were missing the CPU_FTR_NOEXECUTE bit in our cputable for all these processors. The result is that update_mmu_cache() would flush the cache for all pages mapped to userspace which is totally unnecessary on those processors since we already handle flushing on execute in the page fault path.
This should provide a nice speed up ;-)
Signed-off-by Benjamin Herrenschmidt <[email protected]>
---
arch/powerpc/include/asm/cputable.h | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
--- linux-work.orig/arch/powerpc/include/asm/cputable.h 2008-12-01
16:55:53.000000000 +1100
+++ linux-work/arch/powerpc/include/asm/cputable.h 2008-12-01
16:56:32.000000000 +1100
arch/powerpc/include/asm/cputable.h +1 -2
@@ -366,19 +366,20 @@ extern const char *powerpc_base_platform
 #define CPU_FTRS_CLASSIC32	(CPU_FTR_COMMON | \
 	    CPU_FTR_USE_TB | CPU_FTR_HPTE_TABLE)
 #define CPU_FTRS_8XX	(CPU_FTR_USE_TB)
-#define CPU_FTRS_40X	(CPU_FTR_USE_TB | CPU_FTR_NODSISRALIGN)
-#define CPU_FTRS_44X	(CPU_FTR_USE_TB | CPU_FTR_NODSISRALIGN)
+#define CPU_FTRS_40X	(CPU_FTR_USE_TB | CPU_FTR_NODSISRALIGN |  
 CPU_FTR_NOEXECUTE)

arch/powerpc/include/asm/cputable.h extern const char *powerpc_base_platform · line 378

Quoted 10 lines

arch/powerpc/include/asm/cputable.h +3 -2

+#define CPU_FTRS_44X	(CPU_FTR_USE_TB | CPU_FTR_NODSISRALIGN |  
 CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_E200	(CPU_FTR_USE_TB | CPU_FTR_SPE_COMP | \
 	    CPU_FTR_NODSISRALIGN | CPU_FTR_COHERENT_ICACHE | \
-	    CPU_FTR_UNIFIED_ID_CACHE)
+	    CPU_FTR_UNIFIED_ID_CACHE | CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_E500	(CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \
-	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN)
+	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP |  
 CPU_FTR_NODSISRALIGN \

Added a '|' at the end of the line before the escape

Quoted 11 lines

arch/powerpc/include/asm/cputable.h +3 -2

+	    CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_E500_2	(CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \
 	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_BIG_PHYS | \
-	    CPU_FTR_NODSISRALIGN)
+	    CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
 #define CPU_FTRS_E500MC	(CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \
 	    CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_BIG_PHYS | CPU_FTR_NODSISRALIGN  
 | \
-	    CPU_FTR_L2CSR | CPU_FTR_LWSYNC)
+	    CPU_FTR_L2CSR | CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE)

Quoted 1 line

#define CPU_FTRS_GENERIC_32 (CPU_FTR_COMMON | CPU_FTR_NODSISRALIGN)

- k

Benjamin Herrenschmidt

author

2008-12-02 09:20 UTC #2 lore raw

Quoted 10 lines · Kumar Gala

Quoted 8 lines
#define CPU_FTRS_E200	(CPU_FTR_USE_TB | CPU_FTR_SPE_COMP | \
	    CPU_FTR_NODSISRALIGN | CPU_FTR_COHERENT_ICACHE | \
arch/powerpc/include/asm/cputable.h +1 -1
-	    CPU_FTR_UNIFIED_ID_CACHE)
+	    CPU_FTR_UNIFIED_ID_CACHE | CPU_FTR_NOEXECUTE)
#define CPU_FTRS_E500 (CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \
(quoted diff) +1 -1
-	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN)
+	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP |  
CPU_FTR_NODSISRALIGN \
Added a '|' at the end of the line before the escape

Right. Will send a new patch tomorrow. Appart from that, have you verified it doesn't have any adverse effects for you ? I did some quick tests on 440 and things seem to be fine.

Cheers,
Ben.

Trent Piepho

2008-12-03 23:11 UTC #3 lore raw

Quoted 13 lines · Benjamin Herrenschmidt

Quoted 10 lines · Kumar Gala
Quoted 8 lines
#define CPU_FTRS_E200	(CPU_FTR_USE_TB | CPU_FTR_SPE_COMP | \
	    CPU_FTR_NODSISRALIGN | CPU_FTR_COHERENT_ICACHE | \
arch/powerpc/include/asm/cputable.h +1 -1
-	    CPU_FTR_UNIFIED_ID_CACHE)
+	    CPU_FTR_UNIFIED_ID_CACHE | CPU_FTR_NOEXECUTE)
#define CPU_FTRS_E500 (CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \
(quoted diff) +1 -1
-	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN)
+	    CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP |
CPU_FTR_NODSISRALIGN \
Added a '|' at the end of the line before the escape

Quoted 3 lines

Right. Will send a new patch tomorrow. Appart from that, have you verified it doesn't have any adverse effects for you ? I did some quick tests on 440 and things seem to be fine.

#ifdef __powerpc64__
#define LONG_ASM_CONST(x) ASM_CONST(x)
#else
#define LONG_ASM_CONST(x) 0
#endif

#define CPU_FTR_NOEXECUTE LONG_ASM_CONST(0x0000000800000000)

Am I not looking at the right code? Since e200 and e500 aren't powerpc64, doesn't adding CPU_FTR_NOEXECUTE have no effect at all?

Benjamin Herrenschmidt

author

2008-12-04 00:08 UTC #4 lore raw

Quoted 10 lines · Trent Piepho

#ifdef __powerpc64__
#define LONG_ASM_CONST(x) ASM_CONST(x)
#else
#define LONG_ASM_CONST(x) 0
#endif
#define CPU_FTR_NOEXECUTE LONG_ASM_CONST(0x0000000800000000)
Am I not looking at the right code? Since e200 and e500 aren't powerpc64, doesn't adding CPU_FTR_NOEXECUTE have no effect at all?

No, you are right, this is a case where I didn't actually verify that the patch had the expected effect :-(

We are running out of low FTR bits, heck, I might make them 64-bit for everybody soon.

I'll fix that up, thanks for spotting it.

Cheers,
Ben.

Steven A. Falco

Acked-by

2008-12-02 15:41 UTC #5 lore raw

In reply to Kumar Gala

Quoted 10 lines · Kumar Gala

Quoted 6 lines · Benjamin Herrenschmidt
We were missing the CPU_FTR_NOEXECUTE bit in our cputable for all these processors. The result is that update_mmu_cache() would flush the cache for all pages mapped to userspace which is totally unnecessary on those processors since we already handle flushing on execute in the page fault path.

Quoted 3 lines

Quoted 3 lines
This should provide a nice speed up ;-)
Signed-off-by Benjamin Herrenschmidt <[email protected]>

I'm running with your patch on a 440 Sequoia board, and it works fine for me.

Acked-by Steven A. Falco <[email protected]>

Steve