Re: [PATCH 17/27] target/s390x: Use cpu_{ld,st}*_mmu in do_csst

[Richard Henderson]

On 5/21/23 04:21, Philippe Mathieu-Daudé wrote:
> Hi Richard,
>
> On 20/5/23 18:26, Richard Henderson wrote:
> > Use cpu_ld16_mmu and cpu_st16_mmu to eliminate the special case,
> > and change all of the *_data_ra functions to match.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> > Cc: qemu-s390x@nongnu.org
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   target/s390x/tcg/mem_helper.c | 65 ++++++++++++++---------------------
> >   1 file changed, 26 insertions(+), 39 deletions(-)
> >
> > diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
> > index 0e0d66b3b6..b6cf24403c 100644
> > --- a/target/s390x/tcg/mem_helper.c
> > +++ b/target/s390x/tcg/mem_helper.c
> > @@ -1737,6 +1737,9 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, 
> > uint64_t a1,
> >                           uint64_t a2, bool parallel)
> >   {
> >       uint32_t mem_idx = cpu_mmu_index(env, false);
> > +    MemOpIdx oi16 = make_memop_idx(MO_TE | MO_128, mem_idx);
> > +    MemOpIdx oi8 = make_memop_idx(MO_TE | MO_64, mem_idx);
>
>
> >               if (parallel) {
> >   #ifdef CONFIG_ATOMIC64
> > -                MemOpIdx oi = make_memop_idx(MO_TEUQ | MO_ALIGN, mem_idx);
> > -                ov = cpu_atomic_cmpxchgq_be_mmu(env, a1, cv, nv, oi, ra);
> > +                ov = cpu_atomic_cmpxchgq_be_mmu(env, a1, cv, nv, oi8, ra);
>
> Why is it safe to remove MO_ALIGN here?

Alignment check already done at the start of the function:

    /* Sanity check the alignments.  */
    if (extract32(a1, 0, fc + 2) || extract32(a2, 0, sc)) {
        goto spec_exception;
    }


r~