From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51057) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyGuy-00036L-3g for qemu-devel@nongnu.org; Mon, 16 Nov 2015 05:21:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZyGuv-0000kl-11 for qemu-devel@nongnu.org; Mon, 16 Nov 2015 05:21:48 -0500 Message-ID: <1447669296.3081.96.camel@kernel.crashing.org> From: Benjamin Herrenschmidt Date: Mon, 16 Nov 2015 21:21:36 +1100 In-Reply-To: <20151116053405.GH2747@voom.fritz.box> References: <1447201710-10229-1-git-send-email-benh@kernel.crashing.org> <1447201710-10229-14-git-send-email-benh@kernel.crashing.org> <20151116053405.GH2747@voom.fritz.box> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 13/77] ppc: tlbie, tlbia and tlbisync are HV only List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On Mon, 2015-11-16 at 16:34 +1100, David Gibson wrote: > On Wed, Nov 11, 2015 at 11:27:26AM +1100, Benjamin Herrenschmidt > wrote: > > Not that anything remotely recent supports tlbia but ... > >=20 > > Signed-off-by: Benjamin Herrenschmidt > > --- > > =C2=A0target-ppc/translate.c | 6 +++--- > > =C2=A01 file changed, 3 insertions(+), 3 deletions(-) > >=20 > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > > index 10eb9e3..014fe5e 100644 > > --- a/target-ppc/translate.c > > +++ b/target-ppc/translate.c > > @@ -4836,7 +4836,7 @@ static void gen_tlbia(DisasContext *ctx) > > =C2=A0#if defined(CONFIG_USER_ONLY) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0gen_inval_exception(ctx, POWERPC_EXCP_P= RIV_OPC); > > =C2=A0#else > > -=C2=A0=C2=A0=C2=A0=C2=A0if (unlikely(ctx->pr)) { > > +=C2=A0=C2=A0=C2=A0=C2=A0if (unlikely(ctx->pr || !ctx->hv)) { >=20 > If I'm reading your previous patch correctly, ctx->hv won't be set > with in problem state, so I think the ctx->pr check is redundant. Ah you are right. I do have second thoughts about that previous patch now that you mention it however. In the real MSR, HV and PR are independant, I wonder if I'm better off making the check explicit... The reason I did it this way is that afaik, there is no such thing as a usermode hypervisor resource in the architecture, so any hypervisor resource is also a supervisor mode one, but having ctx->hv be 0 when MSR:HV=3D1 + MSR:PR=3D1 might make it easy to write incorrect code in other places when deciding for example how to direct interrupts. I'll need to think a bit more about this one. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0gen_inval_excep= tion(ctx, POWERPC_EXCP_PRIV_OPC); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} > > @@ -4850,7 +4850,7 @@ static void gen_tlbiel(DisasContext *ctx) > > =C2=A0#if defined(CONFIG_USER_ONLY) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0gen_inval_exception(ctx, POWERPC_EXCP_P= RIV_OPC); > > =C2=A0#else > > -=C2=A0=C2=A0=C2=A0=C2=A0if (unlikely(ctx->pr)) { > > +=C2=A0=C2=A0=C2=A0=C2=A0if (unlikely(ctx->pr || !ctx->hv)) { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0gen_inval_excep= tion(ctx, POWERPC_EXCP_PRIV_OPC); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} > > @@ -4864,7 +4864,7 @@ static void gen_tlbie(DisasContext *ctx) > > =C2=A0#if defined(CONFIG_USER_ONLY) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0gen_inval_exception(ctx, POWERPC_EXCP_P= RIV_OPC); > > =C2=A0#else > > -=C2=A0=C2=A0=C2=A0=C2=A0if (unlikely(ctx->pr)) { > > +=C2=A0=C2=A0=C2=A0=C2=A0if (unlikely(ctx->pr || !ctx->hv)) { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0gen_inval_excep= tion(ctx, POWERPC_EXCP_PRIV_OPC); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} >=20