From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a13Pw-0004EN-3y for qemu-devel@nongnu.org; Mon, 23 Nov 2015 21:33:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a13Pu-0005Us-O9 for qemu-devel@nongnu.org; Mon, 23 Nov 2015 21:33:16 -0500 Date: Tue, 24 Nov 2015 13:22:36 +1100 From: David Gibson Message-ID: <20151124022236.GE26118@voom.fritz.box> References: <1447201710-10229-1-git-send-email-benh@kernel.crashing.org> <1447201710-10229-22-git-send-email-benh@kernel.crashing.org> <20151120074526.GB7118@voom.redhat.com> <1448326304.4574.29.camel@kernel.crashing.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ZInfyf7laFu/Kiw7" Content-Disposition: inline In-Reply-To: <1448326304.4574.29.camel@kernel.crashing.org> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 21/77] ppc: Rework generation of priv and inval interrupts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Herrenschmidt Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org --ZInfyf7laFu/Kiw7 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 24, 2015 at 11:51:44AM +1100, Benjamin Herrenschmidt wrote: > On Fri, 2015-11-20 at 18:45 +1100, David Gibson wrote: > > snip] > > >=A0 /* tlbiel */ > > >=A0 static void gen_tlbiel(DisasContext *ctx) > > >=A0 { > > >=A0 #if defined(CONFIG_USER_ONLY) > > > -=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > +=A0=A0=A0 GEN_PRIV; > > >=A0 #else > > > -=A0=A0=A0 if (unlikely(ctx->pr || !ctx->hv)) { > > > -=A0=A0=A0=A0=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC= ); > > > -=A0=A0=A0=A0=A0=A0=A0 return; > > > -=A0=A0=A0 } > > > +=A0=A0=A0 CHK_SV; > >=20 > > You have CHK_SV here, but the original code checks for HV, as does > > your new code for tlbia and tlbiel, is that right? >=20 > Yes. tlbiel is supervisor accessible (for weird reasons). >=20 > > [snip] > > >=A0 /* tlbsync */ > > >=A0 static void gen_tlbsync(DisasContext *ctx) > > >=A0 { > > >=A0 #if defined(CONFIG_USER_ONLY) > > > -=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > -#else > > > -=A0=A0=A0 if (unlikely(ctx->pr)) { > > > -=A0=A0=A0=A0=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC= ); > > > -=A0=A0=A0=A0=A0=A0=A0 return; > > > -=A0=A0=A0 } > > > +=A0=A0=A0 GEN_PRIV; > > > +#else=A0=A0=A0=20 > > > +=A0=A0=A0 CHK_HV; > > > + > >=20 > > Old code didn't check for HV, mode, but AFAICT it should have, so > > this looks correct. >=20 > Yes, this is a hypervisor instruction. >=20 > > [snip] > > > @@ -5941,18 +5921,16 @@ static void gen_mfapidi(DisasContext *ctx) > > >=A0 static void gen_tlbiva(DisasContext *ctx) > > >=A0 { > > >=A0 #if defined(CONFIG_USER_ONLY) > > > -=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > +=A0=A0=A0 GEN_PRIV; > > >=A0 #else > > >=A0=A0=A0=A0=A0 TCGv t0; > > > -=A0=A0=A0 if (unlikely(ctx->pr)) { > > > -=A0=A0=A0=A0=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC= ); > > > -=A0=A0=A0=A0=A0=A0=A0 return; > > > -=A0=A0=A0 } > > > + > > > +=A0=A0=A0 CHK_SV; > >=20 > > Is the same thing as tlbivax, or some ancient instruction?=A0 AFAICT > > the ISA says tlbivax is hypervisor privileged. >=20 > "tlbiva" is the 4xx variant, there is no hypervisor mode on these > things. >=20 > > >=A0=A0=A0=A0=A0 t0 =3D tcg_temp_new(); > > >=A0=A0=A0=A0=A0 gen_addr_reg_index(ctx, t0); > > >=A0=A0=A0=A0=A0 gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]); > > >=A0=A0=A0=A0=A0 tcg_temp_free(t0); > > > -#endif > > > +#endif /* defined(CONFIG_USER_ONLY) */ > > >=A0 } > >=20 > > [snip] > > >=A0 static void gen_tlbivax_booke206(DisasContext *ctx) > > >=A0 { > > >=A0 #if defined(CONFIG_USER_ONLY) > > > -=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > +=A0=A0=A0 GEN_PRIV; > > >=A0 #else > > >=A0=A0=A0=A0=A0 TCGv t0; > > > -=A0=A0=A0 if (unlikely(ctx->pr)) { > > > -=A0=A0=A0=A0=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC= ); > > > -=A0=A0=A0=A0=A0=A0=A0 return; > > > -=A0=A0=A0 } > > >=A0=20 > > > +=A0=A0=A0 CHK_SV; > >=20 > > ISA says tlbivax is hypervisor privileged when the CPU has a > > hypervisor mode, which I guess booke206 probably doesn't? >=20 > Right so here, the "problem" is that afaik, TCG doesn't implement > the BookE hypervisor mode. So with my limited BookE testing > ability I prefer sticking to a mechanical replacement that matches > the existing code. It can be fixed later if necessary. Fair enough. > > >=A0=A0=A0=A0=A0 t0 =3D tcg_temp_new(); > > >=A0=A0=A0=A0=A0 gen_addr_reg_index(ctx, t0); > > > - > > >=A0=A0=A0=A0=A0 gen_helper_booke206_tlbivax(cpu_env, t0); > > >=A0=A0=A0=A0=A0 tcg_temp_free(t0); > > > -#endif > > > +#endif /* defined(CONFIG_USER_ONLY) */ > > >=A0 } > > >=A0=20 > > >=A0 static void gen_tlbilx_booke206(DisasContext *ctx) > > >=A0 { > > >=A0 #if defined(CONFIG_USER_ONLY) > > > -=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > +=A0=A0=A0 GEN_PRIV; > > >=A0 #else > > >=A0=A0=A0=A0=A0 TCGv t0; > > > -=A0=A0=A0 if (unlikely(ctx->pr)) { > > > -=A0=A0=A0=A0=A0=A0=A0 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC= ); > > > -=A0=A0=A0=A0=A0=A0=A0 return; > > > -=A0=A0=A0 } > > >=A0=20 > > > +=A0=A0=A0 CHK_SV; > >=20 > > And apparently hv vs. sv privilege of tlbilx depends on the EPCR > > register.=A0 Again, may not be relevant for 2.06. >=20 > Well, here too, I basically preserve existing BookE TCG behaviour, > whether it's correct or not. That can be fixed separately if somebody > cares about BookE HV mode. >=20 > > >=A0=A0=A0=A0=A0 t0 =3D tcg_temp_new(); > > >=A0=A0=A0=A0=A0 gen_addr_reg_index(ctx, t0); > > >=A0=20 > > > @@ -6672,7 +6574,7 @@ static void gen_tlbilx_booke206(DisasContext > > *ctx) > > >=A0=A0=A0=A0=A0 } > > >=A0=20 > > >=A0=A0=A0=A0=A0 tcg_temp_free(t0); > > > -#endif > > > +#endif /* defined(CONFIG_USER_ONLY) */ > > >=A0 } > >=20 >=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 --ZInfyf7laFu/Kiw7 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWU8nsAAoJEGw4ysog2bOS/5AP/2y7TkqBSTaOljwDInhweEJ8 V5DvwYjKOajr3KqHV2BhWuMun9YCgUsOJraGE5K9OgvsdZvQ3e1Uo16j+1jwxNDE nwJwnkI06tbR0axOQ6jln1peSJUWU9m/Mbcn4FSSuNrZQFrWWXnksiqEOy1k+vSY 9rrZGzgd0V3UhfT/FmYbmHeUfasAl4zTQGCDQ/DTdW97rfoiHJ6B5zm18BExFHoz 7lc13T6/aKfrk0TyZhFYsEUKcAlAu2aihlSpSDP/j4zVTfMP5cIguJurB7z5F3my CIHkkzfYkwODBHFOd4knSGfJASBGv5jJ3OQmAeeJIBAemiXnU6TtC3jY1uyXx8aL LyEXobo0I8vg9FSthqQLEtYw0kIsjocUXZyJd6QKu5P8hVXYT6WS8R7vTIJWRwOD sHvZfGamxtLp0TA0XCzIHFdPne1RrE/dMU6UTCLWI3eKWSX9cwbvkcMt6rtyfDCr jPrr2Z+zE6FouuWPovtp8MlFIoBX4lxv7Q1TpBqZokIqeOipKurJRoe5lRw17W0w WrVaS0QIW7xLDIodfxf/lHmyHbFuViTDDi13+CD/RKBOnA+4ehuNz5mjYXRuM7u8 I8hxmqhpQqngow30aAdVqc4OkjHj01m7+KTEhGlxO3H0syc/2GE4MjD2o1IBEMHb z0/AXYzZGFXZtlkRX7hC =UEFU -----END PGP SIGNATURE----- --ZInfyf7laFu/Kiw7--