[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V3 18/23] xen/dm: Introduce xendevicemodel_set_irq_level DM op



Hi Stefano,

On 10/12/2020 02:21, Stefano Stabellini wrote:
On Mon, 30 Nov 2020, Oleksandr Tyshchenko wrote:
From: Julien Grall <julien.grall@xxxxxxx>

This patch adds ability to the device emulator to notify otherend
(some entity running in the guest) using a SPI and implements Arm
specific bits for it. Proposed interface allows emulator to set
the logical level of a one of a domain's IRQ lines.

We can't reuse the existing DM op (xen_dm_op_set_isa_irq_level)
to inject an interrupt as the "isa_irq" field is only 8-bit and
able to cover IRQ 0 - 255, whereas we need a wider range (0 - 1020).

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

---
Please note, this is a split/cleanup/hardening of Julien's PoC:
"Add support for Guest IO forwarding to a device emulator"

***
Please note, I left interface untouched since there is still
an open discussion what interface to use/what information to pass
to the hypervisor. The question whether we should abstract away
the state of the line or not.
***

Let's start with a simple question: is this going to work with
virtio-mmio emulation in QEMU that doesn't lower the state of the line
to end the notification (only calls qemu_set_irq(irq, high))?

See: hw/virtio/virtio-mmio.c:virtio_mmio_update_irq

Hmmm my version of QEMU is using:

level = (qatomic_read(&vdev->isr) != 0);
trace_virtio_mmio_setting_irq(level);
qemu_set_irq(proxy->irq, level);

So QEMU will raise/lower the interrupt based on whether there are pending still interrupts.



Alex (CC'ed) might be able to confirm whether I am reading the QEMU code
correctly. Assuming that it is true that QEMU is only raising the level,
never lowering it, although the emulation is obviously not correct, I
would rather keep QEMU as is for efficiency reasons, and because we
don't want to deviate from the common implementation in QEMU.


Looking at this patch and at vgic_inject_irq, yes, I think it would
work as is.

Our implementation of vgic_inject_irq() is completely bogus as soon as you deal with level interrupt. We are getting away so far, because there are not many fully emulated level interrupt (AFAIK this would only be the pl011). In fact, we carry a gross hack in the emulation to handle them.

In case of the level interrupt, we should keep injecting the interrupt to the guest until the line was lowered down (e.g qemu_set_irq(irq, 0) assuming active-high).



So it looks like we are going to end up with an interface that:

- in theory it is modelling the line closely

For level interrupt we need to know whether the line is low or high. I am struggling to see how this would work if we consider the variable as "trigger".

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.