[RFC PATCH 2/2] hw/ide/ahci: Delay a bit before completing reset

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Give few milliseconds to (emulated) hardware to complete
its reset sequence.

The intent is to have this model better match hardware,
reducing firmware bugs "works in QEMU but not in real
world" such https://github.com/FlyGoat/csmwrap/issues/14.

Reported-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/ide/ahci.h   |  1 +
 hw/ide/ahci.c           | 39 +++++++++++++++++++++++++++++++++++++--
 tests/qtest/ahci-test.c |  4 ++++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/include/hw/ide/ahci.h b/include/hw/ide/ahci.h
index cd07b87811b..82617835195 100644
--- a/include/hw/ide/ahci.h
+++ b/include/hw/ide/ahci.h
@@ -46,6 +46,7 @@ typedef struct AHCIState {
     uint32_t ports;
     qemu_irq irq;
     AddressSpace *as;
+    QEMUTimer *reset_timer;
 } AHCIState;
 
 
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 7e586c7a0a4..cf1c5d3f3db 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -28,6 +28,7 @@
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "qemu/main-loop.h"
+#include "qemu/timer.h"
 #include "system/block-backend.h"
 #include "system/dma.h"
 #include "ahci-internal.h"
@@ -47,6 +48,7 @@ static bool ahci_map_fis_address(AHCIDevice *ad);
 static void ahci_unmap_clb_address(AHCIDevice *ad);
 static void ahci_unmap_fis_address(AHCIDevice *ad);
 static void ahci_reset_delayed(AHCIState *s, bool immediate);
+static void ahci_reset_complete(void *opaque);
 
 static const char *AHCIHostReg_lookup[AHCI_HOST_REG__COUNT] = {
     [AHCI_HOST_REG_CAP]        = "CAP",
@@ -459,7 +461,7 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
             break;
         case AHCI_HOST_REG_CTL: /* R/W */
             if (val & HOST_CTL_RESET) {
-                ahci_reset(s);
+                ahci_reset_delayed(s, true);
             } else {
                 s->control_regs.ghc = (val & 0x3) | HOST_CTL_AHCI_EN;
                 ahci_check_irq(s);
@@ -1591,6 +1593,7 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as)
     s->as = as;
     assert(s->ports > 0);
     s->dev = g_new0(AHCIDevice, s->ports);
+    s->reset_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, ahci_reset_complete, s);
     ahci_reg_init(s);
     irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
     for (i = 0; i < s->ports; i++) {
@@ -1622,8 +1625,12 @@ void ahci_uninit(AHCIState *s)
     }
 
     g_free(s->dev);
+
+    timer_free(s->reset_timer);
 }
 
+#define AHCI_RESET_DELAY_MS     69  /* Arbitrary value less than 500 ms */
+
 static void ahci_reset_complete(void *opaque)
 {
     AHCIState *s = opaque;
@@ -1667,7 +1674,11 @@ static void ahci_reset_delayed(AHCIState *s, bool immediate)
     }
 
     if (immediate) {
+        timer_del(s->reset_timer);
         ahci_reset_complete(s);
+    } else if (!timer_pending(s->reset_timer)) {
+        timer_mod(s->reset_timer,
+                  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + AHCI_RESET_DELAY_MS);
     }
 }
 
@@ -1676,6 +1687,24 @@ void ahci_reset(AHCIState *s)
     ahci_reset_delayed(s, false);
 }
 
+static bool ahci_timer_needed(void *opaque)
+{
+    AHCIState *s = opaque;
+
+    return timer_pending(s->reset_timer);
+}
+
+static const VMStateDescription vmstate_ahci_timer = {
+    .name = "ahci/reset_timer",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = ahci_timer_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_TIMER_PTR(reset_timer, AHCIState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_ncq_tfs = {
     .name = "ncq state",
     .version_id = 1,
@@ -1734,7 +1763,9 @@ static int ahci_state_post_load(void *opaque, int version_id)
         ad = &s->dev[i];
         pr = &ad->port_regs;
 
-        if (!(pr->cmd & PORT_CMD_START) && (pr->cmd & PORT_CMD_LIST_ON)) {
+        if (timer_pending(s->reset_timer)) {
+            /* Reset in progress */
+        } else if (!(pr->cmd & PORT_CMD_START) && (pr->cmd & PORT_CMD_LIST_ON)) {
             error_report("AHCI: DMA engine should be off, but status bit "
                          "indicates it is still running.");
             return -1;
@@ -1823,6 +1854,10 @@ const VMStateDescription vmstate_ahci = {
         VMSTATE_UINT32_EQUAL(ports, AHCIState, NULL),
         VMSTATE_END_OF_LIST()
     },
+    .subsections = (const VMStateDescription * const []) {
+        &vmstate_ahci_timer,
+        NULL
+    }
 };
 
 void ahci_ide_create_devs(AHCIState *ahci, DriveInfo **hd)
diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index e8aabfc13f5..4389aaa7f70 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -40,6 +40,8 @@
 #define TEST_IMAGE_SIZE_MB_LARGE (200 * 1024)
 #define TEST_IMAGE_SIZE_MB_SMALL 64
 
+#define AHCI_RESET_TIMEOUT_NS   (500 * 1000 * 1000)
+
 /*** Globals ***/
 static char *tmp_path;
 static char *debug_path;
@@ -483,6 +485,8 @@ static void ahci_test_hba_spec(AHCIQState *ahci)
 
     g_assert(ahci != NULL);
 
+    qtest_clock_step(ahci->parent->qts, AHCI_RESET_TIMEOUT_NS);
+
     /*
      * Note that the AHCI spec does expect the BIOS to set up a few things:
      * CAP.SSS    - Support for staggered spin-up            (t/f)
-- 
2.47.1