From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42197)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1bl3Jd-0008H4-1U
	for qemu-devel@nongnu.org; Fri, 16 Sep 2016 20:17:09 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1bl3JY-00021V-ON
	for qemu-devel@nongnu.org; Fri, 16 Sep 2016 20:17:07 -0400
Received: from out2-smtp.messagingengine.com ([66.111.4.26]:35358)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cota@braap.org>) id 1bl3JW-0001y1-FG
	for qemu-devel@nongnu.org; Fri, 16 Sep 2016 20:17:04 -0400
Date: Fri, 16 Sep 2016 20:16:51 -0400
From: "Emilio G. Cota" <cota@braap.org>
Message-ID: <20160917001651.GA22240@flamenco>
References: <1474048017-26696-1-git-send-email-rth@twiddle.net>
	<1474048017-26696-31-git-send-email-rth@twiddle.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1474048017-26696-31-git-send-email-rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH v4 30/35] target-arm: emulate aarch64's
 LL/SC using cmpxchg helpers
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org

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