Re: [PATCH v2 20/33] spapr, xics, xive: Introduce SpaprInterruptController QOM interface

[Greg Kurz <groug@kaod.org>]

On Fri, 27 Sep 2019 15:50:15 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The SpaprIrq structure is used to represent ths spapr machine's irq
> backend.  Except that it kind of conflates two concepts: one is the
> backend proper - a specific interrupt controller that we might or
> might not be using, the other is the irq configuration which covers
> the layout of irq space and which interrupt controllers are allowed.
> 
> This leads to some pretty confusing code paths for the "dual"
> configuration where its hooks redirect to other SpaprIrq structures
> depending on the currently active irq controller.
> 
> To clean this up, we start by introducing a new
> SpaprInterruptController QOM interface to represent strictly an
> interrupt controller backend, not counting anything configuration
> related.  We implement this interface in the XICs and XIVE interrupt
> controllers, and in future we'll move relevant methods from SpaprIrq
> into it.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/intc/spapr_xive.c       |  4 ++++
>  hw/intc/xics_spapr.c       |  4 ++++
>  hw/ppc/spapr_irq.c         | 13 +++++++++++++
>  include/hw/ppc/spapr_irq.h | 14 ++++++++++++++
>  4 files changed, 35 insertions(+)
> 
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 04879abf2e..b67e9c3245 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -519,6 +519,10 @@ static const TypeInfo spapr_xive_info = {
>      .instance_init = spapr_xive_instance_init,
>      .instance_size = sizeof(SpaprXive),
>      .class_init = spapr_xive_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_SPAPR_INTC },
> +        { }
> +    },
>  };
>  
>  static void spapr_xive_register_types(void)
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 6e5eb24b3c..4874e6be55 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -343,6 +343,10 @@ static const TypeInfo ics_spapr_info = {
>      .name = TYPE_ICS_SPAPR,
>      .parent = TYPE_ICS,
>      .class_init = ics_spapr_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_SPAPR_INTC },
> +        { }
> +    },
>  };
>  
>  static void xics_spapr_register_types(void)
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 5e448b1fd4..0603c82fe8 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -23,6 +23,12 @@
>  
>  #include "trace.h"
>  
> +static const TypeInfo spapr_intc_info = {
> +    .name = TYPE_SPAPR_INTC,
> +    .parent = TYPE_INTERFACE,
> +    .class_size = sizeof(SpaprInterruptControllerClass),
> +};
> +
>  void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis)
>  {
>      spapr->irq_map_nr = nr_msis;
> @@ -763,3 +769,10 @@ SpaprIrq spapr_irq_xics_legacy = {
>      .set_irq     = spapr_irq_set_irq_xics,
>      .init_kvm    = spapr_irq_init_kvm_xics,
>  };
> +
> +static void spapr_irq_register_types(void)
> +{
> +    type_register_static(&spapr_intc_info);
> +}
> +
> +type_init(spapr_irq_register_types)
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 69a37f608e..b9398e0be3 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -31,6 +31,20 @@
>  
>  typedef struct SpaprMachineState SpaprMachineState;
>  
> +typedef struct SpaprInterruptController SpaprInterruptController;
> +
> +#define TYPE_SPAPR_INTC "spapr-interrupt-controller"
> +#define SPAPR_INTC(obj)                                     \
> +    INTERFACE_CHECK(SpaprInterruptController, (obj), TYPE_SPAPR_INTC)
> +#define SPAPR_INTC_CLASS(klass)                                     \
> +    OBJECT_CLASS_CHECK(SpaprInterruptControllerClass, (klass), TYPE_SPAPR_INTC)
> +#define SPAPR_INTC_GET_CLASS(obj)                                   \
> +    OBJECT_GET_CLASS(SpaprInterruptControllerClass, (obj), TYPE_SPAPR_INTC)
> +
> +typedef struct SpaprInterruptControllerClass {
> +    InterfaceClass parent;
> +} SpaprInterruptControllerClass;
> +
>  void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis);
>  int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align,
>                          Error **errp);