| Message ID | 20240709134504.3500007-3-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: two LDAPR/STLR fixes | expand |
On 7/9/24 06:45, Peter Maydell wrote: > In commit c1a1f80518d360b when we added the FEAT_LSE2 relaxations to > the alignment requirements for atomic and ordered loads and stores, > we didn't quite get it right for LDAPR/LDAPRH/LDAPRB with no > immediate offset. These instructions were handled in the old decoder > as part of disas_ldst_atomic(), but unlike all the other insns that > function decoded (LDADD, LDCLR, etc) these insns are "ordered", not > "atomic", so they should be using check_ordered_align() rather than > check_atomic_align(). Commit c1a1f80518d360b used > check_atomic_align() regardless for everything in > disas_ldst_atomic(). We then carried that incorrect check over in > the decodetree conversion, where LDAPR/LDAPRH/LDAPRB are now handled > by trans_LDAPR(). > > The effect is that when FEAT_LSE2 is implemented, these instructions > don't honour the SCTLR_ELx.nAA bit and will generate alignment > faults when they should not. > > (The LDAPR insns with an immediate offset were in disas_ldst_ldapr_stlr() > and then in trans_LDAPR_i() and trans_STLR_i(), and have always used > the correct check_ordered_align().) > > Use check_ordered_align() in trans_LDAPR(). > > Cc:qemu-stable@nongnu.org > Fixes: c1a1f80518d360b ("target/arm: Relax ordered/atomic alignment checks for LSE2") > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > target/arm/tcg/translate-a64.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c index 6c07aeaf3bd..5ea204d5009 100644 --- a/target/arm/tcg/translate-a64.c +++ b/target/arm/tcg/translate-a64.c @@ -3543,7 +3543,7 @@ static bool trans_LDAPR(DisasContext *s, arg_LDAPR *a) if (a->rn == 31) { gen_check_sp_alignment(s); } - mop = check_atomic_align(s, a->rn, a->sz); + mop = check_ordered_align(s, a->rn, 0, false, a->sz); clean_addr = gen_mte_check1(s, cpu_reg_sp(s, a->rn), false, a->rn != 31, mop); /*
In commit c1a1f80518d360b when we added the FEAT_LSE2 relaxations to the alignment requirements for atomic and ordered loads and stores, we didn't quite get it right for LDAPR/LDAPRH/LDAPRB with no immediate offset. These instructions were handled in the old decoder as part of disas_ldst_atomic(), but unlike all the other insns that function decoded (LDADD, LDCLR, etc) these insns are "ordered", not "atomic", so they should be using check_ordered_align() rather than check_atomic_align(). Commit c1a1f80518d360b used check_atomic_align() regardless for everything in disas_ldst_atomic(). We then carried that incorrect check over in the decodetree conversion, where LDAPR/LDAPRH/LDAPRB are now handled by trans_LDAPR(). The effect is that when FEAT_LSE2 is implemented, these instructions don't honour the SCTLR_ELx.nAA bit and will generate alignment faults when they should not. (The LDAPR insns with an immediate offset were in disas_ldst_ldapr_stlr() and then in trans_LDAPR_i() and trans_STLR_i(), and have always used the correct check_ordered_align().) Use check_ordered_align() in trans_LDAPR(). Cc: qemu-stable@nongnu.org Fixes: c1a1f80518d360b ("target/arm: Relax ordered/atomic alignment checks for LSE2") Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/tcg/translate-a64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)