From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=52481 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OKoj3-0007dt-O8 for qemu-devel@nongnu.org; Sat, 05 Jun 2010 04:27:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OKoj2-0006BH-6g for qemu-devel@nongnu.org; Sat, 05 Jun 2010 04:27:29 -0400 Received: from fmmailgate01.web.de ([217.72.192.221]:42337) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OKoj1-0006BB-MB for qemu-devel@nongnu.org; Sat, 05 Jun 2010 04:27:28 -0400 Message-ID: <4C0A0A69.9070405@web.de> Date: Sat, 05 Jun 2010 10:27:21 +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> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig7B3DAEE3AC5BE3CBF7278A7D" 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) --------------enig7B3DAEE3AC5BE3CBF7278A7D Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 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? The= n the >>>>>>>>> message could specify the originator device, maybe ACK/coalesce= /NACK >>>>>>>>> callbacks and a bigger payload than just 1 bit of level. I thin= k that >>>>>>>>> could achieve the same coalescing effect as what the bidirectio= nal >>>>>>>>> IRQ. The payload could be useful for other purposes, for exampl= e >>>>>>>>> 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. Whoeve= r >>>>>>>> terminates or actually delivers the IRQ would invoke the callbac= k. And >>>>>>>> platforms like sparc64 could evaluate the additional payload poi= nter in >>>>>>>> their irqchips or wherever they need it. IRQ routers on platform= s that >>>>>>>> make use of these messages would have to replicate them when for= warding >>>>>>>> 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 no= n-NULL >>>>>> callback, it is supposed to be called unconditionally, passing the= >>>>>> result of the delivery (delivered, masked, coalesced). And yes, th= e >>>>>> callback will be invoked in the context of the irq handler, so bef= ore >>>>>> 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 occu= r >>> 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 eithe= r >>> 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 stac= k >> 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. >=20 > I think it's safer to use allocation model anyway because this will be > 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); >=20 > 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 now 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. Jan --------------enig7B3DAEE3AC5BE3CBF7278A7D 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 iEYEARECAAYFAkwKCm0ACgkQitSsb3rl5xRodACgl35mXwUkAfwUiDpnJHfJMFc5 oKQAnjqwmQHonF5EXxxpD/7Z8Zhw2y7k =3MPR -----END PGP SIGNATURE----- --------------enig7B3DAEE3AC5BE3CBF7278A7D--