On Tue, Jun 16, 2015 at 08:36:34AM +0200, Thomas Huth wrote: > On Tue, 16 Jun 2015 15:40:05 +1000 > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > On Mon, Jun 15, 2015 at 10:15:09AM +0200, Thomas Huth wrote: > > > On Mon, 15 Jun 2015 16:59:08 +1000 > > > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > > > > > 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. > > > > > > > > > > While at this, use MSR_EP define instead of using 6 directly. > > > > > > > > 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 misleading. > > > > > > Actually, I had the same discussion with Bharata already some weeks ago: > > > > > > http://lists.gnu.org/archive/html/qemu-ppc/2015-05/msg00133.html > > > > > > > A symbolic name is good, but you should create a new one for MSR[IP] > > > > instead. > > > > > > ... and I had to realize that IP = EP. IP likely stands for "interrupt > > > 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 that > > > 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?). > > > > 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. > > I had a look at the 601 User's Manual, and it says: > > Bit Name Description > 25 EP Exception prefix. The setting of this bit specifies whether an > exception vector offset is prepended with Fs or 0s. > > Then, looking at the 603 User's Manual, it says: > > Bit Name Description > 25 IP Exception prefix. The setting of this bit specifies whether an > exception vector offset is prepended with Fs or 0s. > > So it's the very same bit, just the name has already been changed > between the 601 and 603 already. > > 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.
--
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
pgpbAnVazrelg.pgp
Description: PGP signature
