Re: [Qemu-ppc] [PATCH v2 1/6] PPC 85xx: Detect e500v2 / e500mc during runtime

[Scott Wood]

On Thu, 2014-02-06 at 12:40 +0100, Alexander Graf wrote:
> On 04.02.2014, at 02:59, Scott Wood <address@hidden> wrote:
> 
> > On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >> With the qemu-ppce500 machine type we can run the same board with
> >> either an e500v2 or an e500mc core plugged in.
> >> 
> >> This means that the IVOR setup can't be based on compile time decisions,
> >> so instead we have to do a runtime check which CPU generation we're
> >> running on.
> >> 
> >> Signed-off-by: Alexander Graf <address@hidden>
> >> ---
> >> arch/powerpc/cpu/mpc85xx/fixed_ivor.S |   21 ++++++++++++++++-----
> >> 1 file changed, 16 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S 
> >> b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >> index ebbb8c0..635a97e 100644
> >> --- a/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >> +++ b/arch/powerpc/cpu/mpc85xx/fixed_ivor.S
> >> @@ -36,17 +36,25 @@
> >>    SET_IVOR(14, 0x1e0) /* Instruction TLB Error */
> >>    SET_IVOR(15, 0x040) /* Debug */
> >> 
> >> -/* e500v1 & e500v2 only */
> >> -#ifndef CONFIG_E500MC
> >> +  /* Check for CPU */
> >> +  mfpvr   r3
> >> +  srwi    r3, r3, 16
> >> +  /* Compare with e500mc PVR major number */
> >> +  li      r4, 0
> >> +  ori     r4, r4, 0x8023
> >> +  cmpw    r3, r4
> >> +
> >> +  /* e500v1 & e500v2 only */
> >> +  bge     1f
> >>    SET_IVOR(32, 0x200) /* SPE Unavailable */
> >>    SET_IVOR(33, 0x220) /* Embedded FP Data */
> >>    SET_IVOR(34, 0x240) /* Embedded FP Round */
> >> -#endif
> >> +1:
> >> 
> >>    SET_IVOR(35, 0x260) /* Performance monitor */
> >> 
> >> -/* e500mc only */
> >> -#ifdef CONFIG_E500MC
> >> +  /* e500mc only */
> >> +  blt     2f
> >>    SET_IVOR(36, 0x280) /* Processor doorbell */
> >>    SET_IVOR(37, 0x2a0) /* Processor doorbell critical */
> >>    SET_IVOR(38, 0x2c0) /* Guest Processor doorbell */
> >> @@ -54,6 +62,8 @@
> >>    SET_IVOR(40, 0x300) /* Hypervisor system call */
> >>    SET_IVOR(41, 0x320) /* Hypervisor Priviledge */
> >> 
> >> +#ifndef CONFIG_QEMU_E500
> >> +  /* QEMU guests runs in guest mode and can't access GIVORs */
> >>    SET_GIVOR(2, 0x060) /* Guest Data Storage */
> >>    SET_GIVOR(3, 0x080) /* Guest Instruction Storage */
> >>    SET_GIVOR(4, 0x0a0) /* Guest External Input */
> >> @@ -61,3 +71,4 @@
> >>    SET_GIVOR(13, 0x1c0) /* Guest Data TLB Error */
> >>    SET_GIVOR(14, 0x1e0) /* Guest Instruction TLB Error */
> >> #endif
> >> +2:
> > 
> > Again, let's please just remove this entire file.
> 
> I can remove it, but it's a pretty drastic change from the original
> behavior that was introduced 4 1/2 years ago:
> 
>   http://lists.denx.de/pipermail/u-boot/2009-August/058670.html
> 
> I assume the idea was to give OSs one thing less to worry about (IVOR
> setting). If any OS in between 2009 and now relied on that fact, we'll
> break it by removing this code.

Any such OS would already be broken on pre-2009 U-Boot.  Linux doesn't
rely on it.  Neither the code nor the commit message gives a sufficient
rationale, and Kumar didn't answer when asked.  It's incomplete (doesn't
include e6500 IVORs).  It doesn't work on e6500 secondary threads.  The
reset value of these registers is documented as zero, so it's not like
we're just putting things back the way they were.  It only happens
(except on secondary cores) when the bootm/bootz commands are used, not
when using the ELF loader or the go command, nor when using bootm with
IH_TYPE_STANDALONE.

Does anyone know of an OS that actually depends on this?

-Scott