On Wed, Sep 10, 2014 at 12:48 PM, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: > On 10 September 2014 12:43, Christoffer Dall > <christoffer.d...@linaro.org> wrote: >> On Tue, Sep 09, 2014 at 03:53:43PM +0100, Peter Maydell wrote: >>> The pl011 and pl031 devices both use level triggered interrupts, >>> but the device tree we construct was incorrectly telling the >>> kernel to configure the GIC to treat them as edge triggered. >>> This meant that output from the pl011 would hang after a while. >>> >>> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> >>> Cc: qemu-sta...@nongnu.org >>> --- >>> Thanks to Christoffer Dall for figuring out the cause of the hangs here. >>> >>> hw/arm/virt.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index e8f231e..1b343f0 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -371,7 +371,7 @@ static void create_uart(const VirtBoardInfo *vbi, >>> qemu_irq *pic) >>> 2, base, 2, size); >>> qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts", >>> GIC_FDT_IRQ_TYPE_SPI, irq, >>> - GIC_FDT_IRQ_FLAGS_EDGE_LO_HI); >>> + GIC_FDT_IRQ_FLAGS_LEVEL_HI); >>> qemu_fdt_setprop_cells(vbi->fdt, nodename, "clocks", >>> vbi->clock_phandle, vbi->clock_phandle); >>> qemu_fdt_setprop(vbi->fdt, nodename, "clock-names", >>> @@ -398,7 +398,7 @@ static void create_rtc(const VirtBoardInfo *vbi, >>> qemu_irq *pic) >>> 2, base, 2, size); >>> qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts", >>> GIC_FDT_IRQ_TYPE_SPI, irq, >>> - GIC_FDT_IRQ_FLAGS_EDGE_LO_HI); >>> + GIC_FDT_IRQ_FLAGS_LEVEL_HI); >>> qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", >>> vbi->clock_phandle); >>> qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk"); >>> g_free(nodename); >>> -- >>> 1.9.1 >>> >> I've been trying to figure out why we would see this particular hang >> with SMP and not UP (or maybe it is just very much more unlikely to >> happen with UP), but haven't been able to come up with a sequence of >> events to support this yet. It also worries me that we weren't seeing >> this with KVM, since it indicates that we're either doing something >> wrong in the KVM or QEMU GIC emulation code, potentially. >> >> In any case, this patch is correct, so: >> >> Acked-by: Christoffer Dall <christoffer.d...@linaro.org> > > I am still seeing lockups and self detected stalls even with this > patch, and sometimes just hitting a key into the console will get > things moving again. > So I am not convinced yet whether this fixes something fundamentally, > or just works around it by taking an alternative code path through the > kernel which doesn't trigger the same root bug.
As far as I can tell, this fix won't change any other behavior in the kernel than configuring the GICD for that IRQ to level/edge on system boot. I also saw the lockups of the system that went away when giving the pl011 some inputs, but I thought I only saw this before the fix - admittedly I didn't test this very thoroughly. I wonder if we are losing timer interrupts due to the same artifact....? -Christoffer
