| Message ID | 1361900421-28354-8-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | 53111180946a56d314a9c1d07d09b9ef91e847b9 |
| Headers | show |
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
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
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 --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 = {
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(-)