[PATCH v1] riscv: Fix lost icache flush requests in set_icache_stale_mask()

[PATCH v1] riscv: Fix lost icache flush requests in set_icache_stale_mask()

[Nickolai Zeldovich]

set_icache_stale_mask() marks every other hart's icache as stale for the
current mm by reading the current hart's bit, storing an all-ones mask and
then writing the current hart's bit back as it was read:

	stale_cpu = cpumask_test_cpu(cpu, mask);
	cpumask_setall(mask);
	__assign_cpu(cpu, mask, stale_cpu);

mm->context.icache_stale_mask is shared by all harts running the mm and
is updated concurrently without a lock: flush_icache_mm() sets every bit
with cpumask_setall() (a plain store) whenever a thread flushes the
icache, prepare_sync_core_cmd() does the same for
membarrier(MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE), and
flush_icache_deferred() clears the current hart's bit with an atomic
test-and-clear from switch_mm(). Those are safe with each other because
each of them either only sets bits or atomically clears its own hart's
bit, and a bit set one time too many only costs a redundant fence.i.
set_icache_stale_mask() is the one writer that stores a value it read
earlier: the current hart's bit as returned by cpumask_test_cpu(). If
another hart sets that bit between the read and the write-back, the
write-back erases it. The erased bit is a pending deferred icache flush
for the current hart.

Consider a process with two threads on different harts:

  - Thread T1, on hart C, writes new instructions and calls
    riscv_flush_icache(start, end, SYS_RISCV_FLUSH_ICACHE_LOCAL).
    flush_icache_mm(mm, true) flushes hart C and only sets the other
    harts' stale bits, without IPIs: hart A is to flush its icache the
    next time it switches to this mm.

  - Thread T2, on hart A, calls prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX,
    PR_RISCV_CTX_SW_FENCEI_OFF, PR_RISCV_SCOPE_PER_THREAD) at the same
    time. set_icache_stale_mask() reads hart A's bit as clear, T1's
    cpumask_setall() sets it, and __assign_cpu() clears it again.

Hart A then runs another process, and T1 migrates to hart A. switch_mm()
finds hart A's bit clear and does not flush, so T1 executes the old
instructions still in hart A's icache. For a JIT this is a wrong result
or a crash on a mix of old and new code. The guarantee of
membarrier(SYNC_CORE), which relies on the same bits, is lost the same
way. The window is the handful of instructions between the load and the
store, but a thread that calls the prctl while another thread of the
process flushes its icache hits it often, see the numbers below.

