Re: [Qemu-devel] [PATCH v4 5/8] spapr: Consolidate cpu init code into a routine

[Thomas Huth]

On Tue, 16 Jun 2015 15:40:05 +1000
David Gibson <address@hidden> 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 <address@hidden> 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.

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.

 Thomas