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

[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).

-- PMM