Set the other harts' bits one at a time with cpumask_set_cpu(), which
only ever sets a bit, and leave the current hart's bit alone: it already
holds the value the old code tried to preserve. Commit 6b9391b581fd
("riscv: Include riscv_set_icache_flush_ctx prctl") kept the current
hart's bit to cope with concurrent callers of this function; that is
preserved. Commit e5bf9a4b68fa ("riscv: switch set_icache_stale_mask()
to using non-atomic assign_cpu()") noted that the sequence is not atomic
as a whole; it does not need to be, it only must never clear a bit.

Reproduced on a StarFive JH7110 (VisionFive 2, non-coherent icache) with
a test program that runs the sequence above in a loop: T1 executes the
code on hart A to fill its icache, moves to hart C, rewrites the code and
flushes with SYS_RISCV_FLUSH_ICACHE_LOCAL while T2 hammers the prctl on
hart A, lets another process run on hart A, moves back to hart A and
executes the code again. A result computed from the old code is a stale
fetch. Each line is 50000 iterations on a pair of harts (A, C):

  v7.3-rc7 unpatched:               13177 and 14362 stale
  v7.3-rc7 + 100 us delay in the
    read-modify-write window:       49993 and 47675 stale
  v7.3-rc7 + this patch:                0 and     0 stale
  v7.3-rc7 + this patch + delay:        0 and     0 stale

Control runs with T2 idle never read stale code on any kernel, so the
deferred flush itself works; only its request gets lost.

Found while formally verifying set_icache_stale_mask() against the
kernel binary.

Fixes: 6b9391b581fd ("riscv: Include riscv_set_icache_flush_ctx prctl")
Assisted-by: LLM
Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
---

Notes:
    The reproducer used for the numbers in the commit message is below (nolibc,
    build with "make headers_install" done:
    clang --target=riscv64-linux-gnu -Os -static -nostdlib -nostdinc
      -I tools/include/nolibc -I usr/include -include nolibc.h
      -o icache_stale_mask icache_stale_mask.c
    and run as "icache_stale_mask <hartA> <hartC> 50000" for the race and with a
    fifth argument for the control run). It needs hardware with a non-coherent
    icache; QEMU TCG keeps translated code coherent and never shows the symptom.
    
    Build checks: riscv defconfig Image with clang, W=1 on cacheflush.o, and
    allnoconfig (CONFIG_SMP=n, the function is not compiled), all clean.
    checkpatch --strict reports only the missing Signed-off-by.
    
    /*
     * icache_stale_mask.c: a thread's deferred icache flush is lost when
     * prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX, PR_RISCV_CTX_SW_FENCEI_OFF, ...) on
     * one hart races with riscv_flush_icache(SYS_RISCV_FLUSH_ICACHE_LOCAL) on
     * another hart of the same process.
     *
     * Build with the kernel's nolibc (after "make headers_install"):
     *   clang --target=riscv64-linux-gnu -Os -static -nostdlib -nostdinc \
     *     -I tools/include/nolibc -I usr/include -include nolibc.h \
     *     -o icache_stale_mask icache_stale_mask.c
     * Usage: icache_stale_mask <hartA> <hartC> [iterations] [control]
     *
     * Three actors:
     *   T1 (the main thread): writes new code at P from hart C and calls
     *      riscv_flush_icache(P, P + 4096, SYS_RISCV_FLUSH_ICACHE_LOCAL), which
     *      flushes hart C and marks every other hart's icache stale for this mm.
     *   T2 (a thread of the same process, pinned to hart A): calls the prctl in
     *      a loop while T1 flushes.  Its set_icache_stale_mask() reads hart A's
     *      stale bit (clear), then stores the whole mask back with the bit as
     *      read, erasing the bit T1 set in between.
     *   Q  (another process, pinned to hart A): runs on hart A after T2 blocks,
     *      so that T1's arrival on hart A is a switch_mm() into the process,
     *      the point where the kernel performs the deferred flush iff the bit
     *      is set.
     * Each iteration: T1 runs the old code on hart A (hart A's icache now holds
     * it), moves to hart C, writes new code, flushes LOCAL while T2 hammers the
     * prctl, lets Q run on hart A, moves back to hart A and runs the code.  A
     * result computed from the old code is a stale fetch.  With "control" T2
     * never runs: every iteration must then read the new code.
     */
    #define N 1000	/* li a0,0; N x addi a0,a0,v; ret: returns N*v */
    #define PAGE 4096
    
    #ifndef SYS_RISCV_FLUSH_ICACHE_LOCAL
    #define SYS_RISCV_FLUSH_ICACHE_LOCAL 1
    #endif
    #ifndef __NR_riscv_flush_icache
    #define __NR_riscv_flush_icache 259
    #endif
    #ifndef PR_RISCV_SET_ICACHE_FLUSH_CTX
    #define PR_RISCV_SET_ICACHE_FLUSH_CTX 71
    #define PR_RISCV_CTX_SW_FENCEI_ON 0
    #define PR_RISCV_CTX_SW_FENCEI_OFF 1
    #define PR_RISCV_SCOPE_PER_PROCESS 0
    #define PR_RISCV_SCOPE_PER_THREAD 1
    #endif
    
    static unsigned int *rw;	/* the code page, writable mapping */
    static unsigned int *rx;	/* the same page, executable mapping */
    static volatile int go;		/* T2 hammers the prctl while set */
    static int t1_to_t2[2], t2_to_t1[2], t1_to_q[2], q_to_t1[2];
    static int hart_a, hart_c;
    static char t2_stack[65536] __attribute__((aligned(16)));
    
    /* clone a thread (shared mm) running fn(arg) on the given stack */
    long __clone_thread(long flags, void *stack_top, int (*fn)(void *), void *arg);
    asm(".text\n"
        ".globl __clone_thread\n"
        "__clone_thread:\n"
        "	addi	a1, a1, -16\n"
        "	sd	a2, 0(a1)\n"		/* fn  */
        "	sd	a3, 8(a1)\n"		/* arg */
        "	li	a2, 0\n"
        "	li	a3, 0\n"
        "	li	a4, 0\n"
        "	li	a7, 220\n"		/* __NR_clone */
        "	ecall\n"
        "	bnez	a0, 1f\n"
        "	ld	a2, 0(sp)\n"
        "	ld	a0, 8(sp)\n"
        "	jalr	a2\n"
        "	li	a7, 93\n"		/* __NR_exit */
        "	ecall\n"
        "1:	ret\n");
    
    static void pin(int cpu)
    {
    	unsigned long mask = 1UL << cpu;
    
    	if (syscall(__NR_sched_setaffinity, 0, sizeof(mask), &mask))
    		printf("sched_setaffinity(%d) failed\n", cpu);
    }
    
    static void write_text(unsigned int v)
    {
    	int i;
    
    	rw[0] = 0x00000513;
    	for (i = 1; i <= N; i++)
    		rw[i] = 0x00050513 | (v << 20);
    	rw[N + 1] = 0x00008067;
    }
    
    static long run_text(void)
    {
    	return ((long (*)(void))rx)();
    }
    
    static int t2_main(void *arg)
    {
    	char c;
    	long r;
    
    	pin(hart_a);
    	while (read(t1_to_t2[0], &c, 1) == 1) {
    		write(t2_to_t1[1], "", 1);	/* about to hammer the prctl */
    		while (go) {
    			r = syscall(__NR_prctl, PR_RISCV_SET_ICACHE_FLUSH_CTX,
    				    PR_RISCV_CTX_SW_FENCEI_OFF,
    				    PR_RISCV_SCOPE_PER_THREAD, 0, 0);
    			if (r) {
    				printf("prctl failed: %ld\n", r);
    				break;
    			}
    		}
    		write(t2_to_t1[1], "", 1);
    	}
    	return 0;
    }
    
    int main(int argc, char **argv)
    {
    	int iters = argc > 3 ? atoi(argv[3]) : 1000;
    	int control = argc > 4;
    	int i, fd, stale = 0, old_only = 0, bad = 0;
    	long got;
    	char c;
    
    	if (argc < 3) {
    		printf("usage: %s <hartA> <hartC> [iterations] [control]\n", argv[0]);
    		return 2;
    	}
    	hart_a = atoi(argv[1]);
    	hart_c = atoi(argv[2]);
    
    	fd = memfd_create("text", 0);
    	ftruncate(fd, PAGE);
    	rw = mmap(NULL, PAGE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
    	rx = mmap(NULL, PAGE, PROT_READ | PROT_EXEC, MAP_SHARED, fd, 0);
    	pipe(t1_to_t2);
    	pipe(t2_to_t1);
    	pipe(t1_to_q);
    	pipe(q_to_t1);
    
    	if (!fork()) {			/* Q: another mm, pinned to hart A */
    		close(t1_to_q[1]);
    		close(t1_to_t2[1]);
    		pin(hart_a);
    		while (read(t1_to_q[0], &c, 1) == 1)
    			write(q_to_t1[1], "", 1);
    		_exit(0);
    	}
    	__clone_thread(CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND |
    		       CLONE_THREAD | CLONE_SYSVSEM,
    		       t2_stack + sizeof(t2_stack), t2_main, NULL);
    
    	for (i = 0; i < iters; i++) {
    		unsigned int old = 1 + (i & 1), new = 2 - (i & 1);
    
    		pin(hart_a);
    		write_text(old);
    		syscall(__NR_riscv_flush_icache, 0, 0, 0);	/* all harts */
    		got = run_text();		/* hart A caches the old code */
    		if (got != (long)N * old) {
    			bad++;
    			continue;
    		}
    
    		pin(hart_c);
    		if (!control) {
    			go = 1;
    			write(t1_to_t2[1], "", 1);	/* T2 hammers the prctl on A */
    			read(t2_to_t1[0], &c, 1);	/* ... from now on */
    		}
    		write_text(new);
    		/* flush hart C now, mark hart A's icache stale for this mm */
    		syscall(__NR_riscv_flush_icache, rx, (char *)rx + PAGE,
    			SYS_RISCV_FLUSH_ICACHE_LOCAL);
    		if (!control) {
    			go = 0;
    			read(t2_to_t1[0], &c, 1);	/* T2 stops and blocks */
    		}
    		write(t1_to_q[1], "", 1);	/* Q runs on hart A: A leaves our mm */
    		read(q_to_t1[0], &c, 1);
    
    		pin(hart_a);			/* switch_mm into our mm on hart A */
    		got = run_text();
    		if (got != (long)N * new) {
    			stale++;
    			if (got == (long)N * old)
    				old_only++;
    		}
    	}
    	close(t1_to_t2[1]);
    	close(t1_to_q[1]);
    	wait(NULL);
    	printf("harts A=%d C=%d %s: %d of %d iterations ran stale code (%d entirely old, %d warm-up failures)\n",
    	       hart_a, hart_c, control ? "control" : "race", stale, iters,
    	       old_only, bad);
    	return !!stale;
    }

 arch/riscv/mm/cacheflush.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
index f8ead7cb7c7d6..22d6520a05417 100644
--- a/arch/riscv/mm/cacheflush.c
+++ b/arch/riscv/mm/cacheflush.c
@@ -180,20 +180,19 @@ void __init riscv_init_cbo_blocksizes(void)
 static void set_icache_stale_mask(void)
 {
 	int cpu = get_cpu();
+	unsigned int other;
 	cpumask_t *mask;
-	bool stale_cpu;
 
 	/*
-	 * Mark every other hart's icache as needing a flush for
-	 * this MM. Maintain the previous value of the current
-	 * cpu to handle the case when this function is called
-	 * concurrently on different harts.
+	 * Mark every other hart's icache as needing a flush for this MM.
+	 * Set the bits one at a time: flush_icache_mm() and
+	 * flush_icache_deferred() update this mask concurrently, and a
+	 * read-modify-write of the whole mask would discard their updates.
 	 */
 	mask = &current->mm->context.icache_stale_mask;
-	stale_cpu = cpumask_test_cpu(cpu, mask);
-
-	cpumask_setall(mask);
-	__assign_cpu(cpu, mask, stale_cpu);
+	for_each_possible_cpu(other)
+		if (other != cpu)
+			cpumask_set_cpu(other, mask);
 	put_cpu();
 }
 #endif

base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
-- 
2.56.0

Re: [PATCH v1] riscv: Fix lost icache flush requests in set_icache_stale_mask()

[Yury Norov]

On Tue, Oct 13, 2026 at 01:12:37PM -0400, Nickolai Zeldovich wrote:
> set_icache_stale_mask() marks every other hart's icache as stale for the
> current mm by reading the current hart's bit, storing an all-ones mask and
> then writing the current hart's bit back as it was read:
> 
> 	stale_cpu = cpumask_test_cpu(cpu, mask);
> 	cpumask_setall(mask);
> 	__assign_cpu(cpu, mask, stale_cpu);
> 
> mm->context.icache_stale_mask is shared by all harts running the mm and
> is updated concurrently without a lock: flush_icache_mm() sets every bit
> with cpumask_setall() (a plain store) whenever a thread flushes the
> icache, prepare_sync_core_cmd() does the same for
> membarrier(MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE), and
> flush_icache_deferred() clears the current hart's bit with an atomic
> test-and-clear from switch_mm(). Those are safe with each other because
> each of them either only sets bits or atomically clears its own hart's
> bit, and a bit set one time too many only costs a redundant fence.i.
> set_icache_stale_mask() is the one writer that stores a value it read
> earlier: the current hart's bit as returned by cpumask_test_cpu(). If
> another hart sets that bit between the read and the write-back, the
> write-back erases it. The erased bit is a pending deferred icache flush
> for the current hart.
> 
> Consider a process with two threads on different harts:
> 
>   - Thread T1, on hart C, writes new instructions and calls
>     riscv_flush_icache(start, end, SYS_RISCV_FLUSH_ICACHE_LOCAL).
>     flush_icache_mm(mm, true) flushes hart C and only sets the other
>     harts' stale bits, without IPIs: hart A is to flush its icache the
>     next time it switches to this mm.
> 
>   - Thread T2, on hart A, calls prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX,
>     PR_RISCV_CTX_SW_FENCEI_OFF, PR_RISCV_SCOPE_PER_THREAD) at the same
>     time. set_icache_stale_mask() reads hart A's bit as clear, T1's
>     cpumask_setall() sets it, and __assign_cpu() clears it again.
> 
> Hart A then runs another process, and T1 migrates to hart A. switch_mm()
> finds hart A's bit clear and does not flush, so T1 executes the old
> instructions still in hart A's icache. For a JIT this is a wrong result
> or a crash on a mix of old and new code. The guarantee of
> membarrier(SYNC_CORE), which relies on the same bits, is lost the same
> way. The window is the handful of instructions between the load and the
> store, but a thread that calls the prctl while another thread of the
> process flushes its icache hits it often, see the numbers below.
> 
> Set the other harts' bits one at a time with cpumask_set_cpu(), which
> only ever sets a bit, and leave the current hart's bit alone: it already
> holds the value the old code tried to preserve. Commit 6b9391b581fd
> ("riscv: Include riscv_set_icache_flush_ctx prctl") kept the current
> hart's bit to cope with concurrent callers of this function; that is
> preserved. Commit e5bf9a4b68fa ("riscv: switch set_icache_stale_mask()
> to using non-atomic assign_cpu()") noted that the sequence is not atomic
> as a whole; it does not need to be, it only must never clear a bit.
> 
> Reproduced on a StarFive JH7110 (VisionFive 2, non-coherent icache) with
> a test program that runs the sequence above in a loop: T1 executes the
> code on hart A to fill its icache, moves to hart C, rewrites the code and
> flushes with SYS_RISCV_FLUSH_ICACHE_LOCAL while T2 hammers the prctl on
> hart A, lets another process run on hart A, moves back to hart A and
> executes the code again. A result computed from the old code is a stale
> fetch. Each line is 50000 iterations on a pair of harts (A, C):
> 
>   v7.3-rc7 unpatched:               13177 and 14362 stale
>   v7.3-rc7 + 100 us delay in the
>     read-modify-write window:       49993 and 47675 stale
>   v7.3-rc7 + this patch:                0 and     0 stale
>   v7.3-rc7 + this patch + delay:        0 and     0 stale
> 
> Control runs with T2 idle never read stale code on any kernel, so the
> deferred flush itself works; only its request gets lost.
> 
> Found while formally verifying set_icache_stale_mask() against the
> kernel binary.
> 
> Fixes: 6b9391b581fd ("riscv: Include riscv_set_icache_flush_ctx prctl")
> Assisted-by: LLM
> Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
> ---
> 
> Notes:
>     The reproducer used for the numbers in the commit message is below (nolibc,
>     build with "make headers_install" done:
>     clang --target=riscv64-linux-gnu -Os -static -nostdlib -nostdinc
>       -I tools/include/nolibc -I usr/include -include nolibc.h
>       -o icache_stale_mask icache_stale_mask.c
>     and run as "icache_stale_mask <hartA> <hartC> 50000" for the race and with a
>     fifth argument for the control run). It needs hardware with a non-coherent
>     icache; QEMU TCG keeps translated code coherent and never shows the symptom.
>     
>     Build checks: riscv defconfig Image with clang, W=1 on cacheflush.o, and
>     allnoconfig (CONFIG_SMP=n, the function is not compiled), all clean.
>     checkpatch --strict reports only the missing Signed-off-by.
>     
>     /*
>      * icache_stale_mask.c: a thread's deferred icache flush is lost when
>      * prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX, PR_RISCV_CTX_SW_FENCEI_OFF, ...) on
>      * one hart races with riscv_flush_icache(SYS_RISCV_FLUSH_ICACHE_LOCAL) on
>      * another hart of the same process.
>      *
>      * Build with the kernel's nolibc (after "make headers_install"):
>      *   clang --target=riscv64-linux-gnu -Os -static -nostdlib -nostdinc \
>      *     -I tools/include/nolibc -I usr/include -include nolibc.h \
>      *     -o icache_stale_mask icache_stale_mask.c
>      * Usage: icache_stale_mask <hartA> <hartC> [iterations] [control]
>      *
>      * Three actors:
>      *   T1 (the main thread): writes new code at P from hart C and calls
>      *      riscv_flush_icache(P, P + 4096, SYS_RISCV_FLUSH_ICACHE_LOCAL), which
>      *      flushes hart C and marks every other hart's icache stale for this mm.
>      *   T2 (a thread of the same process, pinned to hart A): calls the prctl in
>      *      a loop while T1 flushes.  Its set_icache_stale_mask() reads hart A's
>      *      stale bit (clear), then stores the whole mask back with the bit as
>      *      read, erasing the bit T1 set in between.
>      *   Q  (another process, pinned to hart A): runs on hart A after T2 blocks,
>      *      so that T1's arrival on hart A is a switch_mm() into the process,
>      *      the point where the kernel performs the deferred flush iff the bit
>      *      is set.
>      * Each iteration: T1 runs the old code on hart A (hart A's icache now holds
>      * it), moves to hart C, writes new code, flushes LOCAL while T2 hammers the
>      * prctl, lets Q run on hart A, moves back to hart A and runs the code.  A
>      * result computed from the old code is a stale fetch.  With "control" T2
>      * never runs: every iteration must then read the new code.
>      */
>     #define N 1000	/* li a0,0; N x addi a0,a0,v; ret: returns N*v */
>     #define PAGE 4096
>     
>     #ifndef SYS_RISCV_FLUSH_ICACHE_LOCAL
>     #define SYS_RISCV_FLUSH_ICACHE_LOCAL 1
>     #endif
>     #ifndef __NR_riscv_flush_icache
>     #define __NR_riscv_flush_icache 259
>     #endif
>     #ifndef PR_RISCV_SET_ICACHE_FLUSH_CTX
>     #define PR_RISCV_SET_ICACHE_FLUSH_CTX 71
>     #define PR_RISCV_CTX_SW_FENCEI_ON 0
>     #define PR_RISCV_CTX_SW_FENCEI_OFF 1
>     #define PR_RISCV_SCOPE_PER_PROCESS 0
>     #define PR_RISCV_SCOPE_PER_THREAD 1
>     #endif
>     
>     static unsigned int *rw;	/* the code page, writable mapping */
>     static unsigned int *rx;	/* the same page, executable mapping */
>     static volatile int go;		/* T2 hammers the prctl while set */
>     static int t1_to_t2[2], t2_to_t1[2], t1_to_q[2], q_to_t1[2];
>     static int hart_a, hart_c;
>     static char t2_stack[65536] __attribute__((aligned(16)));
>     
>     /* clone a thread (shared mm) running fn(arg) on the given stack */
>     long __clone_thread(long flags, void *stack_top, int (*fn)(void *), void *arg);
>     asm(".text\n"
>         ".globl __clone_thread\n"
>         "__clone_thread:\n"
>         "	addi	a1, a1, -16\n"
>         "	sd	a2, 0(a1)\n"		/* fn  */
>         "	sd	a3, 8(a1)\n"		/* arg */
>         "	li	a2, 0\n"
>         "	li	a3, 0\n"
>         "	li	a4, 0\n"
>         "	li	a7, 220\n"		/* __NR_clone */
>         "	ecall\n"
>         "	bnez	a0, 1f\n"
>         "	ld	a2, 0(sp)\n"
>         "	ld	a0, 8(sp)\n"
>         "	jalr	a2\n"
>         "	li	a7, 93\n"		/* __NR_exit */
>         "	ecall\n"
>         "1:	ret\n");
>     
>     static void pin(int cpu)
>     {
>     	unsigned long mask = 1UL << cpu;
>     
>     	if (syscall(__NR_sched_setaffinity, 0, sizeof(mask), &mask))
>     		printf("sched_setaffinity(%d) failed\n", cpu);
>     }
>     
>     static void write_text(unsigned int v)
>     {
>     	int i;
>     
>     	rw[0] = 0x00000513;
>     	for (i = 1; i <= N; i++)
>     		rw[i] = 0x00050513 | (v << 20);
>     	rw[N + 1] = 0x00008067;
>     }
>     
>     static long run_text(void)
>     {
>     	return ((long (*)(void))rx)();
>     }
>     
>     static int t2_main(void *arg)
>     {
>     	char c;
>     	long r;
>     
>     	pin(hart_a);
>     	while (read(t1_to_t2[0], &c, 1) == 1) {
>     		write(t2_to_t1[1], "", 1);	/* about to hammer the prctl */
>     		while (go) {
>     			r = syscall(__NR_prctl, PR_RISCV_SET_ICACHE_FLUSH_CTX,
>     				    PR_RISCV_CTX_SW_FENCEI_OFF,
>     				    PR_RISCV_SCOPE_PER_THREAD, 0, 0);
>     			if (r) {
>     				printf("prctl failed: %ld\n", r);
>     				break;
>     			}
>     		}
>     		write(t2_to_t1[1], "", 1);
>     	}
>     	return 0;
>     }
>     
>     int main(int argc, char **argv)
>     {
>     	int iters = argc > 3 ? atoi(argv[3]) : 1000;
>     	int control = argc > 4;
>     	int i, fd, stale = 0, old_only = 0, bad = 0;
>     	long got;
>     	char c;
>     
>     	if (argc < 3) {
>     		printf("usage: %s <hartA> <hartC> [iterations] [control]\n", argv[0]);
>     		return 2;
>     	}
>     	hart_a = atoi(argv[1]);
>     	hart_c = atoi(argv[2]);
>     
>     	fd = memfd_create("text", 0);
>     	ftruncate(fd, PAGE);
>     	rw = mmap(NULL, PAGE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>     	rx = mmap(NULL, PAGE, PROT_READ | PROT_EXEC, MAP_SHARED, fd, 0);
>     	pipe(t1_to_t2);
>     	pipe(t2_to_t1);
>     	pipe(t1_to_q);
>     	pipe(q_to_t1);
>     
>     	if (!fork()) {			/* Q: another mm, pinned to hart A */
>     		close(t1_to_q[1]);
>     		close(t1_to_t2[1]);
>     		pin(hart_a);
>     		while (read(t1_to_q[0], &c, 1) == 1)
>     			write(q_to_t1[1], "", 1);
>     		_exit(0);
>     	}
>     	__clone_thread(CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND |
>     		       CLONE_THREAD | CLONE_SYSVSEM,
>     		       t2_stack + sizeof(t2_stack), t2_main, NULL);
>     
>     	for (i = 0; i < iters; i++) {
>     		unsigned int old = 1 + (i & 1), new = 2 - (i & 1);
>     
>     		pin(hart_a);
>     		write_text(old);
>     		syscall(__NR_riscv_flush_icache, 0, 0, 0);	/* all harts */
>     		got = run_text();		/* hart A caches the old code */
>     		if (got != (long)N * old) {
>     			bad++;
>     			continue;
>     		}
>     
>     		pin(hart_c);
>     		if (!control) {
>     			go = 1;
>     			write(t1_to_t2[1], "", 1);	/* T2 hammers the prctl on A */
>     			read(t2_to_t1[0], &c, 1);	/* ... from now on */
>     		}
>     		write_text(new);
>     		/* flush hart C now, mark hart A's icache stale for this mm */
>     		syscall(__NR_riscv_flush_icache, rx, (char *)rx + PAGE,
>     			SYS_RISCV_FLUSH_ICACHE_LOCAL);
>     		if (!control) {
>     			go = 0;
>     			read(t2_to_t1[0], &c, 1);	/* T2 stops and blocks */
>     		}
>     		write(t1_to_q[1], "", 1);	/* Q runs on hart A: A leaves our mm */
>     		read(q_to_t1[0], &c, 1);
>     
>     		pin(hart_a);			/* switch_mm into our mm on hart A */
>     		got = run_text();
>     		if (got != (long)N * new) {
>     			stale++;
>     			if (got == (long)N * old)
>     				old_only++;
>     		}
>     	}
>     	close(t1_to_t2[1]);
>     	close(t1_to_q[1]);
>     	wait(NULL);
>     	printf("harts A=%d C=%d %s: %d of %d iterations ran stale code (%d entirely old, %d warm-up failures)\n",
>     	       hart_a, hart_c, control ? "control" : "race", stale, iters,
>     	       old_only, bad);
>     	return !!stale;
>     }
> 
>  arch/riscv/mm/cacheflush.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index f8ead7cb7c7d6..22d6520a05417 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -180,20 +180,19 @@ void __init riscv_init_cbo_blocksizes(void)
>  static void set_icache_stale_mask(void)
>  {
>  	int cpu = get_cpu();
> +	unsigned int other;
>  	cpumask_t *mask;
> -	bool stale_cpu;
>  
>  	/*
> -	 * Mark every other hart's icache as needing a flush for
> -	 * this MM. Maintain the previous value of the current
> -	 * cpu to handle the case when this function is called
> -	 * concurrently on different harts.
> +	 * Mark every other hart's icache as needing a flush for this MM.
> +	 * Set the bits one at a time: flush_icache_mm() and
> +	 * flush_icache_deferred() update this mask concurrently, and a
> +	 * read-modify-write of the whole mask would discard their updates.
>  	 */
>  	mask = &current->mm->context.icache_stale_mask;
> -	stale_cpu = cpumask_test_cpu(cpu, mask);
> -
> -	cpumask_setall(mask);
> -	__assign_cpu(cpu, mask, stale_cpu);
> +	for_each_possible_cpu(other)
> +		if (other != cpu)
> +			cpumask_set_cpu(other, mask);

Not judging the motivation, but the implementation should avoid the
for-loop:

        
	cpumask_setall(mask);
        __cpumask_clear_cpu(cpu);

Or I misunderstood your intention?

Thanks,
Yury

>  	put_cpu();
>  }
>  #endif
> 
> base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
> -- 
> 2.56.0

Re: [PATCH v1] riscv: Fix lost icache flush requests in set_icache_stale_mask()

[Nickolai Zeldovich]

On Wed, Oct 28, 2026 at 11:43 AM Yury Norov <ynorov@nvidia.com> wrote:
>
> On Tue, Oct 13, 2026 at 01:12:37PM -0400, Nickolai Zeldovich wrote:
> > set_icache_stale_mask() marks every other hart's icache as stale for the
> > current mm by reading the current hart's bit, storing an all-ones mask and
> > then writing the current hart's bit back as it was read:
> >
> >       stale_cpu = cpumask_test_cpu(cpu, mask);
> >       cpumask_setall(mask);
> >       __assign_cpu(cpu, mask, stale_cpu);
> >
> > mm->context.icache_stale_mask is shared by all harts running the mm and
> > is updated concurrently without a lock: flush_icache_mm() sets every bit
> > with cpumask_setall() (a plain store) whenever a thread flushes the
> > icache, prepare_sync_core_cmd() does the same for
> > membarrier(MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE), and
> > flush_icache_deferred() clears the current hart's bit with an atomic
> > test-and-clear from switch_mm(). Those are safe with each other because
> > each of them either only sets bits or atomically clears its own hart's
> > bit, and a bit set one time too many only costs a redundant fence.i.
> > set_icache_stale_mask() is the one writer that stores a value it read
> > earlier: the current hart's bit as returned by cpumask_test_cpu(). If
> > another hart sets that bit between the read and the write-back, the
> > write-back erases it. The erased bit is a pending deferred icache flush
> > for the current hart.
> >
> > Consider a process with two threads on different harts:
> >
> >   - Thread T1, on hart C, writes new instructions and calls
> >     riscv_flush_icache(start, end, SYS_RISCV_FLUSH_ICACHE_LOCAL).
> >     flush_icache_mm(mm, true) flushes hart C and only sets the other
> >     harts' stale bits, without IPIs: hart A is to flush its icache the
> >     next time it switches to this mm.
> >
> >   - Thread T2, on hart A, calls prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX,
> >     PR_RISCV_CTX_SW_FENCEI_OFF, PR_RISCV_SCOPE_PER_THREAD) at the same
> >     time. set_icache_stale_mask() reads hart A's bit as clear, T1's
> >     cpumask_setall() sets it, and __assign_cpu() clears it again.
> >
> > Hart A then runs another process, and T1 migrates to hart A. switch_mm()
> > finds hart A's bit clear and does not flush, so T1 executes the old
> > instructions still in hart A's icache. For a JIT this is a wrong result
> > or a crash on a mix of old and new code. The guarantee of
> > membarrier(SYNC_CORE), which relies on the same bits, is lost the same
> > way. The window is the handful of instructions between the load and the
> > store, but a thread that calls the prctl while another thread of the
> > process flushes its icache hits it often, see the numbers below.
> >
> > Set the other harts' bits one at a time with cpumask_set_cpu(), which
> > only ever sets a bit, and leave the current hart's bit alone: it already
> > holds the value the old code tried to preserve. Commit 6b9391b581fd
> > ("riscv: Include riscv_set_icache_flush_ctx prctl") kept the current
> > hart's bit to cope with concurrent callers of this function; that is
> > preserved. Commit e5bf9a4b68fa ("riscv: switch set_icache_stale_mask()
> > to using non-atomic assign_cpu()") noted that the sequence is not atomic
> > as a whole; it does not need to be, it only must never clear a bit.
> >
> > Reproduced on a StarFive JH7110 (VisionFive 2, non-coherent icache) with
> > a test program that runs the sequence above in a loop: T1 executes the
> > code on hart A to fill its icache, moves to hart C, rewrites the code and
> > flushes with SYS_RISCV_FLUSH_ICACHE_LOCAL while T2 hammers the prctl on
> > hart A, lets another process run on hart A, moves back to hart A and
> > executes the code again. A result computed from the old code is a stale
> > fetch. Each line is 50000 iterations on a pair of harts (A, C):
> >
> >   v7.3-rc7 unpatched:               13177 and 14362 stale
> >   v7.3-rc7 + 100 us delay in the
> >     read-modify-write window:       49993 and 47675 stale
> >   v7.3-rc7 + this patch:                0 and     0 stale
> >   v7.3-rc7 + this patch + delay:        0 and     0 stale
> >
> > Control runs with T2 idle never read stale code on any kernel, so the
> > deferred flush itself works; only its request gets lost.
> >
> > Found while formally verifying set_icache_stale_mask() against the
> > kernel binary.
> >
> > Fixes: 6b9391b581fd ("riscv: Include riscv_set_icache_flush_ctx prctl")
> > Assisted-by: LLM
> > Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
> > ---
> >
> > Notes:
> >     The reproducer used for the numbers in the commit message is below (nolibc,
> >     build with "make headers_install" done:
> >     clang --target=riscv64-linux-gnu -Os -static -nostdlib -nostdinc
> >       -I tools/include/nolibc -I usr/include -include nolibc.h
> >       -o icache_stale_mask icache_stale_mask.c
> >     and run as "icache_stale_mask <hartA> <hartC> 50000" for the race and with a
> >     fifth argument for the control run). It needs hardware with a non-coherent
> >     icache; QEMU TCG keeps translated code coherent and never shows the symptom.
> >
> >     Build checks: riscv defconfig Image with clang, W=1 on cacheflush.o, and
> >     allnoconfig (CONFIG_SMP=n, the function is not compiled), all clean.
> >     checkpatch --strict reports only the missing Signed-off-by.
> >
> >     /*
> >      * icache_stale_mask.c: a thread's deferred icache flush is lost when
> >      * prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX, PR_RISCV_CTX_SW_FENCEI_OFF, ...) on
> >      * one hart races with riscv_flush_icache(SYS_RISCV_FLUSH_ICACHE_LOCAL) on
> >      * another hart of the same process.
> >      *
> >      * Build with the kernel's nolibc (after "make headers_install"):
> >      *   clang --target=riscv64-linux-gnu -Os -static -nostdlib -nostdinc \
> >      *     -I tools/include/nolibc -I usr/include -include nolibc.h \
> >      *     -o icache_stale_mask icache_stale_mask.c
> >      * Usage: icache_stale_mask <hartA> <hartC> [iterations] [control]
> >      *
> >      * Three actors:
> >      *   T1 (the main thread): writes new code at P from hart C and calls
> >      *      riscv_flush_icache(P, P + 4096, SYS_RISCV_FLUSH_ICACHE_LOCAL), which
> >      *      flushes hart C and marks every other hart's icache stale for this mm.
> >      *   T2 (a thread of the same process, pinned to hart A): calls the prctl in
> >      *      a loop while T1 flushes.  Its set_icache_stale_mask() reads hart A's
> >      *      stale bit (clear), then stores the whole mask back with the bit as
> >      *      read, erasing the bit T1 set in between.
> >      *   Q  (another process, pinned to hart A): runs on hart A after T2 blocks,
> >      *      so that T1's arrival on hart A is a switch_mm() into the process,
> >      *      the point where the kernel performs the deferred flush iff the bit
> >      *      is set.
> >      * Each iteration: T1 runs the old code on hart A (hart A's icache now holds
> >      * it), moves to hart C, writes new code, flushes LOCAL while T2 hammers the
> >      * prctl, lets Q run on hart A, moves back to hart A and runs the code.  A
> >      * result computed from the old code is a stale fetch.  With "control" T2
> >      * never runs: every iteration must then read the new code.
> >      */
> >     #define N 1000    /* li a0,0; N x addi a0,a0,v; ret: returns N*v */
> >     #define PAGE 4096
> >
> >     #ifndef SYS_RISCV_FLUSH_ICACHE_LOCAL
> >     #define SYS_RISCV_FLUSH_ICACHE_LOCAL 1
> >     #endif
> >     #ifndef __NR_riscv_flush_icache
> >     #define __NR_riscv_flush_icache 259
> >     #endif
> >     #ifndef PR_RISCV_SET_ICACHE_FLUSH_CTX
> >     #define PR_RISCV_SET_ICACHE_FLUSH_CTX 71
> >     #define PR_RISCV_CTX_SW_FENCEI_ON 0
> >     #define PR_RISCV_CTX_SW_FENCEI_OFF 1
> >     #define PR_RISCV_SCOPE_PER_PROCESS 0
> >     #define PR_RISCV_SCOPE_PER_THREAD 1
> >     #endif
> >
> >     static unsigned int *rw;  /* the code page, writable mapping */
> >     static unsigned int *rx;  /* the same page, executable mapping */
> >     static volatile int go;           /* T2 hammers the prctl while set */
> >     static int t1_to_t2[2], t2_to_t1[2], t1_to_q[2], q_to_t1[2];
> >     static int hart_a, hart_c;
> >     static char t2_stack[65536] __attribute__((aligned(16)));
> >
> >     /* clone a thread (shared mm) running fn(arg) on the given stack */
> >     long __clone_thread(long flags, void *stack_top, int (*fn)(void *), void *arg);
> >     asm(".text\n"
> >         ".globl __clone_thread\n"
> >         "__clone_thread:\n"
> >         "     addi    a1, a1, -16\n"
> >         "     sd      a2, 0(a1)\n"            /* fn  */
> >         "     sd      a3, 8(a1)\n"            /* arg */
> >         "     li      a2, 0\n"
> >         "     li      a3, 0\n"
> >         "     li      a4, 0\n"
> >         "     li      a7, 220\n"              /* __NR_clone */
> >         "     ecall\n"
> >         "     bnez    a0, 1f\n"
> >         "     ld      a2, 0(sp)\n"
> >         "     ld      a0, 8(sp)\n"
> >         "     jalr    a2\n"
> >         "     li      a7, 93\n"               /* __NR_exit */
> >         "     ecall\n"
> >         "1:   ret\n");
> >
> >     static void pin(int cpu)
> >     {
> >       unsigned long mask = 1UL << cpu;
> >
> >       if (syscall(__NR_sched_setaffinity, 0, sizeof(mask), &mask))
> >               printf("sched_setaffinity(%d) failed\n", cpu);
> >     }
> >
> >     static void write_text(unsigned int v)
> >     {
> >       int i;
> >
> >       rw[0] = 0x00000513;
> >       for (i = 1; i <= N; i++)
> >               rw[i] = 0x00050513 | (v << 20);
> >       rw[N + 1] = 0x00008067;
> >     }
> >
> >     static long run_text(void)
> >     {
> >       return ((long (*)(void))rx)();
> >     }
> >
> >     static int t2_main(void *arg)
> >     {
> >       char c;
> >       long r;
> >
> >       pin(hart_a);
> >       while (read(t1_to_t2[0], &c, 1) == 1) {
> >               write(t2_to_t1[1], "", 1);      /* about to hammer the prctl */
> >               while (go) {
> >                       r = syscall(__NR_prctl, PR_RISCV_SET_ICACHE_FLUSH_CTX,
> >                                   PR_RISCV_CTX_SW_FENCEI_OFF,
> >                                   PR_RISCV_SCOPE_PER_THREAD, 0, 0);
> >                       if (r) {
> >                               printf("prctl failed: %ld\n", r);
> >                               break;
> >                       }
> >               }
> >               write(t2_to_t1[1], "", 1);
> >       }
> >       return 0;
> >     }
> >
> >     int main(int argc, char **argv)
> >     {
> >       int iters = argc > 3 ? atoi(argv[3]) : 1000;
> >       int control = argc > 4;
> >       int i, fd, stale = 0, old_only = 0, bad = 0;
> >       long got;
> >       char c;
> >
> >       if (argc < 3) {
> >               printf("usage: %s <hartA> <hartC> [iterations] [control]\n", argv[0]);
> >               return 2;
> >       }
> >       hart_a = atoi(argv[1]);
> >       hart_c = atoi(argv[2]);
> >
> >       fd = memfd_create("text", 0);
> >       ftruncate(fd, PAGE);
> >       rw = mmap(NULL, PAGE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> >       rx = mmap(NULL, PAGE, PROT_READ | PROT_EXEC, MAP_SHARED, fd, 0);
> >       pipe(t1_to_t2);
> >       pipe(t2_to_t1);
> >       pipe(t1_to_q);
> >       pipe(q_to_t1);
> >
> >       if (!fork()) {                  /* Q: another mm, pinned to hart A */
> >               close(t1_to_q[1]);
> >               close(t1_to_t2[1]);
> >               pin(hart_a);
> >               while (read(t1_to_q[0], &c, 1) == 1)
> >                       write(q_to_t1[1], "", 1);
> >               _exit(0);
> >       }
> >       __clone_thread(CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND |
> >                      CLONE_THREAD | CLONE_SYSVSEM,
> >                      t2_stack + sizeof(t2_stack), t2_main, NULL);
> >
> >       for (i = 0; i < iters; i++) {
> >               unsigned int old = 1 + (i & 1), new = 2 - (i & 1);
> >
> >               pin(hart_a);
> >               write_text(old);
> >               syscall(__NR_riscv_flush_icache, 0, 0, 0);      /* all harts */
> >               got = run_text();               /* hart A caches the old code */
> >               if (got != (long)N * old) {
> >                       bad++;
> >                       continue;
> >               }
> >
> >               pin(hart_c);
> >               if (!control) {
> >                       go = 1;
> >                       write(t1_to_t2[1], "", 1);      /* T2 hammers the prctl on A */
> >                       read(t2_to_t1[0], &c, 1);       /* ... from now on */
> >               }
> >               write_text(new);
> >               /* flush hart C now, mark hart A's icache stale for this mm */
> >               syscall(__NR_riscv_flush_icache, rx, (char *)rx + PAGE,
> >                       SYS_RISCV_FLUSH_ICACHE_LOCAL);
> >               if (!control) {
> >                       go = 0;
> >                       read(t2_to_t1[0], &c, 1);       /* T2 stops and blocks */
> >               }
> >               write(t1_to_q[1], "", 1);       /* Q runs on hart A: A leaves our mm */
> >               read(q_to_t1[0], &c, 1);
> >
> >               pin(hart_a);                    /* switch_mm into our mm on hart A */
> >               got = run_text();
> >               if (got != (long)N * new) {
> >                       stale++;
> >                       if (got == (long)N * old)
> >                               old_only++;
> >               }
> >       }
> >       close(t1_to_t2[1]);
> >       close(t1_to_q[1]);
> >       wait(NULL);
> >       printf("harts A=%d C=%d %s: %d of %d iterations ran stale code (%d entirely old, %d warm-up failures)\n",
> >              hart_a, hart_c, control ? "control" : "race", stale, iters,
> >              old_only, bad);
> >       return !!stale;
> >     }
> >
> >  arch/riscv/mm/cacheflush.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> > index f8ead7cb7c7d6..22d6520a05417 100644
> > --- a/arch/riscv/mm/cacheflush.c
> > +++ b/arch/riscv/mm/cacheflush.c
> > @@ -180,20 +180,19 @@ void __init riscv_init_cbo_blocksizes(void)
> >  static void set_icache_stale_mask(void)
> >  {
> >       int cpu = get_cpu();
> > +     unsigned int other;
> >       cpumask_t *mask;
> > -     bool stale_cpu;
> >
> >       /*
> > -      * Mark every other hart's icache as needing a flush for
> > -      * this MM. Maintain the previous value of the current
> > -      * cpu to handle the case when this function is called
> > -      * concurrently on different harts.
> > +      * Mark every other hart's icache as needing a flush for this MM.
> > +      * Set the bits one at a time: flush_icache_mm() and
> > +      * flush_icache_deferred() update this mask concurrently, and a
> > +      * read-modify-write of the whole mask would discard their updates.
> >        */
> >       mask = &current->mm->context.icache_stale_mask;
> > -     stale_cpu = cpumask_test_cpu(cpu, mask);
> > -
> > -     cpumask_setall(mask);
> > -     __assign_cpu(cpu, mask, stale_cpu);
> > +     for_each_possible_cpu(other)
> > +             if (other != cpu)
> > +                     cpumask_set_cpu(other, mask);
>
> Not judging the motivation, but the implementation should avoid the
> for-loop:
>
>
>         cpumask_setall(mask);
>         __cpumask_clear_cpu(cpu);
>
> Or I misunderstood your intention?

That clears this hart's bit unconditionally, and the bit can be a
flush request another hart has just set: flush_icache_mm(), the
membarrier SYNC_CORE path (cpumask_setall() in
prepare_sync_core_cmd()) or set_icache_stale_mask() itself on another
hart running the same mm.  A request cleared here is lost:
flush_icache_deferred() finds the bit clear and switch_mm() skips the
fence.i, so this hart can later run the mm with stale instructions.

flush_icache_mm() can set all bits and clear its own because it
flushes this hart's icache right after.  set_icache_stale_mask() does
not flush locally, so it has to leave its own bit alone, and since
cpumask has no atomic "set all but one", the patch sets the other bits
one at a time with cpumask_set_cpu().

One alternative is to do what flush_icache_mm() does:

  cpumask_setall(mask);
  cpumask_clear_cpu(cpu, mask);
  smp_mb();
  local_flush_icache_all();

which adds a local fence.i to prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX,
PR_RISCV_CTX_SW_FENCEI_OFF).

Another alternative could be to cpumask_setall(mask) with no clear,
even for the current hart, so it does one redundant fence.i at its
next switch_mm() into this mm.

I haven't tested these alternatives yet; if they sound promising, I'll
run them on my physical RISC-V machines to confirm.

Nickolai.