From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=51867 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PgdHE-0001pc-FR
	for qemu-devel@nongnu.org; Sat, 22 Jan 2011 08:13:13 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <aurelien@aurel32.net>) id 1PgdHD-0004t9-6K
	for qemu-devel@nongnu.org; Sat, 22 Jan 2011 08:13:12 -0500
Received: from hall.aurel32.net ([88.191.126.93]:58706)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <aurelien@aurel32.net>) id 1PgdHC-0004t3-RL
	for qemu-devel@nongnu.org; Sat, 22 Jan 2011 08:13:11 -0500
Date: Sat, 22 Jan 2011 14:13:03 +0100
From: Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] Re: [PATCH 8/8] ahci: fix !msi interrupts
Message-ID: <20110122131303.GI27195@hall.aurel32.net>
References: <1292879604-22268-9-git-send-email-agraf@suse.de>
	<4D34542D.7080301@siemens.com> <4D3467B9.3070207@suse.de>
	<4D346834.8000903@siemens.com> <4D3468A0.3060709@suse.de>
	<4D346C31.8060300@siemens.com> <4D346F5B.7090502@suse.de>
	<4D35587F.3050503@redhat.com>
	<2006A7EF-07DC-4DAA-B6ED-88D24D6B7978@suse.de>
	<4D358E71.3040905@siemens.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <4D358E71.3040905@siemens.com>
Sender: Aurelien Jarno <aurelien@aurel32.net>
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Joerg Roedel <Joerg.Roedel@amd.com>, qemu-devel Developers <qemu-devel@nongnu.org>, Alexander Graf <agraf@suse.de>, Gerd Hoffmann <kraxel@redhat.com>, Sebastian Herbszt <herbszt@gmx.de>

On Tue, Jan 18, 2011 at 01:58:25PM +0100, Jan Kiszka wrote:
> On 2011-01-18 13:05, Alexander Graf wrote:
> > 
> > On 18.01.2011, at 10:08, Gerd Hoffmann wrote:
> > 
> >>  Hi,
> >>
> >>>> Worse might also be that unknown issue that force you to inject an IRQ
> >>>> here. We don't know. That's probably worst.
> >>>
> >>> Well, IIRC the issue was that usually a level high interrupt line would
> >>> simply retrigger an interrupt after enabling the interrupt line in the
> >>> APIC again.
> >>
> >> edge triggered interrupts wouldn't though.
> > 
> > The code change doesn't change anything for edge triggered interrupts - they work fine. Only !msi (== level) ones are broken.
> > 
> >>
> >>> Unless my memory completely fails on me, that didn't happen,
> >>> so I added the manual retrigger on a partial command ACK in ahci code.
> >>
> >> That re-trigger smells like you are not clearing the interrupt line where you should.  For starters try calling ahci_check_irq() after guest writes to PORT_IRQ_STAT.
> > 
> > The problem happened when I had the following:
> > 
> > ahci irq bits = 0
> > <events happen>
> > ahci irq bits = 1 | 2
> > irq line trigger
> > guest clears 1
> > ahci bits = 2
> > <no irq line trigger, since we're still irq high>
> > 
> > On normal hardware, the high state of the irq line would simply trigger another interrupt in the guest when it gets ACKed on the LAPIC. Somehow it doesn't get triggered here. I don't see why clearing the interrupt line would help? It's not what real hardware would do, no?
> > 
> 
> No, real devices would simply leave the line asserted as long as there
> is a reason to.
> 
> So again my question: With which irqchips do you see this effect, kvm's
> in-kernel model and/or qemu's user space model? If there is an issue
> with retriggering a CPU interrupt while the source is still asserted,
> that probably needs to be fixed.
> 

I guess it might be related to a problem identified long time ago on
some targets, and that leads to the following #ifdef in i8259.c:

| /* all targets should do this rather than acking the IRQ in the cpu */
| #if defined(TARGET_MIPS) || defined(TARGET_PPC) || defined(TARGET_ALPHA)

For your information it has been fixed on MIPS in commit 
4de9b249d37c1b382cc3e5a21fad1b4a11cec2fa.

Basically when an interrupt line is high, the interrupt is getting
delivered to the CPU. This part works correctly on x86. The CPU will
take a corresponding action, basically either disabling the interrupt
at the CPU or controller level or doing something on the device so that
it lower its IRQ line. This new IRQ line level should then propagate
through the various controllers, which should also lower their IRQ line
if no other interrupt line are active. This ACK process should then
continue up to the CPU.

For x86 the interrupt state is cleared as soon as the interrupt is
signaled to the CPU (see cpu-exec.c line 407), therefore if an interrupt
is still pending, it won't be seen by the CPU. It's probably what you
observed with AHCI. 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net