[Qemu-devel] [PATCH v5 25/36] spapr: set the interrupt presenter at reset

Cédric Le Goater posted 36 patches 6 years, 11 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v5 25/36] spapr: set the interrupt presenter at reset
Posted by Cédric Le Goater 6 years, 11 months ago
Currently, the interrupt presenter of the VPCU is set at realize
time. Setting it at reset will become useful 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>
---
 include/hw/ppc/spapr_cpu_core.h |  2 ++
 hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
 hw/ppc/spapr_irq.c              | 11 +++++++++++
 3 files changed, 39 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 1811cd48db90..529de0b6b9c8 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -398,3 +398,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 984c6d60cd9f..969efad7e6e9 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -218,6 +218,11 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
 
 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
@@ -370,6 +375,12 @@ static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int version_id)
 
 static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
 {
+    CPUState *cs;
+
+    CPU_FOREACH(cs) {
+        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->xive_tctx_type);
+    }
+
     spapr_xive_mmio_map(spapr->xive);
 }
 
-- 
2.17.2


Re: [Qemu-devel] [PATCH v5 25/36] spapr: set the interrupt presenter at reset
Posted by David Gibson 6 years, 11 months ago
On Fri, Nov 16, 2018 at 11:57:18AM +0100, Cédric Le Goater wrote:
> Currently, the interrupt presenter of the VPCU is set at realize
> time. Setting it at reset will become useful 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>
> ---
>  include/hw/ppc/spapr_cpu_core.h |  2 ++
>  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
>  hw/ppc/spapr_irq.c              | 11 +++++++++++
>  3 files changed, 39 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 1811cd48db90..529de0b6b9c8 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -398,3 +398,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);

We could create some extra links on the cpu to avoid scanning all the
children, but I guess that's a refinement.

Then again.. what do we actually use the cpu->intc pointer for in XIVE
context?  I had a feeling because of the different way notifications
are handled we might not ever need to go from a cpu handle to the
associated TCTX.


> +    cpu->intc = args.intc;
> +}
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 984c6d60cd9f..969efad7e6e9 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -218,6 +218,11 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
>  
>  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
> @@ -370,6 +375,12 @@ static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int version_id)
>  
>  static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>  {
> +    CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->xive_tctx_type);
> +    }
> +
>      spapr_xive_mmio_map(spapr->xive);
>  }
>  

-- 
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
Re: [Qemu-devel] [PATCH v5 25/36] spapr: set the interrupt presenter at reset
Posted by Cédric Le Goater 6 years, 11 months ago
On 11/29/18 5:03 AM, David Gibson wrote:
> On Fri, Nov 16, 2018 at 11:57:18AM +0100, Cédric Le Goater wrote:
>> Currently, the interrupt presenter of the VPCU is set at realize
>> time. Setting it at reset will become useful 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>
>> ---
>>  include/hw/ppc/spapr_cpu_core.h |  2 ++
>>  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
>>  hw/ppc/spapr_irq.c              | 11 +++++++++++
>>  3 files changed, 39 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 1811cd48db90..529de0b6b9c8 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -398,3 +398,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);
> 
> We could create some extra links on the cpu to avoid scanning all the
> children, but I guess that's a refinement.

yes. Like an extra ->intc for xive. but as we can have only one interrupt
controller active, having only one presenter per CPU seems reasonable. 

> Then again.. what do we actually use the cpu->intc pointer for in XIVE
> context?  

yes for the TIMA MMIOs and also when running the matching algo in the XIVE 
presenter.

C.

> I had a feeling because of the different way notifications
> are handled we might not ever need to go from a cpu handle to the
> associated TCTX.
>
> 
>> +    cpu->intc = args.intc;
>> +}
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index 984c6d60cd9f..969efad7e6e9 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -218,6 +218,11 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
>>  
>>  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
>> @@ -370,6 +375,12 @@ static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int version_id)
>>  
>>  static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>>  {
>> +    CPUState *cs;
>> +
>> +    CPU_FOREACH(cs) {
>> +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->xive_tctx_type);
>> +    }
>> +
>>      spapr_xive_mmio_map(spapr->xive);
>>  }
>>  
>