[Qemu-devel] Re: [PATCH] sparc32 esp fix spurious interrupts in chip reset

[Blue Swirl]

On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko
<atar4q...@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. 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?

From 44d40083652d5135ecb567fea4cff4f109ee4cd0 Mon Sep 17 00:00:00 2001
From: Blue Swirl <blauwirbel@gmail.com>
Date: Tue, 1 Jun 2010 17:37:13 +0000
Subject: [PATCH] esp: lower IRQ on soft reset

42f1ced228c9b616cfa2b69846025271618e4ef5 removed irq lowering
during reset. However, for chip reset command and DMA reset signal,
its actually the correct thing to do.

Lower IRQ on soft reset only.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/esp.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 0a8cf6e..7740879 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -419,7 +419,7 @@ static void handle_ti(ESPState *s)
     }
 }
 
-static void esp_reset(DeviceState *d)
+static void esp_hard_reset(DeviceState *d)
 {
     ESPState *s = container_of(d, ESPState, busdev.qdev);
 
@@ -435,10 +435,19 @@ static void esp_reset(DeviceState *d)
     s->rregs[ESP_CFG1] = 7;
 }
 
+static void esp_soft_reset(DeviceState *d)
+{
+    ESPState *s = container_of(d, ESPState, busdev.qdev);
+
+    qemu_irq_lower(s->irq);
+    esp_hard_reset(d);
+}
+
 static void parent_esp_reset(void *opaque, int irq, int level)
 {
-    if (level)
-        esp_reset(opaque);
+    if (level) {
+        esp_soft_reset(opaque);
+    }
 }
 
 static uint32_t esp_mem_readb(void *opaque, target_phys_addr_t addr)
@@ -528,7 +537,7 @@ static void esp_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
             break;
         case CMD_RESET:
             DPRINTF("Chip reset (%2.2x)\n", val);
-            esp_reset(&s->busdev.qdev);
+            esp_soft_reset(&s->busdev.qdev);
             break;
         case CMD_BUSRESET:
             DPRINTF("Bus reset (%2.2x)\n", val);
@@ -679,7 +688,7 @@ static SysBusDeviceInfo esp_info = {
     .qdev.name  = "esp",
     .qdev.size  = sizeof(ESPState),
     .qdev.vmsd  = &vmstate_esp,
-    .qdev.reset = esp_reset,
+    .qdev.reset = esp_hard_reset,
     .qdev.props = (Property[]) {
         {.name = NULL}
     }
-- 
1.5.6.5