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.
