Re: [Qemu-devel] [PATCH] hw/arm/virt: fix pl011 and pl031 irq flags

[Ard Biesheuvel]

On 10 September 2014 12:43, Christoffer Dall
<address@hidden> 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 <address@hidden>
>> Cc: address@hidden
>> ---
>> 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 <address@hidden>

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.