Re: [Qemu-devel] [PATCH v4 30/35] target-arm: emulate aarch64's LL/SC using cmpxchg helpers

2016-09-16 Thread Richard Henderson

On 09/16/2016 05:16 PM, Emilio G. Cota wrote:

> +uint64_t *haddr = g2h(addr);
> +o0 = ldq_le_p(haddr + 0);
> +o1 = ldq_le_p(haddr + 1);
> +oldv = int128_make128(o0, o1);
> +
> +success = int128_eq(oldv, cmpv);
> +if (success) {
> +stq_le_p(haddr + 0, int128_getlo(newv));
> +stq_le_p(haddr + 8, int128_gethi(newv));

Shouldn't this be + 1 instead, just like the above load?

If so, the same applies to the store in the _be function.


Yep, good catch.


r~



Re: [Qemu-devel] [PATCH v4 30/35] target-arm: emulate aarch64's LL/SC using cmpxchg helpers

2016-09-16 Thread Emilio G. Cota
On Fri, Sep 16, 2016 at 10:46:52 -0700, Richard Henderson wrote:
(snip)
> +/* Returns 0 on success; 1 otherwise.  */
> +uint64_t HELPER(paired_cmpxchg64_le)(CPUARMState *env, uint64_t addr,
> + uint64_t new_lo, uint64_t new_hi)
> +{
> +uintptr_t ra = GETPC();
> +Int128 oldv, cmpv, newv;
> +bool success;
> +
> +cmpv = int128_make128(env->exclusive_val, env->exclusive_high);
> +newv = int128_make128(new_lo, new_hi);
> +
> +if (parallel_cpus) {
> +#ifndef CONFIG_ATOMIC128
> +cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> +#else
> +int mem_idx = cpu_mmu_index(env, false);
> +TCGMemOpIdx oi = make_memop_idx(MO_LEQ | MO_ALIGN_16, mem_idx);
> +oldv = helper_atomic_cmpxchgo_le_mmu(env, addr, cmpv, newv, oi, ra);
> +success = int128_eq(oldv, cmpv);
> +#endif
> +} else {
> +uint64_t o0, o1;
> +
> +#ifdef CONFIG_USER_ONLY
> +/* ??? Enforce alignment.  */
> +uint64_t *haddr = g2h(addr);
> +o0 = ldq_le_p(haddr + 0);
> +o1 = ldq_le_p(haddr + 1);
> +oldv = int128_make128(o0, o1);
> +
> +success = int128_eq(oldv, cmpv);
> +if (success) {
> +stq_le_p(haddr + 0, int128_getlo(newv));
> +stq_le_p(haddr + 8, int128_gethi(newv));

Shouldn't this be + 1 instead, just like the above load?

If so, the same applies to the store in the _be function.

Thanks,

Emilio