From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60378) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjHXA-0007fQ-T2 for qemu-devel@nongnu.org; Sun, 11 Sep 2016 23:03:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjHX6-0000oU-LX for qemu-devel@nongnu.org; Sun, 11 Sep 2016 23:03:48 -0400 Date: Mon, 12 Sep 2016 13:02:58 +1000 From: David Gibson Message-ID: <20160912030258.GA15077@voom.fritz.box> References: <1473426865-14191-1-git-send-email-nikunj@linux.vnet.ibm.com> <1473426865-14191-3-git-send-email-nikunj@linux.vnet.ibm.com> <1473462236.8689.197.camel@kernel.crashing.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="k1lZvvs/B4yU6o8G" Content-Disposition: inline In-Reply-To: <1473462236.8689.197.camel@kernel.crashing.org> Subject: Re: [Qemu-devel] [PATCH v2 3/3] target-ppc: tlbie should have global effect List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Herrenschmidt Cc: Nikunj A Dadhania , qemu-ppc@nongnu.org, alex.bennee@linaro.org, qemu-devel@nongnu.org, rth@twiddle.net --k1lZvvs/B4yU6o8G Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Sep 10, 2016 at 09:03:56AM +1000, Benjamin Herrenschmidt wrote: > On Fri, 2016-09-09 at 18:44 +0530, Nikunj A Dadhania wrote: > > +static inline void tlb_clear_flag(CPUState *cs) > > +{ > > +=A0=A0=A0 PowerPCCPU *cpu =3D POWERPC_CPU(cs); > > +=A0=A0=A0 CPUPPCState *env =3D &cpu->env; > > + > > +=A0=A0=A0 env->tlb_need_flush =3D 0; > > +} >=20 > What is the point of making this a separate function ? >=20 > Also I'm not 100% certain about the correctness of clearing > TLB_NEED_GLOBAL_FLUSH on the "other" guy. >=20 > We could have the situation where: >=20 > cpu 1: cpu 2: > sets both .. > isync (clears local flush) .. > > .. set both > .. .. > .. .. > ptesync (clears global flush) .. (both gets cleared) >=20 > Now here, you can see that cpu2 never does a global flush and so the > new translation inserted by cpu 1 is not cleared while architecturally > it should be. >=20 > That being said, I doubt the above scenario can happen in practice, > but I think it's safer if you only clear the local bit on the "other" > CPUs. I'll wait for a respin addressing these comments. Please also add a cover letter on the next version. >=20 > > =A0static inline void check_tlb_flush(CPUPPCState *env, uint32_t > > global) > > =A0{ > > =A0=A0=A0=A0 CPUState *cs =3D CPU(ppc_env_get_cpu(env)); > > @@ -161,6 +169,17 @@ static inline void check_tlb_flush(CPUPPCState > > *env, uint32_t global) > > =A0=A0=A0=A0=A0=A0=A0=A0 tlb_flush(cs, 1); > > =A0=A0=A0=A0=A0=A0=A0=A0 env->tlb_need_flush &=3D ~TLB_NEED_LOCAL_FLUSH; > > =A0=A0=A0=A0 } > > + > > +=A0=A0=A0 if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH))= { > > +=A0=A0=A0=A0=A0=A0=A0 CPUState *other_cs; > > +=A0=A0=A0=A0=A0=A0=A0 CPU_FOREACH(other_cs) { > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (other_cs !=3D cs) { > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 tlb_clear_flag(other_cs); > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 tlb_flush(other_cs, 1); > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 } > > +=A0=A0=A0=A0=A0=A0=A0 } > > +=A0=A0=A0=A0=A0=A0=A0 env->tlb_need_flush &=3D ~TLB_NEED_GLOBAL_FLUSH; > > +=A0=A0=A0 } > > =A0} > > =A0#else >=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 --k1lZvvs/B4yU6o8G Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBAgAGBQJX1hriAAoJEGw4ysog2bOSHq0P/jdZi3mZIMpc89GR9nHu4SvP gfP0v6Fyfl431jBlpYsaI0xibgPibS82E4Myw2Q8Nn5Q+gWZQMu5/ZVy1G+XuZtk jihcL2Sw2gH4NpL1fV0E/kHM54zHiJMUljON1Cidc3DjmNgifTKIKz9tX9pli8Rj peAgGCFJ2JykNUACQb5X50VMiHt0P2q7K/zmju2lJVELHWrcy0L4lUNjLeMLQWZi ZgYObRc+ffN+d+trSeOntRw96pmrrV6f4uVPGZhYVEQ/tq5y6QFHj2aQTctxErN6 tQdF9dDpiyCQ8Ck/CQQ1BSkBOE7m8LFuxDBbKeGvhEgLV1IWT7/TMrQF76qGGVB1 iNtAOvQ+VrcYeKFyZpCneT2wzaa4px7d6b4mLjRLWGXsiJTA5OlDLhqivMaR1S3g V1X8Eiy2n9OPul8pDV/7f8ZPDyBJ9R5cDJJceV53b5gy0/QZMIc2O3Kk72BQPo+k 30e5Bb141eP57iy6E190Uqvay3hwupas6b3fy2RWIeKRw32qrI9YurkX4NSNtdUr xTN9vooPO3xxBvRMsWgHh86h0UcOPKY5bGSqZkqwArN5SyU3r2es/P7E2B3U+Xue I8BRBxYoZbfN53Yo+wPM6fb2gX8YyjB0MGeb8Hr59KRCChVhDyFcATBD5B2SihBu kAIeUsBHwBvIgc5sPRxt =XPSM -----END PGP SIGNATURE----- --k1lZvvs/B4yU6o8G--