Start DMA on the peripheral's request, and clock both directions together

Two related faults in the DMA model, both of which corrupted flash reads.

Transfers started when a channel was enabled. On hardware, enabling only arms a
channel; the transfer begins when the peripheral raises its DMA request. The flash
driver's SPI_ReadBuf arms RX, arms TX, then enables SPI and sets TXDMAEN -- so
firing at arm time clocked the bus before the read command had been sent. SPI now
kicks the armed channels from CR1/CR2 when SPE and a DMA request enable are both
set, which covers the read path (SPE last) and the write path alike.

Each channel also ran to completion independently. SPI is duplex: one clocked byte
is simultaneously sent and received, and the driver relies on that, pairing a
memory-to-peripheral channel feeding dummy bytes with a peripheral-to-memory
channel collecting the reply. Running them in sequence meant TX clocked the whole
transfer out before RX looked at the bus, so RX collected nothing. They are now
stepped together, one byte at a time.

Either fault alone made a 4 KB sector read return zeros. PY25Q16_WriteBuffer reads
a sector into SectorCache, patches it, and writes the whole thing back, so a zeroed
read turned into a zeroed sector -- including the per-band VFO frequencies at
0x9000. That is a second, independent cause of typed frequencies reverting to
18 MHz, on top of the missing page wrap fixed in da1ad7e.

Verified: the frequency area stays 0xFF across a boot where it was previously
zeroed every time, and an instrumented build shows the sector read now returning
0xFF rather than 0x00.

test_flash_persist.py now starts from the pristine image rather than
assets/flash.img. A dirty live image left nothing for the boot to write, which
surfaced as "the image is byte-identical" -- a failure that looks like broken
persistence but is really a dirty fixture. Runs twice in a row cleanly now.

keypad_test.py and the 141 unit tests still pass.

qemu/py32f071.c

@@ -623,6 +623,14 @@ struct PY32SpiState {
 
     PY32SpiXferFn xfer;
     void         *xfer_opaque;
+
+    /*
+     * Set by the DMA model so SPI can kick armed channels when the guest asserts
+     * a DMA request. Without this the request is invisible to DMA and the
+     * transfer has to be started at arm time, which is too early.
+     */
+    void (*dma_kick)(void *dma, PY32SpiState *spi);
+    void  *dma;
 };
 
 #define SPI_CR1  0x00
@@ -634,6 +642,10 @@ struct PY32SpiState {
 #define SPI_SR_TXE  (1u << 1)
 #define SPI_SR_BSY  (1u << 7)
 
+#define SPI_CR1_SPE     (1u << 6)   /* SPI enable */
+#define SPI_CR2_RXDMAEN (1u << 0)   /* RX DMA request enable */
+#define SPI_CR2_TXDMAEN (1u << 1)   /* TX DMA request enable */
+
 void py32_spi_set_xfer(PY32SpiState *s, PY32SpiXferFn fn, void *opaque);
 
 void py32_spi_set_xfer(PY32SpiState *s, PY32SpiXferFn fn, void *opaque)
@@ -672,8 +684,31 @@ static void py32_spi_write(void *opaque, hwaddr addr, uint64_t value, unsigned s
     PY32SpiState *s = opaque;
 
     switch (addr) {
-    case SPI_CR1: s->cr1 = value; break;
+    case SPI_CR1:
-    case SPI_CR2: s->cr2 = value; break;
+        s->cr1 = value;
+        /*
+         * SPE can be the last thing enabled. The flash driver's read path arms both
+         * channels, sets RXDMAEN, enables SPI, then sets TXDMAEN -- but the write
+         * path enables SPI last, so both orders have to work.
+         */
+        if ((value & SPI_CR1_SPE) && (s->cr2 & (SPI_CR2_TXDMAEN | SPI_CR2_RXDMAEN))
+            && s->dma_kick) {
+            s->dma_kick(s->dma, s);
+        }
+        break;
+    case SPI_CR2:
+        s->cr2 = value;
+        /*
+         * A DMA request is what actually starts the transfer on hardware. Kick the
+         * armed channels here rather than when they were enabled: at arm time the
+         * read command has not been clocked out yet, so the reply would be read
+         * before the device had anything to say.
+         */
+        if ((value & (SPI_CR2_TXDMAEN | SPI_CR2_RXDMAEN))
+            && (s->cr1 & SPI_CR1_SPE) && s->dma_kick) {
+            s->dma_kick(s->dma, s);
+        }
+        break;
     case SPI_SR:
         /* Flags are mostly hardware-driven; keep TXE asserted. */
         s->sr = (value & ~SPI_SR_TXE) | SPI_SR_TXE;
@@ -833,52 +868,86 @@ static PY32SpiState *py32_dma_spi_for(PY32DmaState *s, uint32_t paddr)
 }
 
 /*
- * Runs a channel to completion. Transmit channels feed bytes to the device;
+ * Run the armed channels for one SPI peripheral.
- * receive channels store what it returns. When both directions are armed the
+ *
- * transmit side has usually been enabled first, and the flash driver arms RX
+ * SPI is inherently duplex: every clocked byte simultaneously sends one byte and
- * before TX, so a receive channel drives the clock itself -- otherwise nothing
+ * receives one. The firmware exploits this, arming a memory-to-peripheral channel
- * would ever be shifted in.
+ * that feeds dummy bytes and a peripheral-to-memory channel that collects the
+ * reply, both over the same transfer.
+ *
+ * So the two channels have to be stepped together, one byte at a time. Running
+ * them one after another -- as this did when each channel started on its own
+ * enable -- means the TX channel clocks the entire transfer out before the RX
+ * channel ever looks at the bus, and RX collects nothing.
  */
-static void py32_dma_run(PY32DmaState *s, int ch)
+static void py32_dma_run_for_spi(PY32DmaState *s, PY32SpiState *spi)
 {
-    PY32DmaChannel *c = &s->ch[ch];
+    AddressSpace *as = &address_space_memory;
-    PY32SpiState *spi = py32_dma_spi_for(s, c->cpar);
+    int tx = -1, rx = -1;
 
-    if (!spi || c->cndtr == 0) {
+    for (int ch = 0; ch < PY32_DMA_CHANNELS; ch++) {
-        /* Nothing attached, or a zero-length transfer: report completion so the
+        PY32DmaChannel *c = &s->ch[ch];
-         * guest does not wait forever. */
+        if (!(c->ccr & DMA_CCR_EN) || c->cndtr == 0) {
-        s->isr |= DMA_FLAG_TCIF(ch) | DMA_FLAG_GIF(ch);
+            continue;
-        c->cndtr = 0;
+        }
-        py32_dma_update_irq(s);
+        if (py32_dma_spi_for(s, c->cpar) != spi) {
+            continue;
+        }
+        if (c->ccr & DMA_CCR_DIR) {
+            tx = ch;
+        } else {
+            rx = ch;
+        }
+    }
+
+    if (tx < 0 && rx < 0) {
         return;
     }
 
-    const bool from_memory = (c->ccr & DMA_CCR_DIR) != 0;
+    /* Length is whichever side is armed; when both are, they match. */
-    const bool minc = (c->ccr & DMA_CCR_MINC) != 0;
+    uint32_t count = tx >= 0 ? s->ch[tx].cndtr : s->ch[rx].cndtr;
-    uint32_t maddr = c->cmar;
+    uint32_t tx_addr = tx >= 0 ? s->ch[tx].cmar : 0;
-    AddressSpace *as = &address_space_memory;
+    uint32_t rx_addr = rx >= 0 ? s->ch[rx].cmar : 0;
+    const bool tx_inc = tx >= 0 && (s->ch[tx].ccr & DMA_CCR_MINC);
+    const bool rx_inc = rx >= 0 && (s->ch[rx].ccr & DMA_CCR_MINC);
 
-    while (c->cndtr > 0) {
+    while (count > 0) {
-        uint8_t byte = 0xff;
+        uint8_t out = 0xff;
 
-        if (from_memory) {
+        if (tx >= 0) {
-            address_space_read(as, maddr, MEMTXATTRS_UNSPECIFIED, &byte, 1);
+            address_space_read(as, tx_addr, MEMTXATTRS_UNSPECIFIED, &out, 1);
-            (void)py32_spi_xfer_byte(spi, byte);
-        } else {
-            byte = py32_spi_xfer_byte(spi, 0xff);
-            address_space_write(as, maddr, MEMTXATTRS_UNSPECIFIED, &byte, 1);
         }
 
-        if (minc) {
+        const uint8_t in = py32_spi_xfer_byte(spi, out);
-            maddr++;
+
+        if (rx >= 0) {
+            address_space_write(as, rx_addr, MEMTXATTRS_UNSPECIFIED, &in, 1);
         }
-        c->cndtr--;
+
+        if (tx_inc) {
+            tx_addr++;
+        }
+        if (rx_inc) {
+            rx_addr++;
+        }
+        count--;
     }
 
-    s->isr |= DMA_FLAG_TCIF(ch) | DMA_FLAG_GIF(ch);
+    for (int ch = 0; ch < PY32_DMA_CHANNELS; ch++) {
+        if (ch == tx || ch == rx) {
+            s->ch[ch].cndtr = 0;
+            s->isr |= DMA_FLAG_TCIF(ch) | DMA_FLAG_GIF(ch);
+        }
+    }
     py32_dma_update_irq(s);
 }
 
+/* Thin adaptor so SPI can call into DMA without knowing its type. */
+static void py32_dma_kick(void *dma, PY32SpiState *spi)
+{
+    py32_dma_run_for_spi((PY32DmaState *)dma, spi);
+}
+
 static uint64_t py32_dma_read(void *opaque, hwaddr addr, unsigned size)
 {
     PY32DmaState *s = opaque;
@@ -929,12 +998,21 @@ static void py32_dma_write(void *opaque, hwaddr addr, uint64_t value, unsigned s
     case DMA_CPAR:  s->ch[ch].cpar = value;  break;
     case DMA_CMAR:  s->ch[ch].cmar = value;  break;
     case DMA_CCR: {
-        const bool was_enabled = (s->ch[ch].ccr & DMA_CCR_EN) != 0;
+      s->ch[ch].ccr = value;
-        s->ch[ch].ccr = value;
+      /*
-        if (!was_enabled && (value & DMA_CCR_EN)) {
+       * Enabling a channel only arms it. On real hardware the transfer starts
-            py32_dma_run(s, ch);
+       * when the peripheral raises its DMA request, which for SPI means
-        }
+       * SPI_CR2's TXDMAEN. Running it here instead broke duplex reads: the
-        break;
+       * firmware's SPI_ReadBuf arms RX then TX and only then enables SPI, so a
+       * transfer that fired at arm time clocked the bus before the read command
+       * had been sent, and the destination buffer came back as zeros.
+       *
+       * That is what wiped the VFO frequency area. PY25Q16_WriteBuffer reads the
+       * whole 4 KB sector into SectorCache, patches it, and writes it back; the
+       * read returned zeros, so the write-back filled the sector with zeros --
+       * including the per-band frequencies at 0x9000.
+       */
+      break;
     }
     default:
         break;
@@ -1652,6 +1730,12 @@ static void py32f071_soc_realize(DeviceState *dev_soc, Error **errp)
      */
     s->dma.spi[0] = &s->spi[0];
     s->dma.spi[1] = &s->spi[1];
+
+    /* And the reverse link, so a DMA request from SPI can start the transfer. */
+    for (int i = 0; i < 2; i++) {
+        s->spi[i].dma = &s->dma;
+        s->spi[i].dma_kick = py32_dma_kick;
+    }
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->dma), errp)) {
         return;
     }

tools/test_flash_persist.py

@@ -149,7 +149,19 @@ def main():
 
     workdir = tempfile.mkdtemp(prefix="uvk5-persist-")
     image = os.path.join(workdir, "flash.img")
-    shutil.copy(SOURCE_IMAGE, image)
+
+    # Start from the pristine image, not assets/flash.img. The live image may have
+    # been written by an earlier session, and then a boot has nothing left to save
+    # -- which shows up as "the image is byte-identical", a confusing failure that
+    # looks like persistence is broken when it is the fixture that is dirty.
+    pristine_gz = os.path.join(os.path.dirname(SOURCE_IMAGE),
+                               "pristine", "flash-pristine.img.gz")
+    if os.path.exists(pristine_gz):
+        import gzip
+        with gzip.open(pristine_gz, "rb") as src, open(image, "wb") as dst:
+            shutil.copyfileobj(src, dst)
+    else:
+        shutil.copy(SOURCE_IMAGE, image)
     failures = []
 
     try: