Re: [RFC PATCH V1 05/12] hvm/dm: Introduce xendevicemodel_set_irq_level DM op

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 18 Aug 2020, Julien Grall wrote:
> On 11/08/2020 23:48, Stefano Stabellini wrote:
> > On Tue, 11 Aug 2020, Julien Grall wrote:
> > > >   I vaguely
> > > > recall a bug 10+ years ago about this with QEMU on x86 and a line that
> > > > could be both active-high and active-low. So QEMU would raise the
> > > > interrupt but Xen would actually think that QEMU stopped the interrupt.
> > > > 
> > > > To do this right, we would have to introduce an interface between Xen
> > > > and QEMU to propagate the trigger type. Xen would have to tell QEMU when
> > > > the guest changed the configuration. That would work, but it would be
> > > > better if we can figure out a way to do without it to reduce complexity.
> > > Per above, I don't think this is necessary.
> > > 
> > > > 
> > > > Instead, given that QEMU and other emulators don't actually care about
> > > > active-high or active-low, if we have a Xen interface that just says
> > > > "fire the interrupt" we get away from this kind of troubles. It would
> > > > also be more efficient because the total number of hypercalls required
> > > > would be lower.
> > > 
> > > I read "fire interrupt" the interrupt as "Please generate an interrupt
> > > once".
> > > Is it what you definition you expect?
> > 
> > Yes, that is the idea. It would have to take into account the edge/level
> > semantic difference: level would have "start it" and a "stop it".
> 
> I am still struggling to see how this can work:
>     - At the moment, QEMU is only providing us the line state. How can we
> deduce the type of the interrupt? Would it mean a major modification of the
> QEMU API?

Good question. 

I don't think we would need any major modifications of the QEMU APIs.
QEMU already uses two different function calls to trigger an edge
interrupt and to trigger a level interrupt.

Edge interrupts are triggered with qemu_irq_pulse; level interrupts with
qemu_irq_raise/qemu_irq_lower.

It is also possible for devices to call qemu_set_irq directly which
only has the state of the line represented by the "level" argument.
As far as I can tell all interrupts emulated in QEMU (at least the ones
we care about) are active-high.

We have a couple of choices in the implementation, like hooking into
qemu_irq_pulse, and/or checking if the interrupt is level or edge in the
xen interrupt injection function. The latter shouldn't require any
changes in QEMU common code.


FYI looking into the code there is something "strange" in virtio-mmio.c:
it only ever calls qemu_set_irq to start a notification. It doesn't look
like it ever calls qemu_set_irq to stop a notification at all. It is
possible that the state of the line is not accurately emulated for
virtio-mmio.c.


>     - Can you provide a rough sketch how this could be implemented in Xen?

It would work similarly to other emulated interrupt injections on the
Xen side, calling vgic_inject_irq.  We have matching info about
level/edge and active-high/active-low in Xen too, so we could do more
precise emulation of the interrupt flow, although I am aware of the
current limitations of the vgic in that regard.

But I have the feeling I didn't address your concern :-)