Blue Swirl wrote: > On Thu, Jun 3, 2010 at 7:06 AM, Gleb Natapov <g...@redhat.com> wrote: >> 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. > > I'd like to also support EOI handling. When the guest clears the > interrupt condtion, the EOI callback would be called. This could occur > much later than the IRQ delivery time. I'm not sure if we need the > result code in that case. > > If any intermediate device (IOAPIC?) needs to be informed about either > delivery or EOI also, it could create a proxy message with its > callbacks in place. But we need then a separate opaque field (in > addition to payload) to store the original message. > > struct IRQMsg { > DeviceState *src; > void (*delivery_cb)(IRQMsg *msg, int result); > void (*eoi_cb)(IRQMsg *msg, int result); > void *src_opaque; > void *payload; > };
Extending the lifetime of IRQMsg objects beyond the delivery call stack means qemu_malloc/free for every delivery. I think it takes a _very_ appealing reason to justify this. But so far I do not see any use case for eio_cb at all. Jan
signature.asc
Description: OpenPGP digital signature