Revert "linux-user/riscv: invalidate reservations after stores"

This reverts commit 5089493e45.

Rick reported that this patch results in large slowdowns when using
multiple threads. The original patch serialises all stores, resulting in
all vCPUs stopping for every store, which would explain the slowdown.

The results from Rick show

xz compressing 2 MB at -6, without and with the patch:

                 without patch    with patch
   xz -T1        5 s              122 s   (identical output)
   xz -T8        1 s              no output after 600 s

That is too large of an overhead, so let's revert this patch until a
better solution is found.

Reported-by: Rick <rick@sloservers.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Message-ID: <20261001232119.1765494-1-alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

linux-user/riscv/target_cpu.h

@@ -9,8 +9,6 @@ static inline void cpu_clone_regs_child(CPURISCVState *env, target_ulong newsp,
     }
 
     env->gpr[xA0] = 0;
-    env->load_res = -1;
-    env->load_res_size = 0;
 }
 
 static inline void cpu_clone_regs_parent(CPURISCVState *env, unsigned flags)

target/riscv/cpu.c

@@ -1094,9 +1094,6 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
     env->xl = riscv_cpu_mxl(env);
     cs->exception_index = RISCV_EXCP_NONE;
     env->load_res = -1;
-#ifdef CONFIG_USER_ONLY
-    env->load_res_size = 0;
-#endif
     set_default_nan_mode(1, &env->fp_status);
     /* Default NaN value: sign bit clear, frac msb set */
     set_float_default_nan_pattern(0b01000000, &env->fp_status);

target/riscv/cpu.h

@@ -264,9 +264,6 @@ struct CPUArchState {
 
     uint64_t pc;
     uint64_t load_res;
-#ifdef CONFIG_USER_ONLY
-    uint64_t load_res_size;
-#endif
     uint64_t load_val;
 
     /* Floating-Point state */

target/riscv/helper.h

@@ -1361,11 +1361,6 @@ DEF_HELPER_1(ssamoswap_disabled, void, env)
 /* Zalrsc SC write probe */
 DEF_HELPER_FLAGS_3(sc_probe_write, TCG_CALL_NO_WG, void, env, tl, tl)
 
-#ifdef CONFIG_USER_ONLY
-/* Invalidate reservations overlapping a completed linux-user store. */
-DEF_HELPER_3(riscv_invalidate_reservations, void, env, tl, tl)
-#endif
-
 /* SiFive Custom int8 Matrix-Multiply */
 DEF_HELPER_5(sf_vqmaccu_4x8x4, void, ptr, ptr, ptr, env, i32)
 DEF_HELPER_5(sf_vqmacc_4x8x4, void, ptr, ptr, ptr, env, i32)

target/riscv/tcg/insn_trans/trans_rva.c.inc

@@ -32,11 +32,6 @@
 
 static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
 {
-#ifdef CONFIG_USER_ONLY
-    if (!gen_riscv_reservation_serialize(ctx)) {
-        return false;
-    }
-#endif
     TCGv src1;
 
     mop |= MO_ALIGN;
@@ -58,9 +53,6 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
 
     /* Put addr in load_res, data in load_val.  */
     tcg_gen_mov_tl(load_res, src1);
-#ifdef CONFIG_USER_ONLY
-    tcg_gen_movi_tl(load_res_size, memop_size(mop));
-#endif
     gen_set_gpr(ctx, a->rd, load_val);
 
     return true;
@@ -68,17 +60,9 @@ static bool gen_lr(DisasContext *ctx, arg_atomic *a, MemOp mop)
 
 static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
 {
-#ifdef CONFIG_USER_ONLY
-    if (!gen_riscv_reservation_serialize(ctx)) {
-        return false;
-    }
-#endif
     TCGv dest, src1, src2;
     TCGLabel *l1 = gen_new_label();
     TCGLabel *l2 = gen_new_label();
-#ifdef CONFIG_USER_ONLY
-    TCGLabel *l3 = gen_new_label();
-#endif
 
     mop |= MO_ALIGN;
     mop |= ctx->mo_endianness;
@@ -95,11 +79,6 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
     src2 = get_gpr(ctx, a->rs2, EXT_NONE);
     tcg_gen_atomic_cmpxchg_tl(dest, load_res, load_val, src2,
                               ctx->mem_idx, mop);
-#ifdef CONFIG_USER_ONLY
-    tcg_gen_brcond_tl(TCG_COND_NE, dest, load_val, l3);
-    gen_riscv_invalidate_reservations(src1, mop);
-    gen_set_label(l3);
-#endif
     tcg_gen_setcond_tl(TCG_COND_NE, dest, dest, load_val);
     gen_set_gpr(ctx, a->rd, dest);
     tcg_gen_br(l2);
@@ -125,9 +104,6 @@ static bool gen_sc(DisasContext *ctx, arg_atomic *a, MemOp mop)
      * an SC to any address, in between an LR and SC pair.
      */
     tcg_gen_movi_tl(load_res, -1);
-#ifdef CONFIG_USER_ONLY
-    tcg_gen_movi_tl(load_res_size, 0);
-#endif
 
     return true;
 }

target/riscv/tcg/insn_trans/trans_rvi.c.inc

@@ -489,9 +489,6 @@ static bool gen_store_tl(DisasContext *ctx, arg_sb *a, MemOp memop)
     }
 
     tcg_gen_qemu_st_tl(data, addr, ctx->mem_idx, memop);
-#ifdef CONFIG_USER_ONLY
-    gen_riscv_invalidate_reservations(addr, memop);
-#endif
     return true;
 }
 
@@ -521,19 +518,11 @@ static bool gen_store_i128(DisasContext *ctx, arg_sb *a, MemOp memop)
         }
         tcg_gen_qemu_st_i128(t16, addrl, ctx->mem_idx, memop);
     }
-#ifdef CONFIG_USER_ONLY
-    gen_riscv_invalidate_reservations(addrl, memop);
-#endif
     return true;
 }
 
 static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
 {
-#ifdef CONFIG_USER_ONLY
-    if (!gen_riscv_reservation_serialize(ctx)) {
-        return false;
-    }
-#endif
     memop |= ctx->mo_endianness;
     if (ctx->cfg_ptr->ext_zama16b) {
         memop |= MO_ATOM_WITHIN16;

target/riscv/tcg/insn_trans/trans_rvzawrs.c.inc

@@ -32,9 +32,6 @@ static bool trans_wrs_sto(DisasContext *ctx, arg_wrs_sto *a)
 
     /* Clear the load reservation  (if any).  */
     tcg_gen_movi_tl(load_res, -1);
-#ifdef CONFIG_USER_ONLY
-    tcg_gen_movi_tl(load_res_size, 0);
-#endif
 
     gen_update_pc(ctx, ctx->cur_insn_len);
     tcg_gen_exit_tb(NULL, 0);

target/riscv/tcg/op_helper.c

@@ -19,9 +19,6 @@
  */
 
 #include "qemu/osdep.h"
-#ifdef CONFIG_USER_ONLY
-#include "qemu/rcu.h"
-#endif
 #include "cpu.h"
 #include "target/riscv/tcg/csr.h"
 #ifndef CONFIG_USER_ONLY
@@ -301,38 +298,6 @@ void helper_sc_probe_write(CPURISCVState *env, target_ulong addr,
     probe_write(env, addr, size, mmu_idx, ra);
 }
 
-#ifdef CONFIG_USER_ONLY
-
-void helper_riscv_invalidate_reservations(CPURISCVState *env,
-                                          target_ulong addr,
-                                          target_ulong size)
-{
-    CPUState *cpu;
-
-    /* EXCP_ATOMIC keeps other guest harts out while this list is updated. */
-    WITH_RCU_READ_LOCK_GUARD() {
-        CPU_FOREACH(cpu) {
-            CPURISCVState *other_env = cpu_env(cpu);
-            target_ulong reservation = other_env->load_res;
-            target_ulong reservation_size = other_env->load_res_size;
-            bool overlap;
-
-            if (other_env == env || reservation == (target_ulong)-1) {
-                continue;
-            }
-            overlap = reservation < addr
-                ? addr - reservation < reservation_size
-                : reservation - addr < size;
-            if (overlap) {
-                other_env->load_res = -1;
-                other_env->load_res_size = 0;
-            }
-        }
-    }
-}
-
-#endif
-
 #ifndef CONFIG_USER_ONLY
 
 target_ulong helper_sret(CPURISCVState *env)

target/riscv/tcg/translate.c

@@ -42,9 +42,6 @@ static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc;
 static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
 static TCGv_i32 cpu_vl, cpu_vstart;
 static TCGv load_res;
-#ifdef CONFIG_USER_ONLY
-static TCGv load_res_size;
-#endif
 static TCGv load_val;
 
 /*
@@ -1153,34 +1150,10 @@ static bool gen_unary_per_ol(DisasContext *ctx, arg_r2 *a, DisasExtend ext,
     return gen_unary(ctx, a, ext, f_tl);
 }
 
-#ifdef CONFIG_USER_ONLY
-static bool gen_riscv_reservation_serialize(DisasContext *ctx)
-{
-    /* Keep the memory operation and reservation update in one guest step. */
-    if (tb_cflags(ctx->base.tb) & CF_PARALLEL) {
-        gen_helper_exit_atomic(tcg_env);
-        ctx->base.is_jmp = DISAS_NORETURN;
-        return false;
-    }
-    return true;
-}
-
-static void gen_riscv_invalidate_reservations(TCGv addr, MemOp mop)
-{
-    gen_helper_riscv_invalidate_reservations(tcg_env, addr,
-                                             tcg_constant_tl(memop_size(mop)));
-}
-#endif
-
 static bool gen_amo(DisasContext *ctx, arg_atomic *a,
                     void(*func)(TCGv, TCGv, TCGv, TCGArg, MemOp),
                     MemOp mop)
 {
-#ifdef CONFIG_USER_ONLY
-    if (!gen_riscv_reservation_serialize(ctx)) {
-        return false;
-    }
-#endif
     TCGv dest = dest_gpr(ctx, a->rd);
     TCGv src1, src2 = get_gpr(ctx, a->rs2, EXT_NONE);
     MemOp size = mop & MO_SIZE;
@@ -1195,9 +1168,6 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
     decode_save_opc(ctx, RISCV_UW2_ALWAYS_STORE_AMO);
     src1 = get_address(ctx, a->rs1, 0);
     func(dest, src1, src2, ctx->mem_idx, mop);
-#ifdef CONFIG_USER_ONLY
-    gen_riscv_invalidate_reservations(src1, mop);
-#endif
 
     gen_set_gpr(ctx, a->rd, dest);
     return true;
@@ -1526,10 +1496,6 @@ void riscv_translate_init(void)
     size_t pc_offset     = offsetof(CPURISCVState, pc) + field_offset;
     size_t res_offset    = offsetof(CPURISCVState, load_res) + field_offset;
     size_t val_offset    = offsetof(CPURISCVState, load_val) + field_offset;
-#ifdef CONFIG_USER_ONLY
-    size_t res_size_offset = offsetof(CPURISCVState, load_res_size)
-                             + field_offset;
-#endif
 
     for (i = 1; i < 32; i++) {
         cpu_gpr[i] = tcg_global_mem_new(tcg_env,
@@ -1549,9 +1515,5 @@ void riscv_translate_init(void)
     cpu_vl = tcg_global_mem_new_i32(tcg_env, vl_offset, "vl");
     cpu_vstart = tcg_global_mem_new_i32(tcg_env, vstart_offset, "vstart");
     load_res = tcg_global_mem_new(tcg_env, res_offset, "load_res");
-#ifdef CONFIG_USER_ONLY
-    load_res_size = tcg_global_mem_new(tcg_env, res_size_offset,
-                                       "load_res_size");
-#endif
     load_val = tcg_global_mem_new(tcg_env, val_offset, "load_val");
 }