From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33831) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8vZZ-0003Yd-Hp for qemu-devel@nongnu.org; Tue, 04 Sep 2012 12:01:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8vZV-0003Tj-B9 for qemu-devel@nongnu.org; Tue, 04 Sep 2012 12:01:53 -0400 Received: from david.siemens.de ([192.35.17.14]:15418) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8vZU-0003TL-By for qemu-devel@nongnu.org; Tue, 04 Sep 2012 12:01:48 -0400 Message-ID: <504625E5.7090903@siemens.com> Date: Tue, 04 Sep 2012 18:01:41 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1346640974-30974-1-git-send-email-mmogilvi_qemu@miniinfo.net> <1346640974-30974-6-git-send-email-mmogilvi_qemu@miniinfo.net> <50446F9A.4070809@web.de> <5046135B.2080200@redhat.com> In-Reply-To: <5046135B.2080200@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Matthew Ogilvie , "Maciej W. Rozycki" , qemu-devel@nongnu.org On 2012-09-04 16:42, Paolo Bonzini wrote: > Il 04/09/2012 16:29, Maciej W. Rozycki ha scritto: >> So first of all, the *output* of the 8259A is always edge triggered, >> regardless of whether it's the master or one of the slaves (only one slave >> is used in the PC/AT architecture, but up to eight are supported; the >> PC/XT had none). > > I swear I read all your message :) but this seems to be the crux. It means > that something like this ought to fix the bug too. Matthew, can you post > your code or test it? > > diff --git a/hw/i8259.c b/hw/i8259.c > index 53daf78..3dc1dff 100644 > --- a/hw/i8259.c > +++ b/hw/i8259.c > @@ -104,12 +104,11 @@ static void pic_update_irq(PICCommonState *s) > int irq; > > irq = pic_get_irq(s); > + qemu_irq_lower(s->int_out[0]); > if (irq >= 0) { > DPRINTF("pic%d: imr=%x irr=%x padd=%d\n", > s->master ? 0 : 1, s->imr, s->irr, s->priority_add); > qemu_irq_raise(s->int_out[0]); > - } else { > - qemu_irq_lower(s->int_out[0]); > } > } I don't think this can be correct in all scenario. E.g., we also call pic_update_irq in case a level-triggered input was updated but didn't change the output state of the PIC (high-high transition). With your patch, the output will now generate edges. What I'm trying to understand and translate from the description is rather "note that for inputs a high-to-low transition cancels the interrupt as in the level-triggered mode." This is surely not what we do right now. OTOH, I'm afraid that switching to this mode in the PIC can cause problems elsewhere, with devices that actually inject short low-high-low signals. Still wrapping my head around it... Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux