diff mbox

[3/4] xics: rework initialization

Message ID 1374043057-27208-4-git-send-email-aik@ozlabs.ru
State New
Headers show

Commit Message

Alexey Kardashevskiy July 17, 2013, 6:37 a.m. UTC
Currently RTAS and hypercalls are registered in the XICS class init
function. The upcoming XICS-KVM will inherit from XICS but will use
another API to register RTAS tokens with KVM so registration has
to move from the class init function (common for both XICS and
XICS-KVM) to the _realize function (specific to the controller).

This moves ICS creation to _realize as there is no point to create
some child devices in initfn() (ICS) and some in realize() (ICPs).

As initfn is no more needed, this removes it.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/intc/xics.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

Comments

Andreas Färber July 31, 2013, 7:22 p.m. UTC | #1
Am 17.07.2013 08:37, schrieb Alexey Kardashevskiy:
> Currently RTAS and hypercalls are registered in the XICS class init
> function. The upcoming XICS-KVM will inherit from XICS but will use
> another API to register RTAS tokens with KVM so registration has
> to move from the class init function (common for both XICS and
> XICS-KVM) to the _realize function (specific to the controller).

This sounds sensible, but the reason is definitely not that there is
only class_init when you have two classes, but that registration of
global state belongs into realize.

> This moves ICS creation to _realize as there is no point to create
> some child devices in initfn() (ICS) and some in realize() (ICPs).
> 
> As initfn is no more needed, this removes it.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/intc/xics.c | 35 +++++++++++++++--------------------
>  1 file changed, 15 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 283c2dd..a359f52 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -689,9 +689,23 @@ static void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
>  static void xics_realize(DeviceState *dev, Error **errp)
>  {
>      XICSState *icp = XICS(dev);
> -    ICSState *ics = icp->ics;
> +    ICSState *ics;
>      int i;
>  
> +    spapr_rtas_register("ibm,set-xive", rtas_set_xive);
> +    spapr_rtas_register("ibm,get-xive", rtas_get_xive);
> +    spapr_rtas_register("ibm,int-off", rtas_int_off);
> +    spapr_rtas_register("ibm,int-on", rtas_int_on);
> +
> +    spapr_register_hypercall(H_CPPR, h_cppr);
> +    spapr_register_hypercall(H_IPI, h_ipi);
> +    spapr_register_hypercall(H_XIRR, h_xirr);
> +    spapr_register_hypercall(H_EOI, h_eoi);
> +
> +    icp->ics = ICS(object_new(TYPE_ICS));
> +    ics = icp->ics;
> +    object_property_add_child(OBJECT(icp), "ics", OBJECT(icp->ics), NULL);
> +
>      ics->nr_irqs = icp->nr_irqs;
>      ics->offset = XICS_IRQ_BASE;
>      ics->icp = icp;
> @@ -707,14 +721,6 @@ static void xics_realize(DeviceState *dev, Error **errp)
>      }
>  }
>  
> -static void xics_initfn(Object *obj)
> -{
> -    XICSState *xics = XICS(obj);
> -
> -    xics->ics = ICS(object_new(TYPE_ICS));
> -    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
> -}
> -
>  static Property xics_properties[] = {
>      DEFINE_PROP_UINT32("nr_servers", XICSState, nr_servers, -1),
>      DEFINE_PROP_UINT32("nr_irqs", XICSState, nr_irqs, -1),

This looks wrong, both before and after. It should be both created using
object_initialize() and added as child property in instance_init. If
TYPE_ICS is a device, then in realize it should get realized.

Regards,
Andreas

> @@ -730,16 +736,6 @@ static void xics_class_init(ObjectClass *oc, void *data)
>      dc->props = xics_properties;
>      dc->reset = xics_reset;
>      k->cpu_setup = xics_cpu_setup;
> -
> -    spapr_rtas_register("ibm,set-xive", rtas_set_xive);
> -    spapr_rtas_register("ibm,get-xive", rtas_get_xive);
> -    spapr_rtas_register("ibm,int-off", rtas_int_off);
> -    spapr_rtas_register("ibm,int-on", rtas_int_on);
> -
> -    spapr_register_hypercall(H_CPPR, h_cppr);
> -    spapr_register_hypercall(H_IPI, h_ipi);
> -    spapr_register_hypercall(H_XIRR, h_xirr);
> -    spapr_register_hypercall(H_EOI, h_eoi);
>  }
>  
>  static const TypeInfo xics_info = {
> @@ -748,7 +744,6 @@ static const TypeInfo xics_info = {
>      .instance_size = sizeof(XICSState),
>      .class_size = sizeof(XICSStateClass),
>      .class_init    = xics_class_init,
> -    .instance_init = xics_initfn,
>  };
>  
>  static void xics_register_types(void)
diff mbox

Patch

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 283c2dd..a359f52 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -689,9 +689,23 @@  static void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
 static void xics_realize(DeviceState *dev, Error **errp)
 {
     XICSState *icp = XICS(dev);
-    ICSState *ics = icp->ics;
+    ICSState *ics;
     int i;
 
+    spapr_rtas_register("ibm,set-xive", rtas_set_xive);
+    spapr_rtas_register("ibm,get-xive", rtas_get_xive);
+    spapr_rtas_register("ibm,int-off", rtas_int_off);
+    spapr_rtas_register("ibm,int-on", rtas_int_on);
+
+    spapr_register_hypercall(H_CPPR, h_cppr);
+    spapr_register_hypercall(H_IPI, h_ipi);
+    spapr_register_hypercall(H_XIRR, h_xirr);
+    spapr_register_hypercall(H_EOI, h_eoi);
+
+    icp->ics = ICS(object_new(TYPE_ICS));
+    ics = icp->ics;
+    object_property_add_child(OBJECT(icp), "ics", OBJECT(icp->ics), NULL);
+
     ics->nr_irqs = icp->nr_irqs;
     ics->offset = XICS_IRQ_BASE;
     ics->icp = icp;
@@ -707,14 +721,6 @@  static void xics_realize(DeviceState *dev, Error **errp)
     }
 }
 
-static void xics_initfn(Object *obj)
-{
-    XICSState *xics = XICS(obj);
-
-    xics->ics = ICS(object_new(TYPE_ICS));
-    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-}
-
 static Property xics_properties[] = {
     DEFINE_PROP_UINT32("nr_servers", XICSState, nr_servers, -1),
     DEFINE_PROP_UINT32("nr_irqs", XICSState, nr_irqs, -1),
@@ -730,16 +736,6 @@  static void xics_class_init(ObjectClass *oc, void *data)
     dc->props = xics_properties;
     dc->reset = xics_reset;
     k->cpu_setup = xics_cpu_setup;
-
-    spapr_rtas_register("ibm,set-xive", rtas_set_xive);
-    spapr_rtas_register("ibm,get-xive", rtas_get_xive);
-    spapr_rtas_register("ibm,int-off", rtas_int_off);
-    spapr_rtas_register("ibm,int-on", rtas_int_on);
-
-    spapr_register_hypercall(H_CPPR, h_cppr);
-    spapr_register_hypercall(H_IPI, h_ipi);
-    spapr_register_hypercall(H_XIRR, h_xirr);
-    spapr_register_hypercall(H_EOI, h_eoi);
 }
 
 static const TypeInfo xics_info = {
@@ -748,7 +744,6 @@  static const TypeInfo xics_info = {
     .instance_size = sizeof(XICSState),
     .class_size = sizeof(XICSStateClass),
     .class_init    = xics_class_init,
-    .instance_init = xics_initfn,
 };
 
 static void xics_register_types(void)