Re: Virtio in Xen on Arm (based on IOREQ concept)

[André Przywara <andre.przywara@xxxxxxx>]

On 21/07/2020 17:09, Oleksandr wrote:
> 
> On 21.07.20 17:58, André Przywara wrote:
>> On 21/07/2020 15:52, Oleksandr wrote:
>>> On 21.07.20 17:32, André Przywara wrote:
>>>> On 21/07/2020 14:43, Julien Grall wrote:
>>> Hello Andre, Julien
>>>
>>>
>>>>> (+ Andre)
>>>>>
>>>>> Hi Oleksandr,
>>>>>
>>>>> On 21/07/2020 13:26, Oleksandr wrote:
>>>>>> On 20.07.20 23:38, Stefano Stabellini wrote:
>>>>>>> For instance, what's your take on notifications with virtio-mmio?
>>>>>>> How
>>>>>>> are they modelled today? Are they good enough or do we need MSIs?
>>>>>> Notifications are sent from device (backend) to the driver (frontend)
>>>>>> using interrupts. Additional DM function was introduced for that
>>>>>> purpose xendevicemodel_set_irq_level() which results in
>>>>>> vgic_inject_irq() call.
>>>>>>
>>>>>> Currently, if device wants to notify a driver it should trigger the
>>>>>> interrupt by calling that function twice (high level at first, then
>>>>>> low level).
>>>>> This doesn't look right to me. Assuming the interrupt is trigger when
>>>>> the line is high-level, the backend should only issue the hypercall
>>>>> once
>>>>> to set the level to high. Once the guest has finish to process all the
>>>>> notifications the backend would then call the hypercall to lower the
>>>>> interrupt line.
>>>>>
>>>>> This means the interrupts should keep firing as long as the interrupt
>>>>> line is high.
>>>>>
>>>>> It is quite possible that I took some shortcut when implementing the
>>>>> hypercall, so this should be corrected before anyone start to rely on
>>>>> it.
>>>> So I think the key question is: are virtio interrupts level or edge
>>>> triggered? Both QEMU and kvmtool advertise virtio-mmio interrupts as
>>>> edge-triggered.
>>>>   From skimming through the virtio spec I can't find any explicit
>>>> mentioning of the type of IRQ, but the usage of MSIs indeed hints at
>>>> using an edge property. Apparently reading the PCI ISR status register
>>>> clears it, which again sounds like edge. For virtio-mmio the driver
>>>> needs to explicitly clear the interrupt status register, which again
>>>> says: edge (as it's not the device clearing the status).
>>>>
>>>> So the device should just notify the driver once, which would cause one
>>>> vgic_inject_irq() call. It would be then up to the driver to clear up
>>>> that status, by reading PCI ISR status or writing to virtio-mmio's
>>>> interrupt-acknowledge register.
>>>>
>>>> Does that make sense?
>>> When implementing Xen backend, I didn't have an already working example
>>> so only guessed. I looked how kvmtool behaved when actually triggering
>>> the interrupt on Arm [1].
>>>
>>> Taking into the account that Xen PoC on Arm advertises [2] the same irq
>>> type (TYPE_EDGE_RISING) as kvmtool [3] I decided to follow the model of
>>> triggering an interrupt. Could you please explain, is this wrong?
>> Yes, kvmtool does a double call needlessly (on x86, ppc and arm, mips is
>> correct).
>> I just chased it down in the kernel, a KVM_IRQ_LINE ioctl with level=low
>> is ignored when the target IRQ is configured as edge (which it is,
>> because the DT says so), check vgic_validate_injection() in the kernel.
>>
>> So you should only ever need one call to set the line "high" (actually:
>> trigger the edge pulse).
> 
> Got it, thanks for the explanation. Have just removed an extra action
> (setting low level) and checked.
> 

Just for the records: the KVM API documentation explicitly mentions:
"Note that edge-triggered interrupts require the level to be set to 1
and then back to 0." So kvmtool is just following the book.

Setting it to 0 still does nothing *on ARM*, and the x86 IRQ code is far
to convoluted to easily judge what's really happening here. For MSIs at
least it's equally ignored.

So I guess a clean implementation in Xen does not need two calls, but
some folks with understanding of x86 IRQ handling in Xen should confirm.

Cheers,
Andre.