Re: [Qemu-devel] Re: [RFT][PATCH 07/15] qemu_irq: Add IRQ handlers with delivery feedback

[Gleb Natapov]

On Thu, Jun 03, 2010 at 10:03:00AM +0300, Gleb Natapov wrote:
> On Thu, Jun 03, 2010 at 08:59:23AM +0200, Jan Kiszka wrote:
> > Gleb Natapov wrote:
> > > On Thu, Jun 03, 2010 at 08:23:46AM +0200, Jan Kiszka wrote:
> > >> Blue Swirl wrote:
> > >>> But how about if we introduced instead a message based IRQ? Then the
> > >>> message could specify the originator device, maybe ACK/coalesce/NACK
> > >>> callbacks and a bigger payload than just 1 bit of level. I think that
> > >>> could achieve the same coalescing effect as what the bidirectional
> > >>> IRQ. The payload could be useful for other purposes, for example
> > >>> Sparc64 IRQ messages contain three 64 bit words.
> > >> If there are more users than just IRQ de-coalescing, this indeed sounds
> > >> superior. We could pass objects like this one around:
> > >>
> > >> struct qemu_irq_msg {
> > >>  void (*delivery_cb)(int result);
> > >>  void *payload;
> > >> };
> > >>
> > >> They would be valid within the scope of the IRQ handlers. Whoever
> > >> terminates or actually delivers the IRQ would invoke the callback. And
> > >> platforms like sparc64 could evaluate the additional payload pointer in
> > >> their irqchips or wherever they need it. IRQ routers on platforms that
> > >> make use of these messages would have to replicate them when forwarding
> > >> an event.
> > >>
> > >> OK?
> > >>
> > > Let me see if I understand you correctly. qemu_set_irq() will get
> > > additional parameter qemu_irq_msg and if irq was not coalesced
> > > delivery_cb is called, so there is a guaranty that if delivery_cb is
> > > called it is done before qemu_set_irq() returns. Correct?
> > 
> > If the side that triggers an IRQ passes a message object with a non-NULL
> > callback, it is supposed to be called unconditionally, passing the
> > result of the delivery (delivered, masked, coalesced). And yes, the
> > callback will be invoked in the context of the irq handler, so before
> > qemu_set_irq (or rather some new qemu_set_irq_msg) returns.
> > 
> Looks fine to me.
> 
Except that delivery_cb should probably get pointer to qemu_irq_msg as a
parameter.

--
                        Gleb.