On Wed, Mar 18, 2015 at 10:26 AM, Alistair Francis <alistair.fran...@xilinx.com> wrote: > On Mon, Mar 16, 2015 at 10:12 PM, Peter Crosthwaite > <peter.crosthwa...@xilinx.com> wrote: >> And connect IRQ outputs to the CPUs. >> >> Reviewed-by: Alistair Francis <alistair.fran...@xilinx.com> >> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com> >> --- >> 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.
So I actually bisected this as a recent regression on: commit a464982499b2f637f6699e3d03e0a9d2e0b5288b (refs/bisect/bad) Author: Paolo Bonzini <pbonz...@redhat.com> Date: Wed Feb 11 17:15:18 2015 +0100 rcu: run RCU callbacks under the BQL This needs to go away sooner or later, but one complication is the complex VFIO data structures that are modified in instance_finalize. Take a shortcut for now. Reviewed-by: Michael Roth <mdr...@linux.vnet.ibm.com> Tested-by: Michael Roth <mdr...@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> Segfault backtrace: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffecd57700 (LWP 20522)] 0x0000555555621580 in qemu_cpu_kick_thread (cpu=0x7ffff7f24088) at /workspaces/pcrost/ssiv/qemu-mainline/cpus.c:1052 1052 err = pthread_kill(cpu->thread->thread, SIG_IPI); (gdb) bt #0 0x0000555555621580 in qemu_cpu_kick_thread (cpu=0x7ffff7f24088) at /workspaces/pcrost/ssiv/qemu-mainline/cpus.c:1052 #1 0x0000555555622a48 in qemu_mutex_lock_iothread () at /workspaces/pcrost/ssiv/qemu-mainline/cpus.c:1127 #2 0x00005555558d6423 in call_rcu_thread (opaque=<optimized out>) at util/rcu.c:241 #3 0x00007ffff40600a5 in start_thread (arg=0x7fffecd57700) at pthread_create.c:309 #4 0x00007ffff3d8dcfd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111 (gdb) I have sent an RFC for a patch that should fix it. > This is easily fixed by moving the CPU creation first and then connect > the GIC/CPU This will work too, but I think having requirements on ordering of dev inits is fragile and we should avoid it where possible. I'll do your proposed re-ordering if the fix doesn't get through. Regards, Peter > 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 >> >> >