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

[David Gibson]

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.

-- 
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

pgpU35xE4gOho.pgp
Description: PGP signature