From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=43649 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OJXmx-0007g2-GL
	for qemu-devel@nongnu.org; Tue, 01 Jun 2010 16:10:16 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <blauwirbel@gmail.com>) id 1OJXmv-00063j-V9
	for qemu-devel@nongnu.org; Tue, 01 Jun 2010 16:10:15 -0400
Received: from mail-pz0-f196.google.com ([209.85.222.196]:56737)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <blauwirbel@gmail.com>) id 1OJXmv-00062W-Ql
	for qemu-devel@nongnu.org; Tue, 01 Jun 2010 16:10:13 -0400
Received: by pzk34 with SMTP id 34so3090105pzk.29
	for <qemu-devel@nongnu.org>; Tue, 01 Jun 2010 13:10:06 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <AANLkTik6S4wLCqFcFmcnFV_fsIa77JPCbicdRPWufodF@mail.gmail.com>
References: <1275258946-15739-1-git-send-email-atar4qemu@gmail.com>
	<AANLkTil6HsFU422XJ9LMOHEKyS1iTH2aDU7Shzu2sVF8@mail.gmail.com>
	<AANLkTik6S4wLCqFcFmcnFV_fsIa77JPCbicdRPWufodF@mail.gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Tue, 1 Jun 2010 20:09:13 +0000
Message-ID: <AANLkTilsN9rDckkBcw7GyUtQDBuy3EdMDXZjsQyW7DL_@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
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: <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: Artyom Tarasenko <atar4qemu@googlemail.com>
Cc: qemu-devel@nongnu.org

On Tue, Jun 1, 2010 at 7:56 PM, Artyom Tarasenko
<atar4qemu@googlemail.com> wrote:
> 2010/6/1 Blue Swirl <blauwirbel@gmail.com>:
>> On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko
>> <atar4qemu@googlemail.com> 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 chip
>> 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)
> +{
> + =C2=A0 =C2=A0ESPState *s =3D container_of(d, ESPState, busdev.qdev);
> +
> + =C2=A0 =C2=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().

Thanks for testing.