From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45271) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z57oO-00084z-Ah for qemu-devel@nongnu.org; Wed, 17 Jun 2015 03:31:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z57oM-0005Pq-AN for qemu-devel@nongnu.org; Wed, 17 Jun 2015 03:31:04 -0400 Date: Wed, 17 Jun 2015 14:43:24 +1000 From: David Gibson Message-ID: <20150617044324.GR13352@voom.redhat.com> References: <1433478358-993-1-git-send-email-bharata@linux.vnet.ibm.com> <1433478358-993-6-git-send-email-bharata@linux.vnet.ibm.com> <20150615065908.GJ13352@voom.redhat.com> <20150615101509.57063182@thh440s> <20150616054005.GP13352@voom.redhat.com> <20150616083634.1661908d@thh440s> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="pqmPt9oPL4cuP/b5" Content-Disposition: inline In-Reply-To: <20150616083634.1661908d@thh440s> Subject: Re: [Qemu-devel] [PATCH v4 5/8] spapr: Consolidate cpu init code into a routine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, agraf@suse.de, aik@ozlabs.ru, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, Bharata B Rao , nfont@linux.vnet.ibm.com --pqmPt9oPL4cuP/b5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 16, 2015 at 08:36:34AM +0200, Thomas Huth wrote: > On Tue, 16 Jun 2015 15:40:05 +1000 > David Gibson wrote: >=20 > > On Mon, Jun 15, 2015 at 10:15:09AM +0200, Thomas Huth wrote: > > > On Mon, 15 Jun 2015 16:59:08 +1000 > > > David Gibson wrote: > > >=20 > > > > On Fri, Jun 05, 2015 at 09:55:55AM +0530, Bharata B Rao wrote: > > > > > Factor out bits of sPAPR specific CPU initialization code into > > > > > a separate routine so that it can be called from CPU hotplug > > > > > path too. > > > > >=20 > > > > > While at this, use MSR_EP define instead of using 6 directly. > > > >=20 > > > > Don't do this please. MSR[EP] is an obsolete flag from 601. The > > > > MSR[IP] flag that we're controlling here just happened to re-use the > > > > same bit position, so using the existing MSR_EP define is misleadin= g. > > >=20 > > > Actually, I had the same discussion with Bharata already some weeks a= go: > > >=20 > > > http://lists.gnu.org/archive/html/qemu-ppc/2015-05/msg00133.html > > >=20 > > > > A symbolic name is good, but you should create a new one for MSR[IP] > > > > instead. > > >=20 > > > ... and I had to realize that IP =3D EP. IP likely stands for "interr= upt > > > prefix" (I guess), and EP simply means "exception prefix", so just two > > > words for the same meaning. It's just the "on 601" comment in QEMU th= at > > > is completely misleading. So IMHO it should be fine to keep the > > > "MSR_EP" here (and maybe update the comment in cpu.h with a separate > > > patch?). > >=20 > > I don't entirely agree. Yes EP and IP have related functions - it's > > pretty common in ppc history that when an MSR bit is re-used it's for > > something similar (for example IS/IR). But MSR[IP] is still a > > different name from MSR[EP], and I don't know if the semantics are > > identical, though I'm sure they're similar. >=20 > I had a look at the 601 User's Manual, and it says: >=20 > Bit Name Description > 25 EP Exception prefix. The setting of this bit specifies whether an > exception vector offset is prepended with Fs or 0s. >=20 > Then, looking at the 603 User's Manual, it says: >=20 > Bit Name Description > 25 IP Exception prefix. The setting of this bit specifies whether an > exception vector offset is prepended with Fs or 0s. >=20 > So it's the very same bit, just the name has already been changed > between the 601 and 603 already. >=20 > So I think it's either fine to keep the MSR_EP in the patch (and change > the comment for the define in a separate patch?), or to introduce > another define, MSR_IP, with a comment that it is the same as MSR_EP, > just with a different name. I'd still prefer to see the modern name in the #define. > However, I still wonder whether this bit applies to the spapr code at > all since it is not defined in the modern PowerISA spec anymore, as far > as I can see. Hrm.. yeah. I am wondering if it is not architected, but actually present as an implementation specific MSR bit on the modern CPUs. I see that Bharata has reposted leaving the literal 6 as is, which is probably a fair call until we find a consensus 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 --pqmPt9oPL4cuP/b5 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVgPrsAAoJEGw4ysog2bOSvhMP/0oi9GIxYeEgl2aN/MzGCK9S /JZ6xYGUXMTpTBECSx4J0EYzIVN2XqET8g7mYMk+NyAgzr4H6mGLkFXuJnp+OKks ntZEsV/x3K9f+Ffyd14x4vlUt5Q6kwXeFgDoitNwVb6ybZC48BRQS+KDOvHkaEZ8 f783OdgXregkvVgGgb17tGdhW+cSzWi/8fLRsSz2LqjtnmGH/hkXpT/+f8We3SML uEBsLB2MVba1gR3VtU1w0M7Iql3DVQ7AqWkSsMkMa1GUuq9OBizjnl3+rXeoAiKD 45bWG6bq4CoB4b6OfURpnA232dobwk+b1gV5AyrnGpOi7Fut/izBe/SfXbpxrpFK 2wqchQw5JNN4O+p8v6FOpfC7FYiQFZimSVtpXhdqMbwRMONi65gV3D+HGInwxZI3 D7IzUmkGS/CGqJtdbfxCpqAktBWGPhcVK1UC2NPkw4AcPxBSI2CMUJDgnq+AA6TH RYlLG0T8SMmnFrveWbqRHs7bpIhF8xa6z26sFMikwYThjfE3J0/AMwlLyjXNd1G1 eh2AYevFQ7EvRFHoodDFKSATFrclctOskhabIt9BHwd11EEtuj00zI8XNcZiWjlz 0+/9qlsUlnKqKLC5vnGEOV6KU/Sr0x+iKiOvvxWFxkwAXJZyL0OkrHnIarg8kQ8n hCOZojw7IvIYFRCcfic8 =YJU9 -----END PGP SIGNATURE----- --pqmPt9oPL4cuP/b5--