From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MGczV-0007Kk-6z
	for qemu-devel@nongnu.org; Tue, 16 Jun 2009 14:02:37 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MGczQ-0007IO-1e
	for qemu-devel@nongnu.org; Tue, 16 Jun 2009 14:02:36 -0400
Received: from [199.232.76.173] (port=48134 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MGczP-0007IL-UO
	for qemu-devel@nongnu.org; Tue, 16 Jun 2009 14:02:31 -0400
Received: from mail-bw0-f223.google.com ([209.85.218.223]:64909)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <blauwirbel@gmail.com>) id 1MGczP-0003jC-Bi
	for qemu-devel@nongnu.org; Tue, 16 Jun 2009 14:02:31 -0400
Received: by bwz23 with SMTP id 23so3802263bwz.34
	for <qemu-devel@nongnu.org>; Tue, 16 Jun 2009 11:02:30 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <200906161814.58491.paul@codesourcery.com>
References: <20090616124702.GS19508@redhat.com>
	<200906161814.58491.paul@codesourcery.com>
Date: Tue, 16 Jun 2009 21:02:29 +0300
Message-ID: <f43fc5580906161102x5e0a0bcdme8e275c3bd3084e0@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH] Register usb-uhci reset function.
From: Blue Swirl <blauwirbel@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
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.gnu.org/pipermail/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: Paul Brook <paul@codesourcery.com>
Cc: qemu-devel@nongnu.org, Gleb Natapov <gleb@redhat.com>

On 6/16/09, Paul Brook <paul@codesourcery.com> wrote:
> On Tuesday 16 June 2009, Gleb Natapov wrote:
>  > Update irq line on reset. Reseting irq line is required because
>  > racing irq from pci device will call piix3_set_irq(). piix3_set_irq()
>  > will remember current level in pci_irq_levels[]. The PIC line will be
>  > triggered if one of pci_irq_levels[] is set (depends on piix3 config).
>  > If for instance pci_irq_levels[0] and pci_irq_levels[1] are mapped to
>  > the same PIC irq and during reset pci_irq_levels[1] == 1, but device
>  > that drives pci_irq_levels[0] is initialized first the device driver
>  > will not be able to lower irq line.
>
>
> This is nonsense.
>
>  The only relevant circumstances are if the devices raises an IRQ, and is then
>  reset by software while the system is running. It's got nothing to do with
>  piix3, PCI bus interrupt sharing or system reset. If you are seeing problems
>  after a system reset then your bug lies elsewhere.

No, the scenario Gleb describes is true, piix_pci.c has internal state
for the OR of the IRQs coming in and the state is not reset because
there is no reset handler. This is buggy.

The real fix is to implement piix3 reset correctly, not like Gleb
proposes now but 3/3 of his earlier patches.

As a less attractive alternative, piix3 could store the IRQ state for
each incoming IRQ, not OR state. Then all the devices would need to
call lower_irq to reach correct reset state at piix3.