[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH target-arm v3 04/15] arm: xlnx-zynqmp: Add GIC
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH target-arm v3 04/15] arm: xlnx-zynqmp: Add GIC |
Date: |
Wed, 18 Mar 2015 18:41:29 +0530 |
On Wed, Mar 18, 2015 at 10:26 AM, Alistair Francis
<address@hidden> wrote:
> 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.
So I actually bisected this as a recent regression on:
commit a464982499b2f637f6699e3d03e0a9d2e0b5288b (refs/bisect/bad)
Author: Paolo Bonzini <address@hidden>
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 <address@hidden>
Tested-by: Michael Roth <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
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
>>
>>
>
[Qemu-devel] [PATCH target-arm v3 02/15] target-arm: cpu64: Add support for cortex-a53, Peter Crosthwaite, 2015/03/16
[Qemu-devel] [PATCH target-arm v3 03/15] arm: Introduce Xilinx ZynqMP SoC, Peter Crosthwaite, 2015/03/16
[Qemu-devel] [PATCH target-arm v3 01/15] target-arm: cpu64: Factor out ARM cortex init, Peter Crosthwaite, 2015/03/16
[Qemu-devel] [PATCH target-arm v3 13/15] arm: xilinx-ep108: Add external RAM, Peter Crosthwaite, 2015/03/16
[Qemu-devel] [PATCH target-arm v3 06/15] net: cadence_gem: Clean up variable names, Peter Crosthwaite, 2015/03/16
[Qemu-devel] [PATCH target-arm v3 10/15] char: cadence_uart: Split state struct and type into header, Peter Crosthwaite, 2015/03/16
[Qemu-devel] [PATCH target-arm v3 09/15] char: cadence_uart: Clean up variable names, Peter Crosthwaite, 2015/03/16