qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST


From: Aurelien Jarno
Subject: Re: [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST
Date: Fri, 14 Jul 2017 23:01:05 +0200
User-agent: NeoMutt/20170113 (1.7.2)

On 2017-07-10 10:45, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/s390x/helper.h      |   1 +
>  target/s390x/cpu_models.c  |   2 +
>  target/s390x/mem_helper.c  | 189 
> +++++++++++++++++++++++++++++++++++++++++++++
>  target/s390x/translate.c   |  13 +++-
>  target/s390x/insn-data.def |   2 +
>  5 files changed, 206 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index ede8471..513b402 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -1353,6 +1353,195 @@ void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
>      env->regs[r1 + 1] = int128_getlo(oldv);
>  }
>  
> +uint32_t HELPER(csst)(CPUS390XState *env, uint32_t r3, uint64_t a1, uint64_t 
> a2)
> +{
> +#if !defined(CONFIG_USER_ONLY) || defined(CONFIG_ATOMIC128)
> +    uint32_t mem_idx = cpu_mmu_index(env, false);
> +#endif
> +    uintptr_t ra = GETPC();
> +    uint32_t fc = extract32(env->regs[0], 0, 8);
> +    uint32_t sc = extract32(env->regs[0], 8, 8);
> +    uint64_t pl = get_address(env, 1) & -16;
> +    uint64_t svh, svl;
> +    uint32_t cc;
> +
> +    /* Sanity check the function code and storage characteristic.  */
> +    if (fc > 1 || sc > 3) {
> +        if (!s390_has_feat(S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2)) {
> +            goto spec_exception;
> +        }
> +        if (fc > 2 || sc > 4 || (fc == 2 && (r3 & 1))) {
> +            goto spec_exception;
> +        }
> +    }
> +
> +    /* Sanity check the alignments.  */
> +    if (extract32(a1, 0, 4 << fc) || extract32(a2, 0, 1 << sc)) {
> +        goto spec_exception;
> +    }
> +
> +    /* Sanity check writability of the store address.  */
> +#ifndef CONFIG_USER_ONLY
> +    probe_write(env, a2, mem_idx, ra);
> +#endif
> +
> +    /* Note that the compare-and-swap is atomic, and the store is atomic, but
> +       the complete operation is not.  Therefore we do not need to assert 
> serial
> +       context in order to implement this.  That said, restart early if we 
> can't
> +       support either operation that is supposed to be atomic.  */
> +    if (parallel_cpus) {
> +        int mask = 0;
> +#if !defined(CONFIG_ATOMIC64)
> +        mask = -8;
> +#elif !defined(CONFIG_ATOMIC128)
> +        mask = -16;
> +#endif
> +        if (((4 << fc) | (1 << sc)) & mask) {
> +            cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> +        }
> +    }

This doesn't look correct. For a 16-byte store, ie sc = 4, and with
ATOMIC128 support, ie mask = -16, the condition is true.

> +    /* All loads happen before all stores.  For simplicity, load the entire
> +       store value area from the parameter list.  */
> +    svh = cpu_ldq_data_ra(env, pl + 16, ra);
> +    svl = cpu_ldq_data_ra(env, pl + 24, ra);
> +
> +    switch (fc) {
> +    case 0:
> +        {
> +            uint32_t nv = cpu_ldl_data_ra(env, pl, ra);
> +            uint32_t cv = env->regs[r3];
> +            uint32_t ov;
> +
> +            if (parallel_cpus) {
> +#ifdef CONFIG_USER_ONLY
> +                uint32_t *haddr = g2h(a1);
> +                ov = atomic_cmpxchg__nocheck(haddr, cv, nv);
> +#else
> +                TCGMemOpIdx oi = make_memop_idx(MO_TEUL | MO_ALIGN, mem_idx);
> +                ov = helper_atomic_cmpxchgl_be_mmu(env, a1, cv, nv, oi, ra);
> +#endif

Not a problem with the patch itself, but how complicated would it be to
make helper_atomic_cmpxchgl_be_mmu (just like the o version) available
also in user mode? That would make less #ifdef in the backends.


The remaining of the patch looks all good to me.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net



reply via email to

[Prev in Thread] Current Thread [Next in Thread]