Re: [Qemu-devel] [PATCH v4 24/24] arm11mpcore: Split off RealView MPCore

[Andreas Färber]

Am 13.09.2013 17:33, schrieb Peter Maydell:
> On 11 September 2013 15:37, Andreas Färber <address@hidden> wrote:
>> Signed-off-by: Andreas Färber <address@hidden>
>> ---
>>  hw/cpu/Makefile.objs     |   1 +
>>  hw/cpu/arm11mpcore.c     | 121 -----------------------------------------
>>  hw/cpu/realview_mpcore.c | 139 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 140 insertions(+), 121 deletions(-)
>>  create mode 100644 hw/cpu/realview_mpcore.c
>>
>> diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
>> index df287c1..6381238 100644
>> --- a/hw/cpu/Makefile.objs
>> +++ b/hw/cpu/Makefile.objs
>> @@ -1,4 +1,5 @@
>>  obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
>> +obj-$(CONFIG_REALVIEW) += realview_mpcore.o
>>  obj-$(CONFIG_A9MPCORE) += a9mpcore.o
>>  obj-$(CONFIG_A15MPCORE) += a15mpcore.o
>>  obj-$(CONFIG_ICC_BUS) += icc_bus.o
>> diff --git a/hw/cpu/arm11mpcore.c b/hw/cpu/arm11mpcore.c
>> index 0ec27c7..717d3e4 100644
>> --- a/hw/cpu/arm11mpcore.c
>> +++ b/hw/cpu/arm11mpcore.c
>> @@ -134,126 +134,6 @@ static void mpcore_priv_initfn(Object *obj)
>>      qdev_set_parent_bus(DEVICE(&s->wdtimer), sysbus_get_default());
>>  }
>>
>> -#define TYPE_REALVIEW_MPCORE_RIRQ "realview_mpcore"
>> -#define REALVIEW_MPCORE_RIRQ(obj) \
>> -    OBJECT_CHECK(mpcore_rirq_state, (obj), TYPE_REALVIEW_MPCORE_RIRQ)
>> -
>> -/* Dummy PIC to route IRQ lines.  The baseboard has 4 independent IRQ
>> -   controllers.  The output of these, plus some of the raw input lines
>> -   are fed into a single SMP-aware interrupt controller on the CPU.  */
>> -typedef struct {
>> -    SysBusDevice parent_obj;
>> -
>> -    qemu_irq cpuic[32];
>> -    qemu_irq rvic[4][64];
>> -    uint32_t num_cpu;
>> -
>> -    ARM11MPCorePriveState priv;
>> -    RealViewGICState gic[4];
>> -} mpcore_rirq_state;
>> -
>> -/* Map baseboard IRQs onto CPU IRQ lines.  */
>> -static const int mpcore_irq_map[32] = {
>> -    -1, -1, -1, -1,  1,  2, -1, -1,
>> -    -1, -1,  6, -1,  4,  5, -1, -1,
>> -    -1, 14, 15,  0,  7,  8, -1, -1,
>> -    -1, -1, -1, -1,  9,  3, -1, -1,
>> -};
>> -
>> -static void mpcore_rirq_set_irq(void *opaque, int irq, int level)
>> -{
>> -    mpcore_rirq_state *s = (mpcore_rirq_state *)opaque;
>> -    int i;
>> -
>> -    for (i = 0; i < 4; i++) {
>> -        qemu_set_irq(s->rvic[i][irq], level);
>> -    }
>> -    if (irq < 32) {
>> -        irq = mpcore_irq_map[irq];
>> -        if (irq >= 0) {
>> -            qemu_set_irq(s->cpuic[irq], level);
>> -        }
>> -    }
>> -}
>> -
>> -static void realview_mpcore_realize(DeviceState *dev, Error **errp)
>> -{
>> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> -    mpcore_rirq_state *s = REALVIEW_MPCORE_RIRQ(dev);
>> -    DeviceState *priv = DEVICE(&s->priv);
>> -    DeviceState *gic;
>> -    SysBusDevice *gicbusdev;
>> -    Error *err = NULL;
>> -    int n;
>> -    int i;
>> -
>> -    qdev_prop_set_uint32(priv, "num-cpu", s->num_cpu);
>> -    object_property_set_bool(OBJECT(&s->priv), true, "realized", &err);
>> -    if (err != NULL) {
>> -        error_propagate(errp, err);
>> -        return;
>> -    }
>> -    sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->priv));
>> -    for (i = 0; i < 32; i++) {
>> -        s->cpuic[i] = qdev_get_gpio_in(priv, i);
>> -    }
>> -    /* ??? IRQ routing is hardcoded to "normal" mode.  */
>> -    for (n = 0; n < 4; n++) {
>> -        object_property_set_bool(OBJECT(&s->gic[n]), true, "realized", 
>> &err);
>> -        if (err != NULL) {
>> -            error_propagate(errp, err);
>> -            return;
>> -        }
>> -        gic = DEVICE(&s->gic[n]);
>> -        gicbusdev = SYS_BUS_DEVICE(&s->gic[n]);
>> -        sysbus_mmio_map(gicbusdev, 0, 0x10040000 + n * 0x10000);
>> -        sysbus_connect_irq(gicbusdev, 0, s->cpuic[10 + n]);
>> -        for (i = 0; i < 64; i++) {
>> -            s->rvic[n][i] = qdev_get_gpio_in(gic, i);
>> -        }
>> -    }
>> -    qdev_init_gpio_in(dev, mpcore_rirq_set_irq, 64);
>> -}
>> -
>> -static void mpcore_rirq_init(Object *obj)
>> -{
>> -    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> -    mpcore_rirq_state *s = REALVIEW_MPCORE_RIRQ(obj);
>> -    SysBusDevice *privbusdev;
>> -    int i;
>> -
>> -    object_initialize(&s->priv, sizeof(s->priv), TYPE_ARM11MPCORE_PRIV);
>> -    qdev_set_parent_bus(DEVICE(&s->priv), sysbus_get_default());
>> -    privbusdev = SYS_BUS_DEVICE(&s->priv);
>> -    sysbus_init_mmio(sbd, sysbus_mmio_get_region(privbusdev, 0));
>> -
>> -    for (i = 0; i < 4; i++) {
>> -        object_initialize(&s->gic[i], sizeof(s->gic[i]), TYPE_REALVIEW_GIC);
>> -        qdev_set_parent_bus(DEVICE(&s->gic[i]), sysbus_get_default());
>> -    }
>> -}
>> -
>> -static Property mpcore_rirq_properties[] = {
>> -    DEFINE_PROP_UINT32("num-cpu", mpcore_rirq_state, num_cpu, 1),
>> -    DEFINE_PROP_END_OF_LIST(),
>> -};
>> -
>> -static void mpcore_rirq_class_init(ObjectClass *klass, void *data)
>> -{
>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>> -
>> -    dc->realize = realview_mpcore_realize;
>> -    dc->props = mpcore_rirq_properties;
>> -}
>> -
>> -static const TypeInfo mpcore_rirq_info = {
>> -    .name          = TYPE_REALVIEW_MPCORE_RIRQ,
>> -    .parent        = TYPE_SYS_BUS_DEVICE,
>> -    .instance_size = sizeof(mpcore_rirq_state),
>> -    .instance_init = mpcore_rirq_init,
>> -    .class_init    = mpcore_rirq_class_init,
>> -};
>> -
>>  static Property mpcore_priv_properties[] = {
>>      DEFINE_PROP_UINT32("num-cpu", ARM11MPCorePriveState, num_cpu, 1),
>>      /* The ARM11 MPCORE TRM says the on-chip controller may have
>> @@ -286,7 +166,6 @@ static const TypeInfo mpcore_priv_info = {
>>
>>  static void arm11mpcore_register_types(void)
>>  {
>> -    type_register_static(&mpcore_rirq_info);
>>      type_register_static(&mpcore_priv_info);
>>  }
>>
>> diff --git a/hw/cpu/realview_mpcore.c b/hw/cpu/realview_mpcore.c
>> new file mode 100644
>> index 0000000..c39a2da
>> --- /dev/null
>> +++ b/hw/cpu/realview_mpcore.c
>> @@ -0,0 +1,139 @@
>> +/*
>> + * RealView ARM11MPCore internal peripheral emulation
> 
> This isn't really internal to anything, or at least if
> properly modelled it shouldn't be. It's part of the
> board: as the comment notes there are 4 GIC instances
> on the board, which are wired up to the 11MPCore's
> internal GIC. Ideally it ought to be untangled and
> it shouldn't specifically own an 11MPCorePrivState
> object. That said, we were already mismodelling this
> I think so I'm happy to let this patch go through as-is.
> 
>> +    ARM11MPCorePriveState priv;
> 
> You might add a patch somewhere fixing the typo
> (PriveState->PrivState).

Will do, thanks.

Background of the split is an investigation of dc->vmsd assignments for
my VMState RFC. Similar split-offs for board-private devices will
follow, e.g., musicpal.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg