diff mbox

[RFC,03/26] target/ppc/POWER9: add POWERPC_EXCP_POWER9

Message ID 1499274819-15607-4-git-send-email-clg@kaod.org
State New
Headers show

Commit Message

Cédric Le Goater July 5, 2017, 5:13 p.m. UTC
Prepare ground for the new exception model XIVE of POWER9.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/cpu-qom.h        | 2 ++
 target/ppc/excp_helper.c    | 9 ++++++---
 target/ppc/translate.c      | 3 ++-
 target/ppc/translate_init.c | 2 +-
 4 files changed, 11 insertions(+), 5 deletions(-)

Comments

David Gibson July 10, 2017, 10:26 a.m. UTC | #1
On Wed, Jul 05, 2017 at 07:13:16PM +0200, Cédric Le Goater wrote:
> Prepare ground for the new exception model XIVE of POWER9.

I'm a bit confused by this.  The excp_model is about the CPU core's
irq model, not the external irq controller's.

Now.. I could imagine the POWER9 having a different core model that
came along with XIVE, but I can't see this new model being used for
anything anywhere in the rest of the series.

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  target/ppc/cpu-qom.h        | 2 ++
>  target/ppc/excp_helper.c    | 9 ++++++---
>  target/ppc/translate.c      | 3 ++-
>  target/ppc/translate_init.c | 2 +-
>  4 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index d0cf6ca2a971..d7b78cf3f71c 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -132,6 +132,8 @@ enum powerpc_excp_t {
>      POWERPC_EXCP_POWER7,
>      /* POWER8 exception model           */
>      POWERPC_EXCP_POWER8,
> +    /* POWER9 exception model           */
> +    POWERPC_EXCP_POWER9,
>  };
>  
>  /*****************************************************************************/
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 3a9f0861e773..dc7dff36a580 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -148,9 +148,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>       */
>  #if defined(TARGET_PPC64)
>      if (excp_model == POWERPC_EXCP_POWER7 ||
> -        excp_model == POWERPC_EXCP_POWER8) {
> +        excp_model == POWERPC_EXCP_POWER8 ||
> +        excp_model == POWERPC_EXCP_POWER9) {
>          lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> -        if (excp_model == POWERPC_EXCP_POWER8) {
> +        if (excp_model == POWERPC_EXCP_POWER8 ||
> +            excp_model == POWERPC_EXCP_POWER9) {
>              ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
>          } else {
>              ail = 0;
> @@ -651,7 +653,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>          if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
>              new_msr |= (target_ulong)1 << MSR_LE;
>          }
> -    } else if (excp_model == POWERPC_EXCP_POWER8) {
> +    } else if (excp_model == POWERPC_EXCP_POWER8 ||
> +               excp_model == POWERPC_EXCP_POWER9) {
>          if (new_msr & MSR_HVB) {
>              if (env->spr[SPR_HID0] & HID0_HILE) {
>                  new_msr |= (target_ulong)1 << MSR_LE;
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index c0cd64d927c2..2d8c1b9e6836 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7064,7 +7064,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>  
>  #if defined(TARGET_PPC64)
>      if (env->excp_model == POWERPC_EXCP_POWER7 ||
> -        env->excp_model == POWERPC_EXCP_POWER8) {
> +        env->excp_model == POWERPC_EXCP_POWER8 ||
> +        env->excp_model == POWERPC_EXCP_POWER9) {
>          cpu_fprintf(f, "HSRR0 " TARGET_FMT_lx " HSRR1 " TARGET_FMT_lx "\n",
>                      env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
>      }
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index 53aff5a7b734..b8c7b8150318 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -8962,7 +8962,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>      pcc->sps = &POWER7_POWER8_sps;
>      pcc->radix_page_info = &POWER9_radix_page_info;
>  #endif
> -    pcc->excp_model = POWERPC_EXCP_POWER8;
> +    pcc->excp_model = POWERPC_EXCP_POWER9;
>      pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
>      pcc->bfd_mach = bfd_mach_ppc64;
>      pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
Cédric Le Goater July 10, 2017, 12:49 p.m. UTC | #2
On 07/10/2017 12:26 PM, David Gibson wrote:
> On Wed, Jul 05, 2017 at 07:13:16PM +0200, Cédric Le Goater wrote:
>> Prepare ground for the new exception model XIVE of POWER9.
> 
> I'm a bit confused by this.  The excp_model is about the CPU core's
> irq model, not the external irq controller's.

yes this is true, but the POWER9 CPU is the only criteria we have 
to distinguish a machine supporting XIVE and XICS from one only 
supporting XICS.

My idea was to use this flag to activate the OV5_XIVE_EXPLOIT bit 
in ibm,arch-vec-5-platform-support ov5_platform, like this is done
for the MMU. See spapr_dt_ov5_platform_support()
 
> Now.. I could imagine the POWER9 having a different core model that
> came along with XIVE, but I can't see this new model being used for
> anything anywhere in the rest of the series.

See patch 26. But, maybe, I am taking a shortcut and we need another
family of flags. 

Thanks,

C. 

>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  target/ppc/cpu-qom.h        | 2 ++
>>  target/ppc/excp_helper.c    | 9 ++++++---
>>  target/ppc/translate.c      | 3 ++-
>>  target/ppc/translate_init.c | 2 +-
>>  4 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
>> index d0cf6ca2a971..d7b78cf3f71c 100644
>> --- a/target/ppc/cpu-qom.h
>> +++ b/target/ppc/cpu-qom.h
>> @@ -132,6 +132,8 @@ enum powerpc_excp_t {
>>      POWERPC_EXCP_POWER7,
>>      /* POWER8 exception model           */
>>      POWERPC_EXCP_POWER8,
>> +    /* POWER9 exception model           */
>> +    POWERPC_EXCP_POWER9,
>>  };
>>  
>>  /*****************************************************************************/
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 3a9f0861e773..dc7dff36a580 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -148,9 +148,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>>       */
>>  #if defined(TARGET_PPC64)
>>      if (excp_model == POWERPC_EXCP_POWER7 ||
>> -        excp_model == POWERPC_EXCP_POWER8) {
>> +        excp_model == POWERPC_EXCP_POWER8 ||
>> +        excp_model == POWERPC_EXCP_POWER9) {
>>          lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>> -        if (excp_model == POWERPC_EXCP_POWER8) {
>> +        if (excp_model == POWERPC_EXCP_POWER8 ||
>> +            excp_model == POWERPC_EXCP_POWER9) {
>>              ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
>>          } else {
>>              ail = 0;
>> @@ -651,7 +653,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>>          if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
>>              new_msr |= (target_ulong)1 << MSR_LE;
>>          }
>> -    } else if (excp_model == POWERPC_EXCP_POWER8) {
>> +    } else if (excp_model == POWERPC_EXCP_POWER8 ||
>> +               excp_model == POWERPC_EXCP_POWER9) {
>>          if (new_msr & MSR_HVB) {
>>              if (env->spr[SPR_HID0] & HID0_HILE) {
>>                  new_msr |= (target_ulong)1 << MSR_LE;
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index c0cd64d927c2..2d8c1b9e6836 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -7064,7 +7064,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>>  
>>  #if defined(TARGET_PPC64)
>>      if (env->excp_model == POWERPC_EXCP_POWER7 ||
>> -        env->excp_model == POWERPC_EXCP_POWER8) {
>> +        env->excp_model == POWERPC_EXCP_POWER8 ||
>> +        env->excp_model == POWERPC_EXCP_POWER9) {
>>          cpu_fprintf(f, "HSRR0 " TARGET_FMT_lx " HSRR1 " TARGET_FMT_lx "\n",
>>                      env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
>>      }
>> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
>> index 53aff5a7b734..b8c7b8150318 100644
>> --- a/target/ppc/translate_init.c
>> +++ b/target/ppc/translate_init.c
>> @@ -8962,7 +8962,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>>      pcc->sps = &POWER7_POWER8_sps;
>>      pcc->radix_page_info = &POWER9_radix_page_info;
>>  #endif
>> -    pcc->excp_model = POWERPC_EXCP_POWER8;
>> +    pcc->excp_model = POWERPC_EXCP_POWER9;
>>      pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
>>      pcc->bfd_mach = bfd_mach_ppc64;
>>      pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>
Benjamin Herrenschmidt July 10, 2017, 9 p.m. UTC | #3
On Mon, 2017-07-10 at 14:49 +0200, Cédric Le Goater wrote:
> On 07/10/2017 12:26 PM, David Gibson wrote:
> > On Wed, Jul 05, 2017 at 07:13:16PM +0200, Cédric Le Goater wrote:
> > > Prepare ground for the new exception model XIVE of POWER9.
> > 
> > I'm a bit confused by this.  The excp_model is about the CPU core's
> > irq model, not the external irq controller's.
> 
> yes this is true, but the POWER9 CPU is the only criteria we have 
> to distinguish a machine supporting XIVE and XICS from one only 
> supporting XICS.

Why ? I don't understand.

We do want an EXCP_POWER9 for other things, like the fact that we have
a separate interrupt input for hypervisor, with associated vectors
etc...  but that still doesn't relate to what interrupt controller is
there.

> My idea was to use this flag to activate the OV5_XIVE_EXPLOIT bit 
> in ibm,arch-vec-5-platform-support ov5_platform, like this is done
> for the MMU. See spapr_dt_ov5_platform_support()

I disagree, the MMU is in the core, the XIVE isn't. It would be
possibly to make a P9 core if a XICS in theory :-)

> > Now.. I could imagine the POWER9 having a different core model that
> > came along with XIVE, but I can't see this new model being used for
> > anything anywhere in the rest of the series.
> 
> See patch 26. But, maybe, I am taking a shortcut and we need another
> family of flags. 

Or just some kind of enum for the interrupt controller, how do we do
with OpenPIC vs. XICS already ? Old POWER3 had OpenPIC.

> Thanks,
> 
> C. 
> 
> > > 
> > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > ---
> > >  target/ppc/cpu-qom.h        | 2 ++
> > >  target/ppc/excp_helper.c    | 9 ++++++---
> > >  target/ppc/translate.c      | 3 ++-
> > >  target/ppc/translate_init.c | 2 +-
> > >  4 files changed, 11 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> > > index d0cf6ca2a971..d7b78cf3f71c 100644
> > > --- a/target/ppc/cpu-qom.h
> > > +++ b/target/ppc/cpu-qom.h
> > > @@ -132,6 +132,8 @@ enum powerpc_excp_t {
> > >      POWERPC_EXCP_POWER7,
> > >      /* POWER8 exception model           */
> > >      POWERPC_EXCP_POWER8,
> > > +    /* POWER9 exception model           */
> > > +    POWERPC_EXCP_POWER9,
> > >  };
> > >  
> > >  /*****************************************************************************/
> > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > > index 3a9f0861e773..dc7dff36a580 100644
> > > --- a/target/ppc/excp_helper.c
> > > +++ b/target/ppc/excp_helper.c
> > > @@ -148,9 +148,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> > >       */
> > >  #if defined(TARGET_PPC64)
> > >      if (excp_model == POWERPC_EXCP_POWER7 ||
> > > -        excp_model == POWERPC_EXCP_POWER8) {
> > > +        excp_model == POWERPC_EXCP_POWER8 ||
> > > +        excp_model == POWERPC_EXCP_POWER9) {
> > >          lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> > > -        if (excp_model == POWERPC_EXCP_POWER8) {
> > > +        if (excp_model == POWERPC_EXCP_POWER8 ||
> > > +            excp_model == POWERPC_EXCP_POWER9) {
> > >              ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
> > >          } else {
> > >              ail = 0;
> > > @@ -651,7 +653,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> > >          if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
> > >              new_msr |= (target_ulong)1 << MSR_LE;
> > >          }
> > > -    } else if (excp_model == POWERPC_EXCP_POWER8) {
> > > +    } else if (excp_model == POWERPC_EXCP_POWER8 ||
> > > +               excp_model == POWERPC_EXCP_POWER9) {
> > >          if (new_msr & MSR_HVB) {
> > >              if (env->spr[SPR_HID0] & HID0_HILE) {
> > >                  new_msr |= (target_ulong)1 << MSR_LE;
> > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > > index c0cd64d927c2..2d8c1b9e6836 100644
> > > --- a/target/ppc/translate.c
> > > +++ b/target/ppc/translate.c
> > > @@ -7064,7 +7064,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
> > >  
> > >  #if defined(TARGET_PPC64)
> > >      if (env->excp_model == POWERPC_EXCP_POWER7 ||
> > > -        env->excp_model == POWERPC_EXCP_POWER8) {
> > > +        env->excp_model == POWERPC_EXCP_POWER8 ||
> > > +        env->excp_model == POWERPC_EXCP_POWER9) {
> > >          cpu_fprintf(f, "HSRR0 " TARGET_FMT_lx " HSRR1 " TARGET_FMT_lx "\n",
> > >                      env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
> > >      }
> > > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > > index 53aff5a7b734..b8c7b8150318 100644
> > > --- a/target/ppc/translate_init.c
> > > +++ b/target/ppc/translate_init.c
> > > @@ -8962,7 +8962,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
> > >      pcc->sps = &POWER7_POWER8_sps;
> > >      pcc->radix_page_info = &POWER9_radix_page_info;
> > >  #endif
> > > -    pcc->excp_model = POWERPC_EXCP_POWER8;
> > > +    pcc->excp_model = POWERPC_EXCP_POWER9;
> > >      pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
> > >      pcc->bfd_mach = bfd_mach_ppc64;
> > >      pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
Cédric Le Goater July 11, 2017, 9:01 a.m. UTC | #4
On 07/10/2017 11:00 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2017-07-10 at 14:49 +0200, Cédric Le Goater wrote:
>> On 07/10/2017 12:26 PM, David Gibson wrote:
>>> On Wed, Jul 05, 2017 at 07:13:16PM +0200, Cédric Le Goater wrote:
>>>> Prepare ground for the new exception model XIVE of POWER9.
>>>
>>> I'm a bit confused by this.  The excp_model is about the CPU core's
>>> irq model, not the external irq controller's.
>>
>> yes this is true, but the POWER9 CPU is the only criteria we have 
>> to distinguish a machine supporting XIVE and XICS from one only 
>> supporting XICS.
> 
> Why ? I don't understand.
> 
> We do want an EXCP_POWER9 for other things, like the fact that we have
> a separate interrupt input for hypervisor, with associated vectors
> etc...  but that still doesn't relate to what interrupt controller is
> there.
> 
>> My idea was to use this flag to activate the OV5_XIVE_EXPLOIT bit 
>> in ibm,arch-vec-5-platform-support ov5_platform, like this is done
>> for the MMU. See spapr_dt_ov5_platform_support()
> 
> I disagree, the MMU is in the core, the XIVE isn't. It would be
> possibly to make a P9 core if a XICS in theory :-)

ok. I understand. We could even "build" one in QEMU. HW would be 
another story ... 

So should XIVE support be a sPAPR machine property only enabled if 
'cpu_model' matches "POWER9.*" ? The XICS/XIVE initialization is done 
quite early in the machine init so this needs some checks.

>>> Now.. I could imagine the POWER9 having a different core model that
>>> came along with XIVE, but I can't see this new model being used for
>>> anything anywhere in the rest of the series.
>>
>> See patch 26. But, maybe, I am taking a shortcut and we need another
>> family of flags. 
> 
> Or just some kind of enum for the interrupt controller, how do we do
> with OpenPIC vs. XICS already ? Old POWER3 had OpenPIC.

AFAICT, we don't have such a CPU in QEMU/ppc. 


We could use some extra flag to change the ICS behavior. The path I am
taking duplicates the ICS code but in real, we only need to change the
irq handlers. 

Thanks,

C.
David Gibson July 11, 2017, 1:27 p.m. UTC | #5
On Tue, Jul 11, 2017 at 11:01:15AM +0200, Cédric Le Goater wrote:
> On 07/10/2017 11:00 PM, Benjamin Herrenschmidt wrote:
> > On Mon, 2017-07-10 at 14:49 +0200, Cédric Le Goater wrote:
> >> On 07/10/2017 12:26 PM, David Gibson wrote:
> >>> On Wed, Jul 05, 2017 at 07:13:16PM +0200, Cédric Le Goater wrote:
> >>>> Prepare ground for the new exception model XIVE of POWER9.
> >>>
> >>> I'm a bit confused by this.  The excp_model is about the CPU core's
> >>> irq model, not the external irq controller's.
> >>
> >> yes this is true, but the POWER9 CPU is the only criteria we have 
> >> to distinguish a machine supporting XIVE and XICS from one only 
> >> supporting XICS.
> > 
> > Why ? I don't understand.
> > 
> > We do want an EXCP_POWER9 for other things, like the fact that we have
> > a separate interrupt input for hypervisor, with associated vectors
> > etc...  but that still doesn't relate to what interrupt controller is
> > there.
> > 
> >> My idea was to use this flag to activate the OV5_XIVE_EXPLOIT bit 
> >> in ibm,arch-vec-5-platform-support ov5_platform, like this is done
> >> for the MMU. See spapr_dt_ov5_platform_support()
> > 
> > I disagree, the MMU is in the core, the XIVE isn't. It would be
> > possibly to make a P9 core if a XICS in theory :-)
> 
> ok. I understand. We could even "build" one in QEMU. HW would be 
> another story ... 
> 
> So should XIVE support be a sPAPR machine property only enabled if 
> 'cpu_model' matches "POWER9.*" ? The XICS/XIVE initialization is done 
> quite early in the machine init so this needs some checks.

Basically, yes.  The interrupt controller setup is generally something
the machine looks after.  What I'd actually suggest is a machine
parameter for XICS vs. XIVE, whose default value is based on the CPU
model.  Just as we could build a POWER9 with XICS in qemu, we could
build a POWER8 with XIVE.

> 
> >>> Now.. I could imagine the POWER9 having a different core model that
> >>> came along with XIVE, but I can't see this new model being used for
> >>> anything anywhere in the rest of the series.
> >>
> >> See patch 26. But, maybe, I am taking a shortcut and we need another
> >> family of flags. 
> > 
> > Or just some kind of enum for the interrupt controller, how do we do
> > with OpenPIC vs. XICS already ? Old POWER3 had OpenPIC.
> 
> AFAICT, we don't have such a CPU in QEMU/ppc.

More to the point we don't have any machine type for those old POWER3
setups.

> We could use some extra flag to change the ICS behavior. The path I am
> taking duplicates the ICS code but in real, we only need to change the
> irq handlers. 
> 
> Thanks,
> 
> C. 
>
Cédric Le Goater July 11, 2017, 1:52 p.m. UTC | #6
On 07/11/2017 03:27 PM, David Gibson wrote:
> On Tue, Jul 11, 2017 at 11:01:15AM +0200, Cédric Le Goater wrote:
>> On 07/10/2017 11:00 PM, Benjamin Herrenschmidt wrote:
>>> On Mon, 2017-07-10 at 14:49 +0200, Cédric Le Goater wrote:
>>>> On 07/10/2017 12:26 PM, David Gibson wrote:
>>>>> On Wed, Jul 05, 2017 at 07:13:16PM +0200, Cédric Le Goater wrote:
>>>>>> Prepare ground for the new exception model XIVE of POWER9.
>>>>>
>>>>> I'm a bit confused by this.  The excp_model is about the CPU core's
>>>>> irq model, not the external irq controller's.
>>>>
>>>> yes this is true, but the POWER9 CPU is the only criteria we have 
>>>> to distinguish a machine supporting XIVE and XICS from one only 
>>>> supporting XICS.
>>>
>>> Why ? I don't understand.
>>>
>>> We do want an EXCP_POWER9 for other things, like the fact that we have
>>> a separate interrupt input for hypervisor, with associated vectors
>>> etc...  but that still doesn't relate to what interrupt controller is
>>> there.
>>>
>>>> My idea was to use this flag to activate the OV5_XIVE_EXPLOIT bit 
>>>> in ibm,arch-vec-5-platform-support ov5_platform, like this is done
>>>> for the MMU. See spapr_dt_ov5_platform_support()
>>>
>>> I disagree, the MMU is in the core, the XIVE isn't. It would be
>>> possibly to make a P9 core if a XICS in theory :-)
>>
>> ok. I understand. We could even "build" one in QEMU. HW would be 
>> another story ... 
>>
>> So should XIVE support be a sPAPR machine property only enabled if 
>> 'cpu_model' matches "POWER9.*" ? The XICS/XIVE initialization is done 
>> quite early in the machine init so this needs some checks.
> 
> Basically, yes.  The interrupt controller setup is generally something
> the machine looks after.  What I'd actually suggest is a machine
> parameter for XICS vs. XIVE, whose default value is based on the CPU
> model.  

yes. That is what I have been starting to work with now : 
 
+static bool ppc_support_xive(MachineState *machine)
+{
+   PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(first_cpu);
+
+   return pcc->pvr_match(pcc, CPU_POWERPC_POWER9_BASE);
+}

I am using the PVR to catch the cpu 'host' model. We could add 
later on a machine property to disable the XIVE mode.  


> Just as we could build a POWER9 with XICS in qemu, we could build a 
> POWER8 with XIVE.

That might be the case with a POWER9 running in POWER8 compat mode. 
I need to check. 

Thanks,

C. 


>>>>> Now.. I could imagine the POWER9 having a different core model that
>>>>> came along with XIVE, but I can't see this new model being used for
>>>>> anything anywhere in the rest of the series.
>>>>
>>>> See patch 26. But, maybe, I am taking a shortcut and we need another
>>>> family of flags. 
>>>
>>> Or just some kind of enum for the interrupt controller, how do we do
>>> with OpenPIC vs. XICS already ? Old POWER3 had OpenPIC.
>>
>> AFAICT, we don't have such a CPU in QEMU/ppc.
> 
> More to the point we don't have any machine type for those old POWER3
> setups.
> 
>> We could use some extra flag to change the ICS behavior. The path I am
>> taking duplicates the ICS code but in real, we only need to change the
>> irq handlers. 
>>
>> Thanks,
>>
>> C. 
>>
>
Benjamin Herrenschmidt July 11, 2017, 9:20 p.m. UTC | #7
On Tue, 2017-07-11 at 15:52 +0200, Cédric Le Goater wrote:
> 
> > Just as we could build a POWER9 with XICS in qemu, we could build a 
> > POWER8 with XIVE.
> 
> That might be the case with a POWER9 running in POWER8 compat mode. 
> I need to check. 

Yes. In "compat" mode (or more generally if the guest doesn't advertize
support for native XIVE, whether it's in compat or native P9 mode is
irrelevant in fact), we provide the old hcalls and effectively need to
instantiate a XICS in qemu (KVM will exploit the XIVE under the hood
but will make it look like a XICS to qemu ioctl's as well).

Cheers,
Ben.

> Thanks,
> 
> C. 
> 
> 
> > > > > > Now.. I could imagine the POWER9 having a different core model that
> > > > > > came along with XIVE, but I can't see this new model being used for
> > > > > > anything anywhere in the rest of the series.
> > > > > 
> > > > > See patch 26. But, maybe, I am taking a shortcut and we need another
> > > > > family of flags. 
> > > > 
> > > > Or just some kind of enum for the interrupt controller, how do we do
> > > > with OpenPIC vs. XICS already ? Old POWER3 had OpenPIC.
> > > 
> > > AFAICT, we don't have such a CPU in QEMU/ppc.
> > 
> > More to the point we don't have any machine type for those old POWER3
> > setups.
> > 
> > > We could use some extra flag to change the ICS behavior. The path I am
> > > taking duplicates the ICS code but in real, we only need to change the
> > > irq handlers. 
> > > 
> > > Thanks,
> > > 
> > > C. 
> > >
diff mbox

Patch

diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index d0cf6ca2a971..d7b78cf3f71c 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -132,6 +132,8 @@  enum powerpc_excp_t {
     POWERPC_EXCP_POWER7,
     /* POWER8 exception model           */
     POWERPC_EXCP_POWER8,
+    /* POWER9 exception model           */
+    POWERPC_EXCP_POWER9,
 };
 
 /*****************************************************************************/
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 3a9f0861e773..dc7dff36a580 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -148,9 +148,11 @@  static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
      */
 #if defined(TARGET_PPC64)
     if (excp_model == POWERPC_EXCP_POWER7 ||
-        excp_model == POWERPC_EXCP_POWER8) {
+        excp_model == POWERPC_EXCP_POWER8 ||
+        excp_model == POWERPC_EXCP_POWER9) {
         lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
-        if (excp_model == POWERPC_EXCP_POWER8) {
+        if (excp_model == POWERPC_EXCP_POWER8 ||
+            excp_model == POWERPC_EXCP_POWER9) {
             ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
         } else {
             ail = 0;
@@ -651,7 +653,8 @@  static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
             new_msr |= (target_ulong)1 << MSR_LE;
         }
-    } else if (excp_model == POWERPC_EXCP_POWER8) {
+    } else if (excp_model == POWERPC_EXCP_POWER8 ||
+               excp_model == POWERPC_EXCP_POWER9) {
         if (new_msr & MSR_HVB) {
             if (env->spr[SPR_HID0] & HID0_HILE) {
                 new_msr |= (target_ulong)1 << MSR_LE;
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index c0cd64d927c2..2d8c1b9e6836 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7064,7 +7064,8 @@  void ppc_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
 
 #if defined(TARGET_PPC64)
     if (env->excp_model == POWERPC_EXCP_POWER7 ||
-        env->excp_model == POWERPC_EXCP_POWER8) {
+        env->excp_model == POWERPC_EXCP_POWER8 ||
+        env->excp_model == POWERPC_EXCP_POWER9) {
         cpu_fprintf(f, "HSRR0 " TARGET_FMT_lx " HSRR1 " TARGET_FMT_lx "\n",
                     env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
     }
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 53aff5a7b734..b8c7b8150318 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8962,7 +8962,7 @@  POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
     pcc->sps = &POWER7_POWER8_sps;
     pcc->radix_page_info = &POWER9_radix_page_info;
 #endif
-    pcc->excp_model = POWERPC_EXCP_POWER8;
+    pcc->excp_model = POWERPC_EXCP_POWER9;
     pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
     pcc->bfd_mach = bfd_mach_ppc64;
     pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |