[PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers

[mihai.caraman at freescale.com]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Saturday, July 26, 2014 3:11 AM
> To: Caraman Mihai Claudiu-B02008
> Cc: Alexander Graf; kvm-ppc at vger.kernel.org; kvm at vger.kernel.org;
> linuxppc-dev at lists.ozlabs.org
> Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for
> SPE/FP/AltiVec int numbers
> 
> On Thu, 2014-07-24 at 04:16 -0500, Caraman Mihai Claudiu-B02008 wrote:
> > Scott, Alex's request to define SPE handlers only for e500v2 implies
> changes
> > in 32-bit FSL kernel to have exclusive configurations for e200/e500v2
> and
> > e500mc/e5500. We would probably need something like this, what's your
> take on it?
> 
> That is already a compile-time decision.

Yes, but is not fully implemented.

> 
> > diff --git a/arch/powerpc/kernel/head_fsl_booke.S
> b/arch/powerpc/kernel/head_fsl_booke.S
> > index b497188..9d41015 100644
> > --- a/arch/powerpc/kernel/head_fsl_booke.S
> > +++ b/arch/powerpc/kernel/head_fsl_booke.S
> > @@ -613,6 +613,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> >         mfspr   r10, SPRN_SPRG_RSCRATCH0
> >         b       InstructionStorage
> >
> > +/* Define SPE handlers only for e500v2 and e200 */
> > +#ifndef CONFIG_PPC_E500MC
> >  #ifdef CONFIG_SPE
> >         /* SPE Unavailable */
> >         START_EXCEPTION(SPEUnavailable)
> > @@ -626,7 +628,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> >         EXCEPTION(0x2020, SPE_ALTIVEC_UNAVAIL, SPEUnavailable, \
> >                   unknown_exception, EXC_XFER_EE)
> >  #endif /* CONFIG_SPE */
> > +#endif
> 
> This is redundant:
> 
>         config SPE
>                 bool "SPE Support"
>                 depends on E200 || (E500 && !PPC_E500MC)
>                 default y

I see what you mean, but it's not redundant. Alex was looking to remove SPE
handlers for e500mc+ and the proposal handled !SPE case. In the new
light I find this variant more readable:

+/* Define SPE handlers for e200 and e500v2 */
 #ifdef CONFIG_SPE
        /* SPE Unavailable */
        START_EXCEPTION(SPEUnavailable)
@@ -622,11 +623,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
        b       fast_exception_return
 1:     addi    r3,r1,STACK_FRAME_OVERHEAD
        EXC_XFER_EE_LITE(0x2010, KernelSPE)
-#else
+#elif defined(CONFIG_E200) || \
+      (defined(CONFIG_E500) && !defined(CONFIG_PPC_E500MC))
        EXCEPTION(0x2020, SPE_ALTIVEC_UNAVAIL, SPEUnavailable, \
                  unknown_exception, EXC_XFER_EE)
 #endif /* CONFIG_SPE */

> 
> > diff --git a/arch/powerpc/kernel/cputable.c
> b/arch/powerpc/kernel/cputable.c
> > index c1faade..3ab65c2 100644
> > --- a/arch/powerpc/kernel/cputable.c
> > +++ b/arch/powerpc/kernel/cputable.c
> > @@ -2030,6 +2030,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
> >  #endif /* CONFIG_PPC32 */
> >  #ifdef CONFIG_E500
> >  #ifdef CONFIG_PPC32
> > +#ifndef CONFIG_PPC_E500MC
> >         {       /* e500 */
> >                 .pvr_mask               = 0xffff0000,
> >                 .pvr_value              = 0x80200000,
> > @@ -2069,6 +2070,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
> >                 .machine_check          = machine_check_e500,
> >                 .platform               = "ppc8548",
> >         },
> > +#endif /* !CONFIG_PPC_E500MC */
> >         {       /* e500mc */
> >                 .pvr_mask               = 0xffff0000,
> >                 .pvr_value              = 0x80230000,
> >
> 
> This looks a bit strange -- e500v2 gets excluded if PPC_E500MC, but
> e500mc gets included in !PPC_E500MC?

Right, my main purpose was to get rid of __setup_e500_ivors on PPC_E500MC
which refers SPEUnavailable. I will add an #else to exclude e500mc.

The "generic E500 PPC" default cpu advertises PPC_FEATURE_HAS_SPE_COMP.
Do we want to excluded it if PPC_E500MC? Is this cpu useful without cpu_setup()
and if so why don't we also have a default for 64-bit?

-Mike