From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51378) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a13Py-0004FY-MH for qemu-devel@nongnu.org; Mon, 23 Nov 2015 21:33:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a13Pu-0005Um-FP for qemu-devel@nongnu.org; Mon, 23 Nov 2015 21:33:18 -0500 Date: Tue, 24 Nov 2015 13:22:14 +1100 From: David Gibson Message-ID: <20151124022214.GD26118@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> <1448325876.4574.24.camel@kernel.crashing.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="KdquIMZPjGJQvRdI" Content-Disposition: inline In-Reply-To: <1448325876.4574.24.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 --KdquIMZPjGJQvRdI Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 24, 2015 at 11:44:36AM +1100, Benjamin Herrenschmidt wrote: > On Fri, 2015-11-20 at 18:45 +1100, David Gibson wrote: > >=A0 > > So, I'm not 100% following the logic below, but it looks like the > > existing code used SPR_NOACCESS to mark things which generated a > > privilege exception compared to NULL for things which generated an > > invalid instruction exception.=A0=A0Using that encoding, can you simpli= fy > > the logic here?=A0=A0Alternatively can you use the logic here to avoid > > the SPR_NOACESS encoding? >=20 > Well, so the SPR_NOACCESS has to do with how you react to a known SPR > who has explicit access permissions. The logic below is described in > the ISA for an unknown SPR number. >=20 > I don't know whether the access permission of "known" SPRs always > honor the 0x10 bit trick, and changing that in qemu would be a > fairly large patch. So I'd rather stick to the logic here for > "unknown" SPRs which matches the ISA definition. >=20 > I'll update the patch though for arch 2.07 as it defines a few > reserved SPRs as no-ops. Ok, that makes sense. > However: >=20 > > > -=A0=A0=A0=A0=A0=A0=A0=A0gen_inval_exception(ctx, POWERPC_EXCP_INVAL_= SPR); > > > + > > > +=A0=A0=A0=A0=A0=A0=A0=A0/* The behaviour depends on MSR:PR and SPR# = bit 0x10, > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0* it can generate a priv, a hv emu or a n= o-op > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0*/ > > > +=A0=A0=A0=A0=A0=A0=A0=A0if (sprn & 0x10) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (ctx->pr) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0gen_priv_exception(c= tx, POWERPC_EXCP_INVAL_SPR); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0} > > > +=A0=A0=A0=A0=A0=A0=A0=A0} else { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (ctx->pr || sprn =3D=3D 0 || = sprn =3D=3D 4 || sprn =3D=3D 5 || > > > sprn =3D=3D 6) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0gen_hvpriv_exception= (ctx, POWERPC_EXCP_INVAL_SPR); > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0} > > > +=A0=A0=A0=A0=A0=A0=A0=A0} > > > +#if !defined(CONFIG_USER_ONLY) > > > +=A0=A0=A0=A0=A0=A0=A0=A0/* HV priv */ > > > +=A0=A0=A0=A0=A0=A0=A0=A0if (ctx->spr_cb[sprn].hea_read) { > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0gen_priv_exception(ctx, POWERPC_= EXCP_INVAL_SPR); > > > +=A0=A0=A0=A0=A0=A0=A0=A0} >=20 > That latest bit is bogus. >=20 > > If you're in PR mode, and it's an SPR with an hea_read function and > > has the 0x10 bit set, won't this call gen_priv_exception twice? >=20 > Yes, I've removed it. It should be handled by the SPR_NOACCESS. >=20 > > I also see no path here which will call gen_inval_exception(), is > > that > > right?=A0=A0If you're in HV mode and it's a truly invalid SPRN, isn't > > that > > what you'd want? >=20 > No, the ISA says it's a nop. Huh, ok. Some comments referencing the ISA might be useful here. --=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 --KdquIMZPjGJQvRdI Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWU8nWAAoJEGw4ysog2bOS/bwQAMfGnxgYM7Jp2LcitHw54SVr oEiNPdqPf/Wa2eKr1kvw6hRRKn+iYoI2dv7IORuY4iMbHL6KRxNhb8jrQQJuwZa0 zjXMsejZz57c8mLyM4xMf+HO4CBjTWO6iZu4d5pR1XZUOWIZM9eZPhyXovStnx+O 8Ob8B3tT4ycYjobws1EOR9PJM/C04mqciqEV/USflUqt4/NrrljOROlxj6s2BNBs 7N7Zfdv97j98OGrDvdiHyALH4P7zpQSmf28Z5mtOyh8xtRixO+82MRgOc6/Xe6yR /R0AorhJX3KAq5WK2diy988gY2xZGH89j4PLtx6fvYqIvOTXFzWpTyoi6OKeKaF1 dOr+yFdsz1hrE7WIUDvMXEiokYoZiUvO3wuAQyOjDlGfAKdKF1LIndPJwA/0BZlp S/Dsl4q61MnhG9Wyy1FLvz9CmmXbzmiIu99eghewpa8OzlUOH+NYNeDAMF0hQlll dppauFOUyq+xf50+Icp7O7K3RUKxt+ll5sSWLoEQ/W5DSPypSX98Y5vUts36I6oh SrIDXoK9Moy9/o5eZyXgR09KMNhaG4I3shp88yC3l9T0JaU5qA1enHpEvYSHb2Z5 uGIjFJ+dlu+W9wf1WLIrLdgsjVzJPb/3si4rUQ4nc67k2E/0RxEEL8XQCUqHs1Yd qkl2N/kPsfat0zX6xf5K =MmO8 -----END PGP SIGNATURE----- --KdquIMZPjGJQvRdI--