From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60934 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OKsHK-0006sM-C8 for qemu-devel@nongnu.org; Sat, 05 Jun 2010 08:15:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OKsHJ-0002eN-5c for qemu-devel@nongnu.org; Sat, 05 Jun 2010 08:15:06 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:43376) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OKsHI-0002eF-Or for qemu-devel@nongnu.org; Sat, 05 Jun 2010 08:15:05 -0400 Message-ID: <4C0A3FC0.7080803@web.de> Date: Sat, 05 Jun 2010 14:14:56 +0200 From: Jan Kiszka MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [RFT][PATCH 07/15] qemu_irq: Add IRQ handlers with delivery feedback References: <20100530200722.GA6243@redhat.com> <20100531051905.GD24302@redhat.com> <20100601183058.GB6191@redhat.com> <4C074A72.3070507@web.de> <20100603063456.GM24302@redhat.com> <4C0752CB.9030701@web.de> <20100603070300.GN24302@redhat.com> <20100603070559.GO24302@redhat.com> <4C099471.3060507@web.de> <4C0A0A69.9070405@web.de> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig5F69F5B40A60562925D167D3" Sender: jan.kiszka@web.de List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org, Gleb Natapov , Juan Quintela This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig5F69F5B40A60562925D167D3 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Blue Swirl wrote: > On Sat, Jun 5, 2010 at 8:27 AM, Jan Kiszka wrote: >> Blue Swirl wrote: >>> On Sat, Jun 5, 2010 at 12:04 AM, Jan Kiszka wrote= : >>>> Blue Swirl wrote: >>>>> On Thu, Jun 3, 2010 at 7:06 AM, Gleb Natapov wrot= e: >>>>>> 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? T= hen the >>>>>>>>>>> message could specify the originator device, maybe ACK/coales= ce/NACK >>>>>>>>>>> callbacks and a bigger payload than just 1 bit of level. I th= ink that >>>>>>>>>>> could achieve the same coalescing effect as what the bidirect= ional >>>>>>>>>>> IRQ. The payload could be useful for other purposes, for exam= ple >>>>>>>>>>> Sparc64 IRQ messages contain three 64 bit words. >>>>>>>>>> If there are more users than just IRQ de-coalescing, this inde= ed 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. Whoe= ver >>>>>>>>>> terminates or actually delivers the IRQ would invoke the callb= ack. And >>>>>>>>>> platforms like sparc64 could evaluate the additional payload p= ointer in >>>>>>>>>> their irqchips or wherever they need it. IRQ routers on platfo= rms that >>>>>>>>>> make use of these messages would have to replicate them when f= orwarding >>>>>>>>>> an event. >>>>>>>>>> >>>>>>>>>> OK? >>>>>>>>>> >>>>>>>>> Let me see if I understand you correctly. qemu_set_irq() will g= et >>>>>>>>> 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 t= he >>>>>>>> result of the delivery (delivered, masked, coalesced). And yes, = the >>>>>>>> callback will be invoked in the context of the irq handler, so b= efore >>>>>>>> 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_ms= g 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 oc= cur >>>>> 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 eit= her >>>>> 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 st= ack >>>> 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 ca= se >>>> for eio_cb at all. >>> I think it's safer to use allocation model anyway because this will b= e >>> generic code. For example, an intermediate device may want to queue >>> the IRQs. Alternatively, the callbacks could use DeviceState and some= >>> opaque which can be used as the callback context: >>> void (*delivery_cb)(DeviceState *src, void *src_opaque, int result)= ; >>> >>> EOI can be added later if needed, QEMU seems to work fine now without= >>> it. But based on IOAPIC data sheet, I'd suppose it should be need to >>> pass EOI from LAPIC to IOAPIC. It could be used by coalescing as >>> another opportunity to inject IRQs though I guess the guest will ack >>> the IRQ at the same time for both RTC and APIC. >> Let's wait for a real use case for an extended IRQMsg lifetime. For no= w >> we are fine with stack-allocated messages which are way simpler to >> handle. I'm already drafting a first prototype based on this model. >> Switching to dynamic allocation may still happen later on once the >> urgent need shows up. >=20 > Passing around stack allocated objects is asking for trouble. I'd much > rather use the DeviceState/opaque version then, so at least > destination should not need to use IRQMsg for anything. Right, I've hiden the IRQMsg object from the target handler, temporarily storing it in qemu_irq instead. qemu_irq_handler had to be touched anyway, so I'm now passing the IRQ object to it so that it can invoke services to trigger the delivery callback or obtain the payload. Jan --------------enig5F69F5B40A60562925D167D3 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkwKP8UACgkQitSsb3rl5xSvIACfURh5qMSciUWqmX3ICVMSkKC9 6a8AoLqBBoaio27abBg/C7WXo8e2KU81 =qi1w -----END PGP SIGNATURE----- --------------enig5F69F5B40A60562925D167D3--