From patchwork Tue Feb 1 19:58:24 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aurelien Jarno X-Patchwork-Id: 81369 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 64DD3B711B for ; Wed, 2 Feb 2011 07:02:31 +1100 (EST) Received: from localhost ([127.0.0.1]:47858 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PkMPK-0005iy-Vm for incoming@patchwork.ozlabs.org; Tue, 01 Feb 2011 15:00:59 -0500 Received: from [140.186.70.92] (port=60870 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PkMOo-0004w6-Us for qemu-devel@nongnu.org; Tue, 01 Feb 2011 15:00:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PkMMq-0000P2-CT for qemu-devel@nongnu.org; Tue, 01 Feb 2011 14:58:25 -0500 Received: from hall.aurel32.net ([88.191.126.93]:35696) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PkMMq-0000Or-6h for qemu-devel@nongnu.org; Tue, 01 Feb 2011 14:58:24 -0500 Received: from [2001:470:d4ed:0:5e26:aff:fe2b:6f5b] (helo=volta.aurel32.net) by hall.aurel32.net with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1PkMMm-0007Rr-V8; Tue, 01 Feb 2011 20:58:21 +0100 Received: from aurel32 by volta.aurel32.net with local (Exim 4.72) (envelope-from ) id 1PkMMq-0006qm-Nx; Tue, 01 Feb 2011 20:58:25 +0100 Date: Tue, 1 Feb 2011 20:58:24 +0100 From: Aurelien Jarno To: Alexander Graf Subject: Re: [Qemu-devel] [PATCH 7/7] ahci: work around bug with level interrupts Message-ID: <20110201195824.GB17924@volta.aurel32.net> References: <1296571892-12702-1-git-send-email-agraf@suse.de> <1296585301-15510-1-git-send-email-agraf@suse.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1296585301-15510-1-git-send-email-agraf@suse.de> X-Mailer: Mutt 1.5.20 (2009-06-14) User-Agent: Mutt/1.5.20 (2009-06-14) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 88.191.126.93 Cc: Kevin Wolf , Joerg Roedel , qemu-devel Developers , Sebastian Herbszt X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Tue, Feb 01, 2011 at 07:35:01PM +0100, Alexander Graf wrote: > When using level based interrupts, the interrupt is treated the same as an > edge triggered one: leaving the line up does not retrigger the interrupt. > > In fact, when not lowering the line, we won't ever get a new interrupt inside > the guest. So let's always retrigger an interrupt as soon as the OS ack'ed > something on the device. This way we're sure the guest doesn't starve on > interrupts until someone fixes the actual interrupt path. > > Signed-off-by: Alexander Graf > > --- > > v2 -> v3: > > - add comment about the interrupt hack > --- > hw/ide/ahci.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 98bdf70..95e1cf7 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -152,11 +152,15 @@ static void ahci_check_irq(AHCIState *s) > } > } > > + /* XXX We lower the interrupt line here because of a bug with interrupt > + handling in Qemu. When leaving a line up, the interrupt does > + not get retriggered automatically currently. Once that bug is fixed, > + this workaround is not necessary anymore and we only need to lower > + in the else branch. */ > + ahci_irq_lower(s, NULL); > if (s->control_regs.irqstatus && > (s->control_regs.ghc & HOST_CTL_IRQ_EN)) { > ahci_irq_raise(s, NULL); > - } else { > - ahci_irq_lower(s, NULL); > } > } > It's a lot better that way, however I think we should still keep the correct code. Also given this problem only concerns the i386 target (ppc code is actually a bit strange, but at the end does the correct thing), it's something we should probably mention. What about something like that? diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 671b4df..0b153f0 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -489,12 +489,25 @@ static void ahci_check_irq(AHCIState *s) } } +#if 0 if (s->control_regs.irqstatus && (s->control_regs.ghc & HOST_CTL_IRQ_EN)) { ahci_irq_raise(s, NULL); } else { ahci_irq_lower(s, NULL); } +#else + /* XXX We lower the interrupt line here because of a bug with interrupt + handling in Qemu on the i386 target. When leaving a line up, the + interrupt does not get retriggered automatically currently. Once + that bug is fixed, this workaround is not necessary anymore and + we only need to lower in the else branch. */ + ahci_irq_lower(s, NULL); + if (s->control_regs.irqstatus && + (s->control_regs.ghc & HOST_CTL_IRQ_EN)) { + ahci_irq_raise(s, NULL); + } +#endif } static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,