From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=36810 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OM3kv-0003Qv-VH for qemu-devel@nongnu.org; Tue, 08 Jun 2010 14:42:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OM3kq-0006aB-U9 for qemu-devel@nongnu.org; Tue, 08 Jun 2010 14:42:33 -0400 Received: from mail-ww0-f45.google.com ([74.125.82.45]:43870) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OM3kq-0006Zu-N4 for qemu-devel@nongnu.org; Tue, 08 Jun 2010 14:42:28 -0400 Received: by wwb13 with SMTP id 13so18751wwb.4 for ; Tue, 08 Jun 2010 11:42:26 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1275258946-15739-1-git-send-email-atar4qemu@gmail.com> From: Artyom Tarasenko Date: Fri, 4 Jun 2010 22:30:40 +0200 Message-ID: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCH] sparc32 esp fix spurious interrupts in chip reset List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org 2010/6/4 Blue Swirl : > On Tue, Jun 1, 2010 at 8:16 PM, Artyom Tarasenko > wrote: >> 2010/6/1 Blue Swirl : >>> On Tue, Jun 1, 2010 at 7:56 PM, Artyom Tarasenko >>> wrote: >>>> 2010/6/1 Blue Swirl : >>>>> On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko >>>>> wrote: >>>>>> lower interrupt during chip reset. Otherwise the ESP_RSTAT register >>>>>> may get out of sync with the IRQ line status. This effect became >>>>>> visible after commit 65899fe3 >>>>> >>>>> Hard reset handlers should not touch qemu_irqs, because on cold start= , >>>>> the receiving end may be unprepared to handle the signal. >>>> >>>> Wouldn't the real hardware lower irq on the hardware reset? >>> >>> Yes, but since qemu_irqs have no state, and on a cold start or system >>> reset all other devices are guaranteed to be reset, the callback would >>> be useless. >>> >>>> And if it would not, would it still clear the corresponding bit in >>>> the ESP_RSTAT register? >>> >>> All registers are set to zero in the lines below. >>> >>>> >>>>> See >>>>> 0d0a7e69e853639b123798877e019c3c7ee6634a, >>>>> bc26e55a6615dc594be425d293db40d5cdcdb84b and >>>>> 42f1ced228c9b616cfa2b69846025271618e4ef5. >>>>> >>>>> For ESP there are two other sources of reset: signal from DMA and chi= p >>>>> reset command. On those cases, lowering IRQ makes sense. >>>>> >>>>> So the correct fix is to refactor the reset handling a bit. Does this >>>>> patch also fix your test case? >>>> >>>> It does, but >>>> >>>> +static void esp_soft_reset(DeviceState *d) >>>> +{ >>>> + =A0 =A0ESPState *s =3D container_of(d, ESPState, busdev.qdev); >>>> + >>>> + =A0 =A0qemu_irq_lower(s->irq); >>>> >>>> Shouldn't it be esp_lower_irq(s)? What's going to happen to the >>>> DMA_INTR bit if dma was the source of the irq? >>> >>> Again, the registers are zeroed in esp_hard_reset(). >> >> How does it zero the _DMA_ registers? And sparc32_dma does share the >> IRQ line with ESP, doesn't it? > > I'd suppose DMA registers are separate and they would not be cleared > by for example ESP chip reset command. The IRQ goes from ESP to DMA, > DMA has another line going to interrupt controller. But do we have separate DMA lines in qemu? If we do, I'm absolutely fine wi= th qemu_irq_lower(s->irq) . If we don't, imagine the following scenario: DMA rises an IRQ, then esp chip reset happens, and then... DMA can't rise the IRQ anymore. --=20 Regards, Artyom Tarasenko solaris/sparc under qemu blog: http://tyom.blogspot.com/