diff mbox

[v7,07/11] hw/arm_gic: Convert ARM GIC classes to use init/realize

Message ID 1361900421-28354-8-git-send-email-peter.maydell@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Maydell Feb. 26, 2013, 5:40 p.m. UTC 
Convert the ARM GIC classes to use init/realize rather than
SysBusDevice::init. (We have to do them all in one patch to
avoid unconverted subclasses calling a nonexistent SysBusDevice
init function in the base class and crashing.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm_gic.c          |   23 +++++++++++++----------
 hw/arm_gic_common.c   |   26 +++++++++++++++-----------
 hw/arm_gic_internal.h |    2 +-
 hw/armv7m_nvic.c      |   15 ++++++++-------
 4 files changed, 37 insertions(+), 29 deletions(-)

Comments

Andreas Färber March 4, 2013, 11:10 a.m. UTC | #1 
Am 26.02.2013 18:40, schrieb Peter Maydell:
> Convert the ARM GIC classes to use init/realize rather than
> SysBusDevice::init. (We have to do them all in one patch to
> avoid unconverted subclasses calling a nonexistent SysBusDevice
> init function in the base class and crashing.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm_gic.c          |   23 +++++++++++++----------
>  hw/arm_gic_common.c   |   26 +++++++++++++++-----------
>  hw/arm_gic_internal.h |    2 +-
>  hw/armv7m_nvic.c      |   15 ++++++++-------
>  4 files changed, 37 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/arm_gic.c b/hw/arm_gic.c
> index 90e43d0..250e720 100644
> --- a/hw/arm_gic.c
> +++ b/hw/arm_gic.c
> @@ -659,14 +659,18 @@ void gic_init_irqs_and_distributor(GICState *s, int num_irq)
>      memory_region_init_io(&s->iomem, &gic_dist_ops, s, "gic_dist", 0x1000);
>  }
>  
> -static int arm_gic_init(SysBusDevice *dev)
> +static void arm_gic_realize(DeviceState *dev, Error **errp)
>  {
> -    /* Device instance init function for the GIC sysbus device */
> +    /* Device instance realize function for the GIC sysbus device */
>      int i;
> -    GICState *s = FROM_SYSBUS(GICState, dev);
> +    GICState *s = ARM_GIC(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      ARMGICClass *agc = ARM_GIC_GET_CLASS(s);
>  
> -    agc->parent_init(dev);
> +    agc->parent_realize(dev, errp);
> +    if (error_is_set(errp)) {
> +        return;
> +    }
>  
>      gic_init_irqs_and_distributor(s, s->num_irq);
>  
> @@ -686,22 +690,21 @@ static int arm_gic_init(SysBusDevice *dev)
>                                "gic_cpu", 0x100);
>      }
>      /* Distributor */
> -    sysbus_init_mmio(dev, &s->iomem);
> +    sysbus_init_mmio(sbd, &s->iomem);
>      /* cpu interfaces (one for "current cpu" plus one per cpu) */
>      for (i = 0; i <= NUM_CPU(s); i++) {
> -        sysbus_init_mmio(dev, &s->cpuiomem[i]);
> +        sysbus_init_mmio(sbd, &s->cpuiomem[i]);
>      }
> -    return 0;
>  }
>  
>  static void arm_gic_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
>      ARMGICClass *agc = ARM_GIC_CLASS(klass);
> -    agc->parent_init = sbc->init;
> -    sbc->init = arm_gic_init;
> +
>      dc->no_user = 1;
> +    agc->parent_realize = dc->realize;
> +    dc->realize = arm_gic_realize;
>  }
>  
>  static const TypeInfo arm_gic_info = {
> diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c
> index 2947622..3b2955c 100644
> --- a/hw/arm_gic_common.c
> +++ b/hw/arm_gic_common.c
> @@ -104,31 +104,35 @@ static int gic_load(QEMUFile *f, void *opaque, int version_id)
>      return 0;
>  }
>  
> -static int arm_gic_common_init(SysBusDevice *dev)
> +static void arm_gic_common_realize(DeviceState *dev, Error **errp)
>  {
> -    GICState *s = FROM_SYSBUS(GICState, dev);
> +    GICState *s = ARM_GIC_COMMON(dev);
>      int num_irq = s->num_irq;
>  
>      if (s->num_cpu > NCPU) {
> -        hw_error("requested %u CPUs exceeds GIC maximum %d\n",
> -                 s->num_cpu, NCPU);
> +        error_setg(errp, "requested %u CPUs exceeds GIC maximum %d\n",

Please drop \n for error_setg(). Probably would be worth adding to a
convert-to-realize section on the Wiki.

> +                   s->num_cpu, NCPU);
> +        return;
>      }
>      s->num_irq += GIC_BASE_IRQ;
>      if (s->num_irq > GIC_MAXIRQ) {
> -        hw_error("requested %u interrupt lines exceeds GIC maximum %d\n",
> -                 num_irq, GIC_MAXIRQ);
> +        error_setg(errp,
> +                   "requested %u interrupt lines exceeds GIC maximum %d\n",
> +                   num_irq, GIC_MAXIRQ);
> +        return;
>      }
>      /* ITLinesNumber is represented as (N / 32) - 1 (see
>       * gic_dist_readb) so this is an implementation imposed
>       * restriction, not an architectural one:
>       */
>      if (s->num_irq < 32 || (s->num_irq % 32)) {
> -        hw_error("%d interrupt lines unsupported: not divisible by 32\n",
> -                 num_irq);
> +        error_setg(errp,
> +                   "%d interrupt lines unsupported: not divisible by 32\n",
> +                   num_irq);
> +        return;
>      }
>  
>      register_savevm(NULL, "arm_gic", -1, 3, gic_save, gic_load, s);
> -    return 0;
>  }
>  
>  static void arm_gic_common_reset(DeviceState *dev)
> @@ -173,12 +177,12 @@ static Property arm_gic_common_properties[] = {
>  
>  static void arm_gic_common_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +
>      dc->reset = arm_gic_common_reset;
> +    dc->realize = arm_gic_common_realize;
>      dc->props = arm_gic_common_properties;
>      dc->no_user = 1;
> -    sc->init = arm_gic_common_init;
>  }
>  
>  static const TypeInfo arm_gic_common_type = {
> diff --git a/hw/arm_gic_internal.h b/hw/arm_gic_internal.h
> index 3640be0..3ba37f3 100644
> --- a/hw/arm_gic_internal.h
> +++ b/hw/arm_gic_internal.h
> @@ -132,7 +132,7 @@ typedef struct ARMGICCommonClass {
>  
>  typedef struct ARMGICClass {
>      ARMGICCommonClass parent_class;
> -    int (*parent_init)(SysBusDevice *dev);
> +    DeviceRealize parent_realize;
>  } ARMGICClass;
>  
>  #endif /* !QEMU_ARM_GIC_INTERNAL_H */
> diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
> index d5798d0..3c79674 100644
> --- a/hw/armv7m_nvic.c
> +++ b/hw/armv7m_nvic.c
> @@ -41,7 +41,7 @@ typedef struct NVICClass {
>      /*< private >*/
>      ARMGICClass parent_class;
>      /*< public >*/
> -    int (*parent_init)(SysBusDevice *dev);
> +    DeviceRealize parent_realize;
>      void (*parent_reset)(DeviceState *dev);
>  } NVICClass;
>  
> @@ -465,7 +465,7 @@ static void armv7m_nvic_reset(DeviceState *dev)
>      systick_reset(s);
>  }
>  
> -static int armv7m_nvic_init(SysBusDevice *dev)
> +static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
>  {
>      nvic_state *s = NVIC(dev);
>      NVICClass *nc = NVIC_GET_CLASS(s);
> @@ -475,7 +475,10 @@ static int armv7m_nvic_init(SysBusDevice *dev)
>      /* Tell the common code we're an NVIC */
>      s->gic.revision = 0xffffffff;
>      s->num_irq = s->gic.num_irq;
> -    nc->parent_init(dev);
> +    nc->parent_realize(dev, errp);
> +    if (error_is_set(errp)) {
> +        return;
> +    }
>      gic_init_irqs_and_distributor(&s->gic, s->num_irq);
>      /* The NVIC and system controller register area looks like this:
>       *  0..0xff : system control registers, including systick
> @@ -503,7 +506,6 @@ static int armv7m_nvic_init(SysBusDevice *dev)
>       */
>      memory_region_add_subregion(get_system_memory(), 0xe000e000, &s->container);
>      s->systick.timer = qemu_new_timer_ns(vm_clock, systick_timer_tick, s);
> -    return 0;
>  }
>  
>  static void armv7m_nvic_instance_init(Object *obj)
> @@ -526,13 +528,12 @@ static void armv7m_nvic_class_init(ObjectClass *klass, void *data)
>  {
>      NVICClass *nc = NVIC_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
>  
>      nc->parent_reset = dc->reset;
> -    nc->parent_init = sdc->init;
> -    sdc->init = armv7m_nvic_init;
> +    nc->parent_realize = dc->realize;
>      dc->vmsd  = &vmstate_nvic;
>      dc->reset = armv7m_nvic_reset;
> +    dc->realize = armv7m_nvic_realize;
>  }
>  
>  static const TypeInfo armv7m_nvic_info = {
> 

Otherwise looks fine, thanks.

Andreas
Peter Maydell March 4, 2013, 11:50 a.m. UTC | #2 
On 4 March 2013 19:10, Andreas Färber <afaerber@suse.de> wrote:
> Am 26.02.2013 18:40, schrieb Peter Maydell:

>>      if (s->num_cpu > NCPU) {
>> -        hw_error("requested %u CPUs exceeds GIC maximum %d\n",
>> -                 s->num_cpu, NCPU);
>> +        error_setg(errp, "requested %u CPUs exceeds GIC maximum %d\n",
>
> Please drop \n for error_setg(). Probably would be worth adding to a
> convert-to-realize section on the Wiki.

Doh. That's such a trivial change I intend to just make it in
passing when I put these changes into target-arm.next rather
than sending out an entire fresh round of patches, unless you
object.

> Otherwise looks fine, thanks.

Should I mark such a fixed-up patch with your reviewed-by tag?

thanks
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Färber March 4, 2013, 12:04 p.m. UTC | #3 
Am 04.03.2013 12:50, schrieb Peter Maydell:
> On 4 March 2013 19:10, Andreas Färber <afaerber@suse.de> wrote:
>> Am 26.02.2013 18:40, schrieb Peter Maydell:
> 
>>>      if (s->num_cpu > NCPU) {
>>> -        hw_error("requested %u CPUs exceeds GIC maximum %d\n",
>>> -                 s->num_cpu, NCPU);
>>> +        error_setg(errp, "requested %u CPUs exceeds GIC maximum %d\n",
>>
>> Please drop \n for error_setg(). Probably would be worth adding to a
>> convert-to-realize section on the Wiki.
> 
> Doh. That's such a trivial change I intend to just make it in
> passing when I put these changes into target-arm.next rather
> than sending out an entire fresh round of patches, unless you
> object.
> 
>> Otherwise looks fine, thanks.
> 
> Should I mark such a fixed-up patch with your reviewed-by tag?

Yes, that's fine, thanks.

Andreas
diff mbox

Patch

diff --git a/hw/arm_gic.c b/hw/arm_gic.c
index 90e43d0..250e720 100644
--- a/hw/arm_gic.c
+++ b/hw/arm_gic.c
@@ -659,14 +659,18 @@  void gic_init_irqs_and_distributor(GICState *s, int num_irq)
     memory_region_init_io(&s->iomem, &gic_dist_ops, s, "gic_dist", 0x1000);
 }
 
-static int arm_gic_init(SysBusDevice *dev)
+static void arm_gic_realize(DeviceState *dev, Error **errp)
 {
-    /* Device instance init function for the GIC sysbus device */
+    /* Device instance realize function for the GIC sysbus device */
     int i;
-    GICState *s = FROM_SYSBUS(GICState, dev);
+    GICState *s = ARM_GIC(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     ARMGICClass *agc = ARM_GIC_GET_CLASS(s);
 
-    agc->parent_init(dev);
+    agc->parent_realize(dev, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
 
     gic_init_irqs_and_distributor(s, s->num_irq);
 
@@ -686,22 +690,21 @@  static int arm_gic_init(SysBusDevice *dev)
                               "gic_cpu", 0x100);
     }
     /* Distributor */
-    sysbus_init_mmio(dev, &s->iomem);
+    sysbus_init_mmio(sbd, &s->iomem);
     /* cpu interfaces (one for "current cpu" plus one per cpu) */
     for (i = 0; i <= NUM_CPU(s); i++) {
-        sysbus_init_mmio(dev, &s->cpuiomem[i]);
+        sysbus_init_mmio(sbd, &s->cpuiomem[i]);
     }
-    return 0;
 }
 
 static void arm_gic_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
     ARMGICClass *agc = ARM_GIC_CLASS(klass);
-    agc->parent_init = sbc->init;
-    sbc->init = arm_gic_init;
+
     dc->no_user = 1;
+    agc->parent_realize = dc->realize;
+    dc->realize = arm_gic_realize;
 }
 
 static const TypeInfo arm_gic_info = {
diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c
index 2947622..3b2955c 100644
--- a/hw/arm_gic_common.c
+++ b/hw/arm_gic_common.c
@@ -104,31 +104,35 @@  static int gic_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static int arm_gic_common_init(SysBusDevice *dev)
+static void arm_gic_common_realize(DeviceState *dev, Error **errp)
 {
-    GICState *s = FROM_SYSBUS(GICState, dev);
+    GICState *s = ARM_GIC_COMMON(dev);
     int num_irq = s->num_irq;
 
     if (s->num_cpu > NCPU) {
-        hw_error("requested %u CPUs exceeds GIC maximum %d\n",
-                 s->num_cpu, NCPU);
+        error_setg(errp, "requested %u CPUs exceeds GIC maximum %d\n",
+                   s->num_cpu, NCPU);
+        return;
     }
     s->num_irq += GIC_BASE_IRQ;
     if (s->num_irq > GIC_MAXIRQ) {
-        hw_error("requested %u interrupt lines exceeds GIC maximum %d\n",
-                 num_irq, GIC_MAXIRQ);
+        error_setg(errp,
+                   "requested %u interrupt lines exceeds GIC maximum %d\n",
+                   num_irq, GIC_MAXIRQ);
+        return;
     }
     /* ITLinesNumber is represented as (N / 32) - 1 (see
      * gic_dist_readb) so this is an implementation imposed
      * restriction, not an architectural one:
      */
     if (s->num_irq < 32 || (s->num_irq % 32)) {
-        hw_error("%d interrupt lines unsupported: not divisible by 32\n",
-                 num_irq);
+        error_setg(errp,
+                   "%d interrupt lines unsupported: not divisible by 32\n",
+                   num_irq);
+        return;
     }
 
     register_savevm(NULL, "arm_gic", -1, 3, gic_save, gic_load, s);
-    return 0;
 }
 
 static void arm_gic_common_reset(DeviceState *dev)
@@ -173,12 +177,12 @@  static Property arm_gic_common_properties[] = {
 
 static void arm_gic_common_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
+
     dc->reset = arm_gic_common_reset;
+    dc->realize = arm_gic_common_realize;
     dc->props = arm_gic_common_properties;
     dc->no_user = 1;
-    sc->init = arm_gic_common_init;
 }
 
 static const TypeInfo arm_gic_common_type = {
diff --git a/hw/arm_gic_internal.h b/hw/arm_gic_internal.h
index 3640be0..3ba37f3 100644
--- a/hw/arm_gic_internal.h
+++ b/hw/arm_gic_internal.h
@@ -132,7 +132,7 @@  typedef struct ARMGICCommonClass {
 
 typedef struct ARMGICClass {
     ARMGICCommonClass parent_class;
-    int (*parent_init)(SysBusDevice *dev);
+    DeviceRealize parent_realize;
 } ARMGICClass;
 
 #endif /* !QEMU_ARM_GIC_INTERNAL_H */
diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
index d5798d0..3c79674 100644
--- a/hw/armv7m_nvic.c
+++ b/hw/armv7m_nvic.c
@@ -41,7 +41,7 @@  typedef struct NVICClass {
     /*< private >*/
     ARMGICClass parent_class;
     /*< public >*/
-    int (*parent_init)(SysBusDevice *dev);
+    DeviceRealize parent_realize;
     void (*parent_reset)(DeviceState *dev);
 } NVICClass;
 
@@ -465,7 +465,7 @@  static void armv7m_nvic_reset(DeviceState *dev)
     systick_reset(s);
 }
 
-static int armv7m_nvic_init(SysBusDevice *dev)
+static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
 {
     nvic_state *s = NVIC(dev);
     NVICClass *nc = NVIC_GET_CLASS(s);
@@ -475,7 +475,10 @@  static int armv7m_nvic_init(SysBusDevice *dev)
     /* Tell the common code we're an NVIC */
     s->gic.revision = 0xffffffff;
     s->num_irq = s->gic.num_irq;
-    nc->parent_init(dev);
+    nc->parent_realize(dev, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
     gic_init_irqs_and_distributor(&s->gic, s->num_irq);
     /* The NVIC and system controller register area looks like this:
      *  0..0xff : system control registers, including systick
@@ -503,7 +506,6 @@  static int armv7m_nvic_init(SysBusDevice *dev)
      */
     memory_region_add_subregion(get_system_memory(), 0xe000e000, &s->container);
     s->systick.timer = qemu_new_timer_ns(vm_clock, systick_timer_tick, s);
-    return 0;
 }
 
 static void armv7m_nvic_instance_init(Object *obj)
@@ -526,13 +528,12 @@  static void armv7m_nvic_class_init(ObjectClass *klass, void *data)
 {
     NVICClass *nc = NVIC_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
 
     nc->parent_reset = dc->reset;
-    nc->parent_init = sdc->init;
-    sdc->init = armv7m_nvic_init;
+    nc->parent_realize = dc->realize;
     dc->vmsd  = &vmstate_nvic;
     dc->reset = armv7m_nvic_reset;
+    dc->realize = armv7m_nvic_realize;
 }
 
 static const TypeInfo armv7m_nvic_info = {