On Tue, 2026-10-13 at 13:00 -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 10/13/2026 10:14 AM, Palmer Dabbelt wrote:
> > On Tue, 13 Oct 2026 04:57:49 PDT (-0700),
> > [email protected] wrote:
> > > 
> > > 
> > > On 10/2/2026 10:47 AM, Rick Richard wrote:
> > > > With Ztso, plain loads and stores follow RVTSO, but the
> > > > barriers
> > > > around them use TCG_MO_ALL.  That also orders a store before a
> > > > later
> > > > load, which TSO doesn't require, and is expensive: on x86 hosts
> > > > it
> > > > adds a locked instruction around every load and store in a
> > > > multi-threaded guest.
> > 
> > Thanks.  I'm not quite sure why I had TCG_MO_ALL in there, I think
> > maybe I just misread the TCG_MO_* vs TCG_BAR_* blurb?
> > 
> > Reviewed-by: Palmer Dabbelt <[email protected]>
> > 
> > > > The barrier after a load now only keeps later loads and stores
> > > > behind it, and the barrier before a store only keeps earlier
> > > > loads
> > > > and stores ahead of it.  That is all TSO needs.
> > > > 
> > > 
> > > Fixes: 09c4e88759 ("RISC-V: Add support for Ztso")
> > > 
> > > > Signed-off-by: Rick Richard <[email protected]>
> > > > ---
> > > 
> > > Reviewed-by: Daniel Henrique Barboza
> > > <[email protected]>
> > > 
> > > > I found this while looking into why riscv64 guests were so much
> > > > slower than aarch64 ones under bsd-user on FreeBSD.  The
> > > > default -cpu
> > > > max enables Ztso, and turning it off with ztso=false accounted
> > > > for
> > > > most of the difference.
> > 
> > Maybe we shouldn't default to Ztso, or at least not default to Ztso
> > on host systems that themselves aren't TSO?
> 
> I vote to disable it in the 'max' CPU.  Changing the mem model from
> RVWMO to
> RVTSO seems like a decision users have to make in the command line,
> not
> something we decided to do because this particular CPU model must
> have all
> possible extensions enabled.

Agreed

> 
> In theory we could enable it depending on the host affinity with TSO,
> but I'd
> rather not create a precedent where a TCG CPU model delivers
> different
> extensions based on host capabilities.  We have enough of that in the
> KVM
> driver.

No, let's not do that

Alistair

> 
> 
> Thanks,
> Daniel
> 
> > 
> > > > On an x86-64 host the patch made xz almost four times faster
> > > > and cut
> > > > the time of a clang compile by about 30%, the same as running
> > > > with
> > > > ztso=false.  The output didn't change.
> > > > 
> > > > I also tested it on a Raspberry Pi 3B+, which is arm64 and
> > > > doesn't
> > > > give TSO ordering in hardware, to make sure the weaker barriers
> > > > still
> > > > hold up there.  Everything ran correctly, and xz was about 12%
> > > > faster.
> > > > 
> > > >   target/riscv/tcg/insn_trans/trans_rvi.c.inc | 4 ++--
> > > >   target/riscv/tcg/insn_trans/trans_rvv.c.inc | 4 ++--
> > > >   2 files changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/target/riscv/tcg/insn_trans/trans_rvi.c.inc
> > > > b/target/riscv/tcg/insn_trans/trans_rvi.c.inc
> > > > index 40b4c9aa..d31e4295 100644
> > > > --- a/target/riscv/tcg/insn_trans/trans_rvi.c.inc
> > > > +++ b/target/riscv/tcg/insn_trans/trans_rvi.c.inc
> > > > @@ -424,7 +424,7 @@ static bool gen_load(DisasContext *ctx,
> > > > arg_lb *a, MemOp memop)
> > > >       }
> > > > 
> > > >       if (ctx->ztso) {
> > > > -        tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
> > > > +        tcg_gen_mb(TCG_MO_LD_LD | TCG_MO_LD_ST |
> > > > TCG_BAR_LDAQ);
> > > >       }
> > > > 
> > > >       return out;
> > > > @@ -485,7 +485,7 @@ static bool gen_store_tl(DisasContext *ctx,
> > > > arg_sb *a, MemOp memop)
> > > >       TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
> > > > 
> > > >       if (ctx->ztso) {
> > > > -        tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
> > > > +        tcg_gen_mb(TCG_MO_LD_ST | TCG_MO_ST_ST |
> > > > TCG_BAR_STRL);
> > > >       }
> > > > 
> > > >       tcg_gen_qemu_st_tl(data, addr, ctx->mem_idx, memop);
> > > > diff --git a/target/riscv/tcg/insn_trans/trans_rvv.c.inc
> > > > b/target/riscv/tcg/insn_trans/trans_rvv.c.inc
> > > > index a22e2cae..8c21ab87 100644
> > > > --- a/target/riscv/tcg/insn_trans/trans_rvv.c.inc
> > > > +++ b/target/riscv/tcg/insn_trans/trans_rvv.c.inc
> > > > @@ -731,7 +731,7 @@ static bool ldst_us_trans(uint32_t vd,
> > > > uint32_t rs1, uint32_t data,
> > > >        * around the loop.
> > > >        */
> > > >       if (is_store && s->ztso) {
> > > > -        tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL);
> > > > +        tcg_gen_mb(TCG_MO_LD_ST | TCG_MO_ST_ST |
> > > > TCG_BAR_STRL);
> > > >       }
> > > > 
> > > >       mark_vs_dirty(s);
> > > > @@ -739,7 +739,7 @@ static bool ldst_us_trans(uint32_t vd,
> > > > uint32_t rs1, uint32_t data,
> > > >       fn(dest, mask, base, tcg_env, desc);
> > > > 
> > > >       if (!is_store && s->ztso) {
> > > > -        tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
> > > > +        tcg_gen_mb(TCG_MO_LD_LD | TCG_MO_LD_ST |
> > > > TCG_BAR_LDAQ);
> > > >       }
> > > > 
> > > >       finalize_rvv_inst(s);
> 

Reply via email to