Re: [Qemu-devel] [PATCH target-arm v3 04/15] arm: xlnx-zynqmp: Add GIC

[Alistair Francis]

On Mon, Mar 16, 2015 at 10:12 PM, Peter Crosthwaite
<address@hidden> wrote:
> And connect IRQ outputs to the CPUs.
>
> Reviewed-by: Alistair Francis <address@hidden>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>  hw/arm/xlnx-zynqmp.c         | 19 +++++++++++++++++++
>  include/hw/arm/xlnx-zynqmp.h |  2 ++
>  2 files changed, 21 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 41c207a..9465185 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -17,6 +17,11 @@
>
>  #include "hw/arm/xlnx-zynqmp.h"
>
> +#define GIC_NUM_SPI_INTR 128
> +
> +#define GIC_DIST_ADDR       0xf9010000
> +#define GIC_CPU_ADDR        0xf9020000
> +
>  static void xlnx_zynqmp_init(Object *obj)
>  {
>      XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
> @@ -28,6 +33,9 @@ static void xlnx_zynqmp_init(Object *obj)
>          object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),
>                                    &error_abort);
>      }
> +
> +    object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
> +    qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
>  }
>
>  #define ERR_PROP_CHECK_RETURN(err, errp) do { \
> @@ -43,9 +51,20 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>      uint8_t i;
>      Error *err = NULL;
>
> +    qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
> +    qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
> +    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_CPUS);
> +    object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
> +    ERR_PROP_CHECK_RETURN(err, errp);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 0, GIC_DIST_ADDR);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 1, GIC_CPU_ADDR);
> +

Hey Peter,

The GIC here is causing seg faults because it is being connected
before the CPUs.
This is easily fixed by moving the CPU creation first and then connect
the GIC/CPU
lines after that. It shouldn't break anything as the GIC/CPU connections happen
after realize anyway.

See the code below for what works for me (can boot u-boot):

    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
        object_property_set_int(OBJECT(&s->cpu[i]), QEMU_PSCI_CONDUIT_SMC,
                                "psci-conduit", &error_abort);
        if (i > 0) {
            /* Secondary CPUs start in PSCI powered-down state */
            object_property_set_bool(OBJECT(&s->cpu[i]), true,
                                     "start-powered-off", &error_abort);
        }

        object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
        ERR_PROP_CHECK_RETURN(err, errp);
    }

    qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
    qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
    qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_CPUS);
    object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
    ERR_PROP_CHECK_RETURN(err, errp);
    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 0, GIC_DIST_ADDR);
    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 1, GIC_CPU_ADDR);

    for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
                           qdev_get_gpio_in(DEVICE(&s->cpu[i]), ARM_CPU_IRQ));
        irq = qdev_get_gpio_in(DEVICE(&s->gic),
                               arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 0, irq);
        irq = qdev_get_gpio_in(DEVICE(&s->gic),
                               arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
        qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 1, irq);
    }


Just a note that the code above includes everything from the whole patch
series, but you get the idea.

Thanks,

Alistair

>      for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
>          object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
>          ERR_PROP_CHECK_RETURN(err, errp);
> +
> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
> +                           qdev_get_gpio_in(DEVICE(&s->cpu[i]), 
> ARM_CPU_IRQ));
>      }
>  }
>
> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
> index d6b3b92..d29c7de 100644
> --- a/include/hw/arm/xlnx-zynqmp.h
> +++ b/include/hw/arm/xlnx-zynqmp.h
> @@ -2,6 +2,7 @@
>
>  #include "qemu-common.h"
>  #include "hw/arm/arm.h"
> +#include "hw/intc/arm_gic.h"
>
>  #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
>  #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
> @@ -15,6 +16,7 @@ typedef struct XlnxZynqMPState {
>      /*< public >*/
>
>      ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS];
> +    GICState gic;
>  }  XlnxZynqMPState;
>
>  #define XLNX_ZYNQMP_H_
> --
> 2.3.1.2.g90df61e.dirty
>
>