Re: [PATCH v2 2/2] target/riscv: add helper_tlb_flush_page(), use for sfence.vma with vaddr operand

["Trevor Gamblin" <tgamblin@baylibre.com>]

On Thu Oct 1, 2026 at 1:14 AM EDT, Alistair Francis wrote:
> On Fri, 2026-09-04 at 14:43 -0400, Trevor Gamblin wrote:
>> sfence.vma unconditionally called helper_tlb_flush(), regardless of
>> the
>> rs1 (vaddr) and rs2 (asid) operands, forcing a page-table walk on the
>> next access. Create a page-level counterpart to helper_tlb_flush()
>> called helper_tlb_flush_page(), and use it whenever rs1 != 0, falling
>> back to the existing full flush for rs1 == 0. This makes RISC-V
>> behaviour more similar to ARM's equivalent (tlbi_aa64_vae1_write),
>> which
>> already does tlb_flush_page_by_mmuidx() instead of a full flush.
>> 
>> Pass get_address(ctx, a->rs1, 0) to gen_helper_tlb_flush_page()
>> rather
>> than get_gpr(), so that the address being passed matches the current
>> addr_xl width regardless of '-cpu' input. Otherwise, the raw register
>> value can carry garbage above that width and never match the address
>> the
>> TLB entry was actually filled under, so the flush silently misses and
>> a
>> stale mapping survives.
>> 
>> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Can you fixup sinval.vma and hinval.* as well please?

Sure! I'm travelling next week, but I'll see if I can get patches written in
downtime or right afterward.

Trevor
>
> Alistair
>
>> ---
>>  target/riscv/helper.h                              | 1 +
>>  target/riscv/tcg/insn_trans/trans_privileged.c.inc | 6 +++++-
>>  target/riscv/tcg/op_helper.c                       | 6 ++++++
>>  3 files changed, 12 insertions(+), 1 deletion(-)
>> 
>> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
>> index 4fc2d3a155..652f85a5c7 100644
>> --- a/target/riscv/helper.h
>> +++ b/target/riscv/helper.h
>> @@ -137,6 +137,7 @@ DEF_HELPER_1(ctr_clear, void, env)
>>  DEF_HELPER_1(wfi, void, env)
>>  DEF_HELPER_1(wrs_nto, void, env)
>>  DEF_HELPER_1(tlb_flush, void, env)
>> +DEF_HELPER_2(tlb_flush_page, void, env, tl)
>>  DEF_HELPER_1(tlb_flush_all, void, env)
>>  DEF_HELPER_4(ctr_add_entry, void, env, tl, tl, tl)
>>  /* Native Debug */
>> diff --git a/target/riscv/tcg/insn_trans/trans_privileged.c.inc
>> b/target/riscv/tcg/insn_trans/trans_privileged.c.inc
>> index a8eaccef67..8655fa332e 100644
>> --- a/target/riscv/tcg/insn_trans/trans_privileged.c.inc
>> +++ b/target/riscv/tcg/insn_trans/trans_privileged.c.inc
>> @@ -155,7 +155,11 @@ static bool trans_sfence_vma(DisasContext *ctx,
>> arg_sfence_vma *a)
>>  {
>>  #ifndef CONFIG_USER_ONLY
>>      decode_save_opc(ctx, 0);
>> -    gen_helper_tlb_flush(tcg_env);
>> +    if (a->rs1 == 0) {
>> +        gen_helper_tlb_flush(tcg_env);
>> +    } else {
>> +        gen_helper_tlb_flush_page(tcg_env, get_address(ctx, a->rs1,
>> 0));
>> +    }
>>      return true;
>>  #endif
>>      return false;
>> diff --git a/target/riscv/tcg/op_helper.c
>> b/target/riscv/tcg/op_helper.c
>> index 7f63f3ada5..178b4d271b 100644
>> --- a/target/riscv/tcg/op_helper.c
>> +++ b/target/riscv/tcg/op_helper.c
>> @@ -607,6 +607,12 @@ void helper_tlb_flush(CPURISCVState *env)
>>      tlb_flush(cs);
>>  }
>>  
>> +void helper_tlb_flush_page(CPURISCVState *env, target_ulong addr)
>> +{
>> +    check_sfence_vma(env, GETPC());
>> +    tlb_flush_page(env_cpu(env), addr);
>> +}
>> +
>>  void helper_tlb_flush_all(CPURISCVState *env)
>>  {
>>      CPUState *cs = env_cpu(env);