From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56690) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhU8H-00026m-PA for qemu-devel@nongnu.org; Wed, 07 Sep 2016 00:06:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bhU8F-0004vg-Je for qemu-devel@nongnu.org; Wed, 07 Sep 2016 00:06:40 -0400 Date: Wed, 7 Sep 2016 14:02:52 +1000 From: David Gibson Message-ID: <20160907040252.GJ2780@voom.fritz.box> References: <1472797976-24210-1-git-send-email-nikunj@linux.vnet.ibm.com> <1472797976-24210-4-git-send-email-nikunj@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="yhze8HlyfmXt1APY" Content-Disposition: inline In-Reply-To: <1472797976-24210-4-git-send-email-nikunj@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH RFC 3/4] target-ppc: use atomic_cmpxchg for ld/st reservation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania Cc: qemu-ppc@nongnu.org, alex.bennee@linaro.org, rth@twiddle.net, qemu-devel@nongnu.org, benh@kernel.crashing.org --yhze8HlyfmXt1APY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 02, 2016 at 12:02:55PM +0530, Nikunj A Dadhania wrote: > Signed-off-by: Nikunj A Dadhania This really needs a comment indicating that this implementation isn't strictly correct (although probably good enough in practice). Specifically a racing store which happens to store the same value which was already in memory should clobber the reservation, but won't with this implementation. I had a long discussion at KVM Forum with Emilio Costa about this, in which I discovered just how hard it is to strictly implement store-conditional semantics in terms of anything else. So, this is probably a reasonable substitute, but we should note the fact that it's not 100%. > --- > target-ppc/translate.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) >=20 > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index 4a882b3..447c13e 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -72,6 +72,7 @@ static TCGv cpu_cfar; > #endif > static TCGv cpu_xer, cpu_so, cpu_ov, cpu_ca; > static TCGv cpu_reserve; > +static TCGv cpu_reserve_val; > static TCGv cpu_fpscr; > static TCGv_i32 cpu_access_type; > =20 > @@ -176,6 +177,9 @@ void ppc_translate_init(void) > cpu_reserve =3D tcg_global_mem_new(cpu_env, > offsetof(CPUPPCState, reserve_addr), > "reserve_addr"); > + cpu_reserve_val =3D tcg_global_mem_new(cpu_env, > + offsetof(CPUPPCState, reserve_val), > + "reserve_val"); > =20 > cpu_fpscr =3D tcg_global_mem_new(cpu_env, > offsetof(CPUPPCState, fpscr), "fpscr"= ); > @@ -3086,7 +3090,7 @@ static void gen_##name(DisasContext *ctx) = \ > } \ > tcg_gen_qemu_ld_tl(gpr, t0, ctx->mem_idx, memop); \ > tcg_gen_mov_tl(cpu_reserve, t0); \ > - tcg_gen_st_tl(gpr, cpu_env, offsetof(CPUPPCState, reserve_val)); \ > + tcg_gen_mov_tl(cpu_reserve_val, gpr); \ > tcg_temp_free(t0); \ > } > =20 > @@ -3112,14 +3116,28 @@ static void gen_conditional_store(DisasContext *c= tx, TCGv EA, > int reg, int memop) > { > TCGLabel *l1; > + TCGv_i32 tmp =3D tcg_temp_local_new_i32(); > + TCGv t0; > =20 > + tcg_gen_movi_i32(tmp, 0); > tcg_gen_trunc_tl_i32(cpu_crf[0], cpu_so); > l1 =3D gen_new_label(); > tcg_gen_brcond_tl(TCG_COND_NE, EA, cpu_reserve, l1); > - tcg_gen_ori_i32(cpu_crf[0], cpu_crf[0], 1 << CRF_EQ); > - tcg_gen_qemu_st_tl(cpu_gpr[reg], EA, ctx->mem_idx, memop); > + > + t0 =3D tcg_temp_new(); > + tcg_gen_atomic_cmpxchg_tl(t0, EA, cpu_reserve_val, cpu_gpr[reg], > + ctx->mem_idx, DEF_MEMOP(memop)); > + tcg_gen_setcond_tl(TCG_COND_EQ, t0, t0, cpu_reserve_val); > + tcg_gen_trunc_tl_i32(tmp, t0); > + > gen_set_label(l1); > + tcg_gen_shli_i32(tmp, tmp, CRF_EQ); > + tcg_gen_or_i32(cpu_crf[0], cpu_crf[0], tmp); > tcg_gen_movi_tl(cpu_reserve, -1); > + tcg_gen_movi_tl(cpu_reserve_val, 0); > + > + tcg_temp_free(t0); > + tcg_temp_free_i32(tmp); > } > #endif > =20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --yhze8HlyfmXt1APY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBAgAGBQJXz5FsAAoJEGw4ysog2bOS0yIQALLvmhKtLmTcHlSE08lVbj8P lK2b+QeJVR5oC0Ts/dHPEM27cdnjTkodsebrmY3cVSljt59U5YWpvgc24LTkqh++ smsyW6qzXPZxVRAKM6UV7QfrVafdm22caeQ1dGOomHnlXGjD3/+t3mg+hAD3Ncgg xfR1VYmYMweBP+Px32S4vBFePGNE10xHlecmlNk3njh4LqGoDNyp3NbUpVH5u8U9 rz4jtRlcoxMjAOnA1HV3SYZDuexaNpfWV/jfVo9gKV6TL63mNHI3SL+eNV9H0YOI Humts+fzf8ftIbZS8AnFr0/VO2/orRChNoU3XeJKXHfclrKE6MFOKJiTm5XTiTWO mGH0H9Fn7zyJyESn4pLmitAFtgWWdCotwXiRRdF+sOgg/v7vmMmav6c6sMzpSo1E eDLDacmEZ/2OZ8wq8cNkdnsliei8yqMYacNLB5URrf1Nx36TyhWNKtyQ+5JWgXc4 f+QARL9aFDyqVrix4amZocGXRiQJ471PXv30BbX14ct4iM1tKvyFNnaVB8+gqLre zkWfc9wFXD1VmZ6UcPRt6HxYVj+5y7bsSTYyc8O9iKRpnAuaGFCJLb74jfHHvsFY Gx9Ga8Nr4wRP08CNQLuAIKrUzyfMomdaOVb/vOmRVx5iRpA1W/c5BgA0Q7UlRawS peYswsGQN73f/l7p/nBe =fJbU -----END PGP SIGNATURE----- --yhze8HlyfmXt1APY--