diff mbox series

[v8,09/12] spapr: set the interrupt presenter at reset

Message ID 20181211223823.13770-10-clg@kaod.org
State New
Headers show
Series ppc: support for the XIVE interrupt controller (POWER9) | expand

Commit Message

Cédric Le Goater Dec. 11, 2018, 10:38 p.m. UTC
Currently, the interrupt presenter of the vCPU is set at realize
time. Setting it at reset will become necessary when the new machine
supporting both interrupt modes is introduced. In this machine, the
interrupt mode is chosen at CAS time and activated after a reset.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v7:

 - introduced spapr_irq_reset_xics(). 

 include/hw/ppc/spapr_cpu_core.h |  2 ++
 hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
 hw/ppc/spapr_irq.c              | 13 +++++++++++++
 3 files changed, 41 insertions(+)

Comments

Cédric Le Goater Dec. 13, 2018, 12:52 p.m. UTC | #1
On 12/11/18 11:38 PM, Cédric Le Goater wrote:
> Currently, the interrupt presenter of the vCPU is set at realize
> time. Setting it at reset will become necessary when the new machine
> supporting both interrupt modes is introduced. In this machine, the
> interrupt mode is chosen at CAS time and activated after a reset.

I think we need a second '->intc' pointer specific to XIVE instead. 
How about ->intc_xive ?  

We could just drop this patch and easly get rid of the XiveTCTX object 
leak in spapr_unrealize_vcpu().

C. 

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v7:
> 
>  - introduced spapr_irq_reset_xics(). 
> 
>  include/hw/ppc/spapr_cpu_core.h |  2 ++
>  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
>  hw/ppc/spapr_irq.c              | 13 +++++++++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 9e2821e4b31f..fc8ea9021656 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU *cpu)
>      return (sPAPRCPUState *)cpu->machine_data;
>  }
>  
> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
> +
>  #endif
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 82666436e9b4..afc5d4f4e739 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -397,3 +397,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>  };
>  
>  DEFINE_TYPES(spapr_cpu_core_type_infos)
> +
> +typedef struct ForeachFindIntCArgs {
> +    const char *intc_type;
> +    Object *intc;
> +} ForeachFindIntCArgs;
> +
> +static int spapr_cpu_core_find_intc(Object *child, void *opaque)
> +{
> +    ForeachFindIntCArgs *args = opaque;
> +
> +    if (object_dynamic_cast(child, args->intc_type)) {
> +        args->intc = child;
> +    }
> +
> +    return args->intc != NULL;
> +}
> +
> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
> +{
> +    ForeachFindIntCArgs args = { intc_type, NULL };
> +
> +    object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args);
> +    g_assert(args.intc);
> +
> +    cpu->intc = args.intc;
> +}
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 0999a2b2d69c..814500f22d34 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -12,6 +12,7 @@
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "hw/ppc/spapr.h"
> +#include "hw/ppc/spapr_cpu_core.h"
>  #include "hw/ppc/spapr_xive.h"
>  #include "hw/ppc/xics.h"
>  #include "sysemu/kvm.h"
> @@ -208,6 +209,15 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
>      return 0;
>  }
>  
> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> +{
> +   CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
> +    }
> +}
> +
>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -225,6 +235,7 @@ sPAPRIrq spapr_irq_xics = {
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>      .post_load   = spapr_irq_post_load_xics,
> +    .reset       = spapr_irq_reset_xics,
>  };
>  
>  /*
> @@ -325,6 +336,8 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>      CPU_FOREACH(cs) {
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>  
> +        spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX);
> +
>          /* (TCG) Set the OS CAM line of the thread interrupt context. */
>          spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
>      }
>
David Gibson Dec. 17, 2018, 5:37 a.m. UTC | #2
On Tue, Dec 11, 2018 at 11:38:20PM +0100, Cédric Le Goater wrote:
> Currently, the interrupt presenter of the vCPU is set at realize
> time. Setting it at reset will become necessary when the new machine
> supporting both interrupt modes is introduced. In this machine, the
> interrupt mode is chosen at CAS time and activated after a reset.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Ugly, but necessary.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
>  Changes since v7:
> 
>  - introduced spapr_irq_reset_xics(). 
> 
>  include/hw/ppc/spapr_cpu_core.h |  2 ++
>  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
>  hw/ppc/spapr_irq.c              | 13 +++++++++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 9e2821e4b31f..fc8ea9021656 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU *cpu)
>      return (sPAPRCPUState *)cpu->machine_data;
>  }
>  
> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
> +
>  #endif
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 82666436e9b4..afc5d4f4e739 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -397,3 +397,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>  };
>  
>  DEFINE_TYPES(spapr_cpu_core_type_infos)
> +
> +typedef struct ForeachFindIntCArgs {
> +    const char *intc_type;
> +    Object *intc;
> +} ForeachFindIntCArgs;
> +
> +static int spapr_cpu_core_find_intc(Object *child, void *opaque)
> +{
> +    ForeachFindIntCArgs *args = opaque;
> +
> +    if (object_dynamic_cast(child, args->intc_type)) {
> +        args->intc = child;
> +    }
> +
> +    return args->intc != NULL;
> +}
> +
> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
> +{
> +    ForeachFindIntCArgs args = { intc_type, NULL };
> +
> +    object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args);
> +    g_assert(args.intc);
> +
> +    cpu->intc = args.intc;
> +}
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 0999a2b2d69c..814500f22d34 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -12,6 +12,7 @@
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "hw/ppc/spapr.h"
> +#include "hw/ppc/spapr_cpu_core.h"
>  #include "hw/ppc/spapr_xive.h"
>  #include "hw/ppc/xics.h"
>  #include "sysemu/kvm.h"
> @@ -208,6 +209,15 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
>      return 0;
>  }
>  
> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> +{
> +   CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
> +    }
> +}
> +
>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -225,6 +235,7 @@ sPAPRIrq spapr_irq_xics = {
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>      .post_load   = spapr_irq_post_load_xics,
> +    .reset       = spapr_irq_reset_xics,
>  };
>  
>  /*
> @@ -325,6 +336,8 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>      CPU_FOREACH(cs) {
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>  
> +        spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX);
> +
>          /* (TCG) Set the OS CAM line of the thread interrupt context. */
>          spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
>      }
David Gibson Dec. 17, 2018, 6:01 a.m. UTC | #3
On Thu, Dec 13, 2018 at 01:52:14PM +0100, Cédric Le Goater wrote:
> On 12/11/18 11:38 PM, Cédric Le Goater wrote:
> > Currently, the interrupt presenter of the vCPU is set at realize
> > time. Setting it at reset will become necessary when the new machine
> > supporting both interrupt modes is introduced. In this machine, the
> > interrupt mode is chosen at CAS time and activated after a reset.
> 
> I think we need a second '->intc' pointer specific to XIVE instead. 
> How about ->intc_xive ?

Ah, interesting.  If we're going to need that, we might as well go
to specific ->icp and ->tctx pointers with the right types.

I don't particularly like having those machine specific pointers
within the cpu structure, but we can look at fixing that later by
reviving my patches to move various machine specific stuff within the
cpu into a separate sub-allocation.

> 
> We could just drop this patch and easly get rid of the XiveTCTX object 
> leak in spapr_unrealize_vcpu().
> 
> C. 
> 
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> > 
> >  Changes since v7:
> > 
> >  - introduced spapr_irq_reset_xics(). 
> > 
> >  include/hw/ppc/spapr_cpu_core.h |  2 ++
> >  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
> >  hw/ppc/spapr_irq.c              | 13 +++++++++++++
> >  3 files changed, 41 insertions(+)
> > 
> > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> > index 9e2821e4b31f..fc8ea9021656 100644
> > --- a/include/hw/ppc/spapr_cpu_core.h
> > +++ b/include/hw/ppc/spapr_cpu_core.h
> > @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU *cpu)
> >      return (sPAPRCPUState *)cpu->machine_data;
> >  }
> >  
> > +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
> > +
> >  #endif
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index 82666436e9b4..afc5d4f4e739 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -397,3 +397,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
> >  };
> >  
> >  DEFINE_TYPES(spapr_cpu_core_type_infos)
> > +
> > +typedef struct ForeachFindIntCArgs {
> > +    const char *intc_type;
> > +    Object *intc;
> > +} ForeachFindIntCArgs;
> > +
> > +static int spapr_cpu_core_find_intc(Object *child, void *opaque)
> > +{
> > +    ForeachFindIntCArgs *args = opaque;
> > +
> > +    if (object_dynamic_cast(child, args->intc_type)) {
> > +        args->intc = child;
> > +    }
> > +
> > +    return args->intc != NULL;
> > +}
> > +
> > +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
> > +{
> > +    ForeachFindIntCArgs args = { intc_type, NULL };
> > +
> > +    object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args);
> > +    g_assert(args.intc);
> > +
> > +    cpu->intc = args.intc;
> > +}
> > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> > index 0999a2b2d69c..814500f22d34 100644
> > --- a/hw/ppc/spapr_irq.c
> > +++ b/hw/ppc/spapr_irq.c
> > @@ -12,6 +12,7 @@
> >  #include "qemu/error-report.h"
> >  #include "qapi/error.h"
> >  #include "hw/ppc/spapr.h"
> > +#include "hw/ppc/spapr_cpu_core.h"
> >  #include "hw/ppc/spapr_xive.h"
> >  #include "hw/ppc/xics.h"
> >  #include "sysemu/kvm.h"
> > @@ -208,6 +209,15 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
> >      return 0;
> >  }
> >  
> > +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> > +{
> > +   CPUState *cs;
> > +
> > +    CPU_FOREACH(cs) {
> > +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
> > +    }
> > +}
> > +
> >  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
> >  #define SPAPR_IRQ_XICS_NR_MSIS     \
> >      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> > @@ -225,6 +235,7 @@ sPAPRIrq spapr_irq_xics = {
> >      .dt_populate = spapr_dt_xics,
> >      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> >      .post_load   = spapr_irq_post_load_xics,
> > +    .reset       = spapr_irq_reset_xics,
> >  };
> >  
> >  /*
> > @@ -325,6 +336,8 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
> >      CPU_FOREACH(cs) {
> >          PowerPCCPU *cpu = POWERPC_CPU(cs);
> >  
> > +        spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX);
> > +
> >          /* (TCG) Set the OS CAM line of the thread interrupt context. */
> >          spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
> >      }
> > 
>
Cédric Le Goater Dec. 17, 2018, 10:41 p.m. UTC | #4
On 12/17/18 7:01 AM, David Gibson wrote:
> On Thu, Dec 13, 2018 at 01:52:14PM +0100, Cédric Le Goater wrote:
>> On 12/11/18 11:38 PM, Cédric Le Goater wrote:
>>> Currently, the interrupt presenter of the vCPU is set at realize
>>> time. Setting it at reset will become necessary when the new machine
>>> supporting both interrupt modes is introduced. In this machine, the
>>> interrupt mode is chosen at CAS time and activated after a reset.
>>
>> I think we need a second '->intc' pointer specific to XIVE instead. 
>> How about ->intc_xive ?
> 
> Ah, interesting.  If we're going to need that, we might as well go
> to specific ->icp and ->tctx pointers with the right types.

Hmm, PowerPCCPU is defined under target/ppc and adding the type might 
add some noise. I will check.

> I don't particularly like having those machine specific pointers
> within the cpu structure, but we can look at fixing that later by
> reviving my patches to move various machine specific stuff within the
> cpu into a separate sub-allocation.

ok.

Thanks,

C. 

>>
>> We could just drop this patch and easly get rid of the XiveTCTX object 
>> leak in spapr_unrealize_vcpu().
>>
>> C. 
>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>
>>>  Changes since v7:
>>>
>>>  - introduced spapr_irq_reset_xics(). 
>>>
>>>  include/hw/ppc/spapr_cpu_core.h |  2 ++
>>>  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
>>>  hw/ppc/spapr_irq.c              | 13 +++++++++++++
>>>  3 files changed, 41 insertions(+)
>>>
>>> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
>>> index 9e2821e4b31f..fc8ea9021656 100644
>>> --- a/include/hw/ppc/spapr_cpu_core.h
>>> +++ b/include/hw/ppc/spapr_cpu_core.h
>>> @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU *cpu)
>>>      return (sPAPRCPUState *)cpu->machine_data;
>>>  }
>>>  
>>> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
>>> +
>>>  #endif
>>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>>> index 82666436e9b4..afc5d4f4e739 100644
>>> --- a/hw/ppc/spapr_cpu_core.c
>>> +++ b/hw/ppc/spapr_cpu_core.c
>>> @@ -397,3 +397,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>>>  };
>>>  
>>>  DEFINE_TYPES(spapr_cpu_core_type_infos)
>>> +
>>> +typedef struct ForeachFindIntCArgs {
>>> +    const char *intc_type;
>>> +    Object *intc;
>>> +} ForeachFindIntCArgs;
>>> +
>>> +static int spapr_cpu_core_find_intc(Object *child, void *opaque)
>>> +{
>>> +    ForeachFindIntCArgs *args = opaque;
>>> +
>>> +    if (object_dynamic_cast(child, args->intc_type)) {
>>> +        args->intc = child;
>>> +    }
>>> +
>>> +    return args->intc != NULL;
>>> +}
>>> +
>>> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
>>> +{
>>> +    ForeachFindIntCArgs args = { intc_type, NULL };
>>> +
>>> +    object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args);
>>> +    g_assert(args.intc);
>>> +
>>> +    cpu->intc = args.intc;
>>> +}
>>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>>> index 0999a2b2d69c..814500f22d34 100644
>>> --- a/hw/ppc/spapr_irq.c
>>> +++ b/hw/ppc/spapr_irq.c
>>> @@ -12,6 +12,7 @@
>>>  #include "qemu/error-report.h"
>>>  #include "qapi/error.h"
>>>  #include "hw/ppc/spapr.h"
>>> +#include "hw/ppc/spapr_cpu_core.h"
>>>  #include "hw/ppc/spapr_xive.h"
>>>  #include "hw/ppc/xics.h"
>>>  #include "sysemu/kvm.h"
>>> @@ -208,6 +209,15 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
>>>      return 0;
>>>  }
>>>  
>>> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
>>> +{
>>> +   CPUState *cs;
>>> +
>>> +    CPU_FOREACH(cs) {
>>> +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
>>> +    }
>>> +}
>>> +
>>>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>>>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>>>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
>>> @@ -225,6 +235,7 @@ sPAPRIrq spapr_irq_xics = {
>>>      .dt_populate = spapr_dt_xics,
>>>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>>>      .post_load   = spapr_irq_post_load_xics,
>>> +    .reset       = spapr_irq_reset_xics,
>>>  };
>>>  
>>>  /*
>>> @@ -325,6 +336,8 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>>>      CPU_FOREACH(cs) {
>>>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>  
>>> +        spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX);
>>> +
>>>          /* (TCG) Set the OS CAM line of the thread interrupt context. */
>>>          spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
>>>      }
>>>
>>
>
David Gibson Dec. 18, 2018, midnight UTC | #5
On Mon, Dec 17, 2018 at 11:41:34PM +0100, Cédric Le Goater wrote:
> On 12/17/18 7:01 AM, David Gibson wrote:
> > On Thu, Dec 13, 2018 at 01:52:14PM +0100, Cédric Le Goater wrote:
> >> On 12/11/18 11:38 PM, Cédric Le Goater wrote:
> >>> Currently, the interrupt presenter of the vCPU is set at realize
> >>> time. Setting it at reset will become necessary when the new machine
> >>> supporting both interrupt modes is introduced. In this machine, the
> >>> interrupt mode is chosen at CAS time and activated after a reset.
> >>
> >> I think we need a second '->intc' pointer specific to XIVE instead. 
> >> How about ->intc_xive ?
> > 
> > Ah, interesting.  If we're going to need that, we might as well go
> > to specific ->icp and ->tctx pointers with the right types.
> 
> Hmm, PowerPCCPU is defined under target/ppc and adding the type might 
> add some noise. I will check.

It's a pointer, so you can just declare the type without a
definition.  That should be ok.

> > I don't particularly like having those machine specific pointers
> > within the cpu structure, but we can look at fixing that later by
> > reviving my patches to move various machine specific stuff within the
> > cpu into a separate sub-allocation.
> 
> ok.
> 
> Thanks,
> 
> C. 
> 
> >>
> >> We could just drop this patch and easly get rid of the XiveTCTX object 
> >> leak in spapr_unrealize_vcpu().
> >>
> >> C. 
> >>
> >>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>> ---
> >>>
> >>>  Changes since v7:
> >>>
> >>>  - introduced spapr_irq_reset_xics(). 
> >>>
> >>>  include/hw/ppc/spapr_cpu_core.h |  2 ++
> >>>  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
> >>>  hw/ppc/spapr_irq.c              | 13 +++++++++++++
> >>>  3 files changed, 41 insertions(+)
> >>>
> >>> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> >>> index 9e2821e4b31f..fc8ea9021656 100644
> >>> --- a/include/hw/ppc/spapr_cpu_core.h
> >>> +++ b/include/hw/ppc/spapr_cpu_core.h
> >>> @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU *cpu)
> >>>      return (sPAPRCPUState *)cpu->machine_data;
> >>>  }
> >>>  
> >>> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
> >>> +
> >>>  #endif
> >>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> >>> index 82666436e9b4..afc5d4f4e739 100644
> >>> --- a/hw/ppc/spapr_cpu_core.c
> >>> +++ b/hw/ppc/spapr_cpu_core.c
> >>> @@ -397,3 +397,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
> >>>  };
> >>>  
> >>>  DEFINE_TYPES(spapr_cpu_core_type_infos)
> >>> +
> >>> +typedef struct ForeachFindIntCArgs {
> >>> +    const char *intc_type;
> >>> +    Object *intc;
> >>> +} ForeachFindIntCArgs;
> >>> +
> >>> +static int spapr_cpu_core_find_intc(Object *child, void *opaque)
> >>> +{
> >>> +    ForeachFindIntCArgs *args = opaque;
> >>> +
> >>> +    if (object_dynamic_cast(child, args->intc_type)) {
> >>> +        args->intc = child;
> >>> +    }
> >>> +
> >>> +    return args->intc != NULL;
> >>> +}
> >>> +
> >>> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
> >>> +{
> >>> +    ForeachFindIntCArgs args = { intc_type, NULL };
> >>> +
> >>> +    object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args);
> >>> +    g_assert(args.intc);
> >>> +
> >>> +    cpu->intc = args.intc;
> >>> +}
> >>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> >>> index 0999a2b2d69c..814500f22d34 100644
> >>> --- a/hw/ppc/spapr_irq.c
> >>> +++ b/hw/ppc/spapr_irq.c
> >>> @@ -12,6 +12,7 @@
> >>>  #include "qemu/error-report.h"
> >>>  #include "qapi/error.h"
> >>>  #include "hw/ppc/spapr.h"
> >>> +#include "hw/ppc/spapr_cpu_core.h"
> >>>  #include "hw/ppc/spapr_xive.h"
> >>>  #include "hw/ppc/xics.h"
> >>>  #include "sysemu/kvm.h"
> >>> @@ -208,6 +209,15 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
> >>>      return 0;
> >>>  }
> >>>  
> >>> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> >>> +{
> >>> +   CPUState *cs;
> >>> +
> >>> +    CPU_FOREACH(cs) {
> >>> +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
> >>> +    }
> >>> +}
> >>> +
> >>>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
> >>>  #define SPAPR_IRQ_XICS_NR_MSIS     \
> >>>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> >>> @@ -225,6 +235,7 @@ sPAPRIrq spapr_irq_xics = {
> >>>      .dt_populate = spapr_dt_xics,
> >>>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> >>>      .post_load   = spapr_irq_post_load_xics,
> >>> +    .reset       = spapr_irq_reset_xics,
> >>>  };
> >>>  
> >>>  /*
> >>> @@ -325,6 +336,8 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
> >>>      CPU_FOREACH(cs) {
> >>>          PowerPCCPU *cpu = POWERPC_CPU(cs);
> >>>  
> >>> +        spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX);
> >>> +
> >>>          /* (TCG) Set the OS CAM line of the thread interrupt context. */
> >>>          spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
> >>>      }
> >>>
> >>
> > 
>
diff mbox series

Patch

diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
index 9e2821e4b31f..fc8ea9021656 100644
--- a/include/hw/ppc/spapr_cpu_core.h
+++ b/include/hw/ppc/spapr_cpu_core.h
@@ -53,4 +53,6 @@  static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU *cpu)
     return (sPAPRCPUState *)cpu->machine_data;
 }
 
+void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
+
 #endif
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 82666436e9b4..afc5d4f4e739 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -397,3 +397,29 @@  static const TypeInfo spapr_cpu_core_type_infos[] = {
 };
 
 DEFINE_TYPES(spapr_cpu_core_type_infos)
+
+typedef struct ForeachFindIntCArgs {
+    const char *intc_type;
+    Object *intc;
+} ForeachFindIntCArgs;
+
+static int spapr_cpu_core_find_intc(Object *child, void *opaque)
+{
+    ForeachFindIntCArgs *args = opaque;
+
+    if (object_dynamic_cast(child, args->intc_type)) {
+        args->intc = child;
+    }
+
+    return args->intc != NULL;
+}
+
+void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
+{
+    ForeachFindIntCArgs args = { intc_type, NULL };
+
+    object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args);
+    g_assert(args.intc);
+
+    cpu->intc = args.intc;
+}
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 0999a2b2d69c..814500f22d34 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -12,6 +12,7 @@ 
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_cpu_core.h"
 #include "hw/ppc/spapr_xive.h"
 #include "hw/ppc/xics.h"
 #include "sysemu/kvm.h"
@@ -208,6 +209,15 @@  static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
     return 0;
 }
 
+static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
+{
+   CPUState *cs;
+
+    CPU_FOREACH(cs) {
+        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
+    }
+}
+
 #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
 #define SPAPR_IRQ_XICS_NR_MSIS     \
     (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
@@ -225,6 +235,7 @@  sPAPRIrq spapr_irq_xics = {
     .dt_populate = spapr_dt_xics,
     .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
     .post_load   = spapr_irq_post_load_xics,
+    .reset       = spapr_irq_reset_xics,
 };
 
 /*
@@ -325,6 +336,8 @@  static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
     CPU_FOREACH(cs) {
         PowerPCCPU *cpu = POWERPC_CPU(cs);
 
+        spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX);
+
         /* (TCG) Set the OS CAM line of the thread interrupt context. */
         spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
     }