Re: [Qemu-devel] [Qemu-arm] [PATCH] pl011: do not put into fifo before enabled the interruption

[Wei Xu]

Hi Peter,

On 2018/1/26 17:15, Peter Maydell wrote:
> On 26 January 2018 at 17:05, Wei Xu <address@hidden> wrote:
>> On 2018/1/26 16:36, Peter Maydell wrote:
>>> If the user presses keys before interrupts are enabled,
>>> what ought to happen is:
>>>  * we put the key in the FIFO, and update the int_level flags
>>>  * when the FIFO is full, can_receive starts returning 0 and
>>>    QEMU stops passing us new characters
>>>  * when the guest driver for the pl011 initializes the
>>>    device and enables interrupts then either:
>>>     (a) it does something that clears the FIFO, which will
>>>     mean can_receive starts allowing new chars again, or
>>>     (b) it leaves the FIFO as it is, and we should thus
>>>     immediately raise an interrupt for the characters still
>>>     in the FIFO; when the guest handles this interrupt and
>>>     gets the characters, can_receive will permit new ones
>>>
>>
>> Yes, now it is handled like b.
>>
>>> What is happening in your situation that means this is not
>>> working as expected ?
>>
>> But in the kernel side, the pll011 is triggered as a level interruption.
>> During the booting, if any key is pressed ,the call stack is as below:
>> QEMU side:
>> pl011_update
>> -->qemu_set_irq(level as 0)
>> ---->kvm_arm_gic_set_irq
>>
>> Kernel side:
>> kvm_vm_ioctl_irq_line
>> -->kvm_vgic_inject_irq
>> ---->vgic_validate_injection (if level did not change, return)
>> ---->vgic_queue_irq_unlock
>>
>> Without above changes, in the vgic_validate_injection, because the
>> interruption level is always 0, this irq will not be queued into vgic.
>> And the guest will not read the pl011 fifo.
> 
> The pl011 code should call qemu_set_irq(..., 1) when the
> guest enables interrupts on the device by writing to the int_enabled
> (UARTIMSC) register. That will be a 0-to-1 level change and the KVM
> VGIC should report the interrupt to the guest.
> 

Yes.
And in the pl011_update, the irq level is set by s->int_level & s->int_enabled.
When writing to the int_enabled, not sure why the int_level is set to
0x20(PL011_INT_TX) but int_enabled is 0x50.
It still call qemu_set_irq(..., 0).

I added "s->int_level |= PL011_INT_RX" before calling pl011_update
when writing to the int_enabled and tested it also works.
How do you think about like that?
Thanks!

Best Regards,
Wei

> thanks
> -- PMM
> 
> .
>