[PATCH 0/3] OpenRISC Fixes for jump_label SMP Syncing

[PATCH 0/3] OpenRISC Fixes for jump_label SMP Syncing

[Stafford Horne]

On OpenRISC I noticed our jump_label implementation based on arm64 uses
kick_all_cpus_sync() to force remote cores to pick up static_key updates.  The
call to kick_all_cpus_sync() on OpenRISC does run IPIs on remote cores, but it
does not have any guarantees about instruction cache invalidation.

The jump_label code seems to work fine and passes tests, but I had suspicions
that this was incorrect.  My suspicion was correct, and I was able to reproduce
issues where static_keys on remote cores were left stale after jump_label
updates using a custom test [0].

This series fixes the issues by triggering remote icache invalidation after
jump_label text patching is done.

[0] https://github.com/stffrdhrn/or1k-utils/tree/master/tests/smp_static_key_test

Stafford Horne (3):
  openrisc: Cache invalidation cleanup
  openrisc: Add full instruction cache invalidate functions
  openrisc: Fix jump_label smp syncing

 arch/openrisc/include/asm/cacheflush.h |  4 ++++
 arch/openrisc/kernel/head.S            | 10 ----------
 arch/openrisc/kernel/jump_label.c      |  2 +-
 arch/openrisc/kernel/patching.c        |  3 +++
 arch/openrisc/kernel/smp.c             | 21 +++++++++++++++++++++
 arch/openrisc/mm/cache.c               | 16 ++++++++++++++++
 6 files changed, 45 insertions(+), 11 deletions(-)

-- 
2.53.0

[PATCH 1/3] openrisc: Cache invalidation cleanup

[Stafford Horne]

When working on new cache invalidation functions I noticed these
cleanups in the cache initialization code.  Remove unused and commented
instructions to avoid confusion.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/openrisc/kernel/head.S | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/openrisc/kernel/head.S b/arch/openrisc/kernel/head.S
index bd760066f1cd..65f474c2df4f 100644
--- a/arch/openrisc/kernel/head.S
+++ b/arch/openrisc/kernel/head.S
@@ -852,26 +852,19 @@ _ic_enable:
 	l.sll	r14,r30,r28
 
 	/* Establish number of cache sets
-	   r16 contains number of cache sets
 	   r28 contains log(# of cache sets)
 	*/
 	l.andi  r26,r24,SPR_ICCFGR_NCS
 	l.srli 	r28,r26,3
-	l.ori   r30,r0,1
-	l.sll   r16,r30,r28
 
 	/* Invalidate IC */
 	l.addi  r6,r0,0
 	l.sll   r5,r14,r28
-//        l.mul   r5,r14,r16
-//	l.trap  1
-//	l.addi  r5,r0,IC_SIZE
 1:
 	l.mtspr r0,r6,SPR_ICBIR
 	l.sfne  r6,r5
 	l.bf    1b
 	l.add   r6,r6,r14
- //       l.addi   r6,r6,IC_LINE
 
 	/* Enable IC */
 	l.mfspr r6,r0,SPR_SR
@@ -918,13 +911,10 @@ _dc_enable:
 	l.sll	r14,r30,r28
 
 	/* Establish number of cache sets
-	   r16 contains number of cache sets
 	   r28 contains log(# of cache sets)
 	*/
 	l.andi  r26,r24,SPR_DCCFGR_NCS
 	l.srli 	r28,r26,3
-	l.ori   r30,r0,1
-	l.sll   r16,r30,r28
 
 	/* Invalidate DC */
 	l.addi  r6,r0,0
-- 
2.53.0

[PATCH 2/3] openrisc: Add full instruction cache invalidate functions

[Stafford Horne]

Add functions to invalidate all cache lines which we will use for
static_key patching.

On OpenRISC there is no instruction to invalidate an entire cache so we
loop and invalidate cache lines one by one.  This is not extremely
expensive on OpenRISC as we usually have only a few hundred cache lines.

I considered using the invalidate cache page or range functions.
However, tracking which ranges need invalidation would have been more
expensive than flushing all pages.

Cc: stable@vger.kernel.org
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/openrisc/include/asm/cacheflush.h |  4 ++++
 arch/openrisc/kernel/smp.c             | 21 +++++++++++++++++++++
 arch/openrisc/mm/cache.c               | 16 ++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/arch/openrisc/include/asm/cacheflush.h b/arch/openrisc/include/asm/cacheflush.h
index cd8f971c0fec..7b8c043a831d 100644
--- a/arch/openrisc/include/asm/cacheflush.h
+++ b/arch/openrisc/include/asm/cacheflush.h
@@ -26,6 +26,7 @@ extern void local_icache_page_inv(struct page *page);
 extern void local_dcache_range_flush(unsigned long start, unsigned long end);
 extern void local_dcache_range_inv(unsigned long start, unsigned long end);
 extern void local_icache_range_inv(unsigned long start, unsigned long end);
+extern void local_icache_all_inv(void);
 
 /*
  * Data cache flushing always happen on the local cpu. Instruction cache
@@ -35,10 +36,13 @@ extern void local_icache_range_inv(unsigned long start, unsigned long end);
 #ifndef CONFIG_SMP
 #define dcache_page_flush(page)      local_dcache_page_flush(page)
 #define icache_page_inv(page)        local_icache_page_inv(page)
+#define icache_all_inv()             local_icache_all_inv()
 #else  /* CONFIG_SMP */
 #define dcache_page_flush(page)      local_dcache_page_flush(page)
 #define icache_page_inv(page)        smp_icache_page_inv(page)
+#define icache_all_inv()             smp_icache_all_inv()
 extern void smp_icache_page_inv(struct page *page);
+extern void smp_icache_all_inv(void);
 #endif /* CONFIG_SMP */
 
 /*
diff --git a/arch/openrisc/kernel/smp.c b/arch/openrisc/kernel/smp.c
index 040ca201b692..65599252f3d4 100644
--- a/arch/openrisc/kernel/smp.c
+++ b/arch/openrisc/kernel/smp.c
@@ -346,3 +346,24 @@ void smp_icache_page_inv(struct page *page)
 	on_each_cpu(ipi_icache_page_inv, page, 1);
 }
 EXPORT_SYMBOL(smp_icache_page_inv);
+
+static void ipi_icache_all_inv(void *arg)
+{
+	local_icache_all_inv();
+}
+
+void smp_icache_all_inv(void)
+{
+	if (num_online_cpus() < 2) {
+		local_icache_all_inv();
+		return;
+	}
+
+	/*
+	 * Ensure stores complete before we request remote icaches
+	 * to invalidate.
+	 */
+	mb();
+
+	on_each_cpu(ipi_icache_all_inv, NULL, 1);
+}
diff --git a/arch/openrisc/mm/cache.c b/arch/openrisc/mm/cache.c
index f33df46dae4e..2667d90691b5 100644
--- a/arch/openrisc/mm/cache.c
+++ b/arch/openrisc/mm/cache.c
@@ -63,6 +63,22 @@ void local_icache_page_inv(struct page *page)
 }
 EXPORT_SYMBOL(local_icache_page_inv);
 
+void local_icache_all_inv(void)
+{
+	if (cpu_cache_is_present(SPR_UPR_ICP)) {
+		unsigned long iccfgr = mfspr(SPR_ICCFGR);
+		unsigned long sets = 1 << ((iccfgr & SPR_ICCFGR_NCS) >> 3);
+		unsigned long block_size = 16 << ((iccfgr & SPR_ICCFGR_CBS) >> 7);
+		unsigned long paddr = 0;
+		unsigned long end = sets * block_size;
+
+		while (paddr < end) {
+			mtspr(SPR_ICBIR, paddr);
+			paddr += block_size;
+		}
+	}
+}
+
 void local_dcache_range_flush(unsigned long start, unsigned long end)
 {
 	cache_loop(start, end, SPR_DCBFR, SPR_UPR_DCP);
-- 
2.53.0

[PATCH 3/3] openrisc: Fix jump_label smp syncing

[Stafford Horne]

The original commit 8c30b0018f9d ("openrisc: Add jump label support")
copies from arm64 and does not properly consider how icache invalidation
on remote cores works in OpenRISC.  On OpenRISC remote icaches need to
be invalidated otherwise static key's may remain state after updating.

Fix SMP cache syncing by:

 1. Properly invalidate remote core icaches on SMP systems by using
    icache_all_inv.  The old code uses kick_all_cpus_sync() which runs a
    no-op IPI function call on remote CPU's which does execute a lot of
    code and flushes many cache lines in the process, but does not flush
    all and it's not correct on OpenRISC.
 2. For architectures that do not have WRITETHROUGH caches be sure
    to flush the dcache after patching.

To test this I first reproduced the issue using a custom test module
[0].  The test confirmed that some icache lines maintained stale
static_key code sequences after calling static_branch_enable().  After
this patch there are no longer jump_label coherency issues.

[0] https://github.com/stffrdhrn/or1k-utils/tree/master/tests/smp_static_key_test

Cc: stable@vger.kernel.org # depends on openrisc: Add icache_all_inv
Fixes: 8c30b0018f9d ("openrisc: Add jump label support")
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/openrisc/kernel/jump_label.c | 2 +-
 arch/openrisc/kernel/patching.c   | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/openrisc/kernel/jump_label.c b/arch/openrisc/kernel/jump_label.c
index ab7137c23b46..9cb63f2d2e2b 100644
--- a/arch/openrisc/kernel/jump_label.c
+++ b/arch/openrisc/kernel/jump_label.c
@@ -47,5 +47,5 @@ bool arch_jump_label_transform_queue(struct jump_entry *entry,
 
 void arch_jump_label_transform_apply(void)
 {
-	kick_all_cpus_sync();
+	icache_all_inv();
 }
diff --git a/arch/openrisc/kernel/patching.c b/arch/openrisc/kernel/patching.c
index d186172beb33..5db027b78bc4 100644
--- a/arch/openrisc/kernel/patching.c
+++ b/arch/openrisc/kernel/patching.c
@@ -49,6 +49,9 @@ static int __patch_insn_write(void *addr, u32 insn)
 	waddr = patch_map(addr, FIX_TEXT_POKE0);
 
 	ret = copy_to_kernel_nofault(waddr, &insn, OPENRISC_INSN_SIZE);
+	if (!IS_ENABLED(CONFIG_DCACHE_WRITETHROUGH))
+		local_dcache_range_flush((unsigned long)waddr,
+					 (unsigned long)waddr + OPENRISC_INSN_SIZE);
 	local_icache_range_inv((unsigned long)waddr,
 			       (unsigned long)waddr + OPENRISC_INSN_SIZE);
 
-- 
2.53.0