On Tue, 2026-08-11 at 09:24 +0800, wangyang wrote:
> gen_sc() uses a host cmpxchg against load_val, and AMO store events
> are
> never propagated to other harts' load_res reservations.  An AMO by
> another hart that stores the same numeric value (amoadd +0, same-
> value
> amoswap, amoor 0, amoxor 0, amoand -1) therefore leaves the
> reservation
> intact and a later sc.w/sc.d succeeds, while native RISC-V hardware
> always fails the SC.  The RISC-V A-extension requires SC to fail when
> a
> store to the reservation set from another hart can be observed
> between
> the LR and SC; a preserve-value AMO is still a real atomic store
> event.
> 
> Add a helper that walks all CPUs under RCU after a successful AMO
> store
> and clears overlapping reservations, and call it from gen_amo(). 
> This
> makes the word/doubleword preserve-value screens match native
> behavior;
> a complete upstream fix should linearize reservation-set invalidation
> with the store (the patch is provided as a starting point).
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4149
> Signed-off-by: wangyang <[email protected]>

Reviewed-by: Alistair Francis <[email protected]>

Alistair

> ---
>  target/riscv/helper.h        |  3 +++
>  target/riscv/tcg/op_helper.c | 25 +++++++++++++++++++++++++
>  target/riscv/tcg/translate.c |  2 ++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 542b7c264fc..ed5b972e1b5 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -1358,3 +1358,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)
> +
> +/* Invalidate other harts' reservations after a successful AMO
> store. */
> +DEF_HELPER_3(amo_invalidate_reservations, void, env, tl, tl)
> diff --git a/target/riscv/tcg/op_helper.c
> b/target/riscv/tcg/op_helper.c
> index ba3c7da375a..11cac77e3fe 100644
> --- a/target/riscv/tcg/op_helper.c
> +++ b/target/riscv/tcg/op_helper.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/rcu.h"
>  #include "cpu.h"
>  #include "target/riscv/tcg/csr.h"
>  #include "internals.h"
> @@ -283,6 +284,30 @@ void helper_sc_probe_write(CPURISCVState *env,
> target_ulong addr,
>      probe_write(env, addr, size, mmu_idx, ra);
>  }
>  
> +void helper_amo_invalidate_reservations(CPURISCVState *env,
> +                                        target_ulong addr,
> +                                        target_ulong size)
> +{
> +    CPUState *cpu;
> +
> +    WITH_RCU_READ_LOCK_GUARD() {
> +        CPU_FOREACH(cpu) {
> +            CPURISCVState *other_env = cpu_env(cpu);
> +            target_ulong reservation;
> +
> +            if (other_env == env) {
> +                continue;
> +            }
> +
> +            reservation = qatomic_read(&other_env->load_res);
> +            if (reservation != (target_ulong)-1 &&
> +                reservation >= addr && reservation - addr < size) {
> +                qatomic_set(&other_env->load_res, (target_ulong)-1);
> +            }
> +        }
> +    }
> +}
> +
>  #ifndef CONFIG_USER_ONLY
>  
>  target_ulong helper_sret(CPURISCVState *env)
> diff --git a/target/riscv/tcg/translate.c
> b/target/riscv/tcg/translate.c
> index 9684dbe7528..007a90f1e62 100644
> --- a/target/riscv/tcg/translate.c
> +++ b/target/riscv/tcg/translate.c
> @@ -1159,6 +1159,8 @@ 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);
> +    gen_helper_amo_invalidate_reservations(tcg_env, src1,
> +                                          
> tcg_constant_tl(memop_size(mop)));
>  
>      gen_set_gpr(ctx, a->rd, dest);
>      return true;
> -- 
> 2.43.0

Reply via email to