Serialize DMA transfers on the dext's descriptor ring and check the completed count

dau_driver/tests/integration/test_macos_xdma_hardware.py

@@ -22,3 +22,35 @@ def test_macos_xdma_hardware_discovers_dau_identity() -> None:
     assert device.backend == "macos-xdma"
     assert device.read_register(RegisterOffset.MAGIC) == DAU_MAGIC_WORD
     assert device.read_register_block_snapshot().platform_id
+
+
+def _scratch_addresses() -> tuple[int, int]:
+    """Two device addresses this test may overwrite. Which memory is free to
+    clobber is board geometry, so it comes from the bench, not from here."""
+    raw = os.environ.get("DAU_XDMA_SCRATCH_ADDRESSES", "")
+    addresses = tuple(int(part, 0) for part in raw.split(",") if part.strip())
+    if len(addresses) != 2:
+        pytest.skip("set DAU_XDMA_SCRATCH_ADDRESSES=<a>,<b> to two device addresses the test may overwrite")
+    return addresses[0], addresses[1]
+
+
+def test_concurrent_transfers_on_both_engines_round_trip() -> None:
+    """The H2C and C2H engines share the dext's one descriptor ring. Two
+    threads each write a pattern and read it back, so their transfers
+    interleave across both engines; any transfer that ran on a chain the
+    other thread rewrote returns the wrong bytes or fails."""
+    from concurrent.futures import ThreadPoolExecutor
+
+    device = discover_devices()[0]
+    first, second = _scratch_addresses()
+    length = 1 << 20
+
+    def round_trips(address: int, seed: int) -> int:
+        for iteration in range(64):
+            pattern = bytes((seed + iteration + index) & 0xFF for index in range(length))
+            device.write_dma(device_address=address, data=pattern)
+            assert device.read_dma(device_address=address, byte_count=length) == pattern, f"iteration {iteration} at {address:#x}"
+        return address
+
+    with ThreadPoolExecutor(max_workers=2) as pool:
+        assert sorted(pool.map(round_trips, (first, second), (0x11, 0x77))) == sorted((first, second))

kernel/macos-xdma/xdma/XDMADriver.cpp

@@ -51,6 +51,7 @@ struct XDMADriver_IVars {
     IODMACommand *ringDMACommand;
     uint64_t ringDeviceAddress;   // ring IOVA as seen by the device
     uint64_t ringCPUAddress;      // ring address in the dext's address space
+    IOLock *ringLock;             // one transfer owns the ring, H2C or C2H, at a time
     uint8_t configBarIndex;       // memoryIndex of the XDMA config BAR
     bool configBarValid;
     uint64_t barSize[6];          // mapped length per memoryIndex; 0 = not a memory BAR
@@ -66,6 +67,11 @@ bool XDMADriver::init()
     if (ivars == nullptr) {
         return false;
     }
+    ivars->ringLock = IOLockAlloc();
+    if (ivars->ringLock == nullptr) {
+        IOSafeDeleteNULL(ivars, XDMADriver_IVars, 1);
+        return false;
+    }
 
     os_log(OS_LOG_DEFAULT, "XDMADriver::init() complete");
     return true;
@@ -240,6 +246,10 @@ kern_return_t IMPL(XDMADriver, Stop)
 void XDMADriver::free()
 {
     os_log(OS_LOG_DEFAULT, "XDMADriver::free()");
+    if (ivars != nullptr && ivars->ringLock != nullptr) {
+        IOLockFree(ivars->ringLock);
+        ivars->ringLock = nullptr;
+    }
     IOSafeDeleteNULL(ivars, XDMADriver_IVars, 1);
     super::free();
 }
@@ -367,6 +377,12 @@ kern_return_t XDMADriver::DMATransfer(bool hostToCard, uint64_t device_offset, u
         return kIOReturnDMAError;
     }
 
+    // The H2C and C2H engines share one descriptor ring, so a transfer holds
+    // the ring from writing its chain until the engine has finished reading
+    // it. Without this, a transfer on the other engine could rewrite the
+    // chain while this one is still in flight.
+    IOLockLock(ivars->ringLock);
+
     // Build the descriptor chain
     XDMADescriptor *ring = (XDMADescriptor *)ivars->ringCPUAddress;
     uint64_t dev = device_offset;
@@ -415,7 +431,12 @@ kern_return_t XDMADriver::DMATransfer(bool hostToCard, uint64_t device_offset, u
 
     uint32_t completed = 0;
     ivars->pciDevice->MemoryRead32(bar, chBase + XDMA_ENGINE_COMPLETED_DESC, &completed);
-    bool ok = !(status & XDMA_STAT_BUSY) && !(status & XDMA_STAT_ERROR_MASK);
+    IOLockUnlock(ivars->ringLock);
+
+    // The status bits say the engine stopped without an error; the completed
+    // count says it got through every descriptor of this chain rather than
+    // stopping early on one.
+    bool ok = !(status & XDMA_STAT_BUSY) && !(status & XDMA_STAT_ERROR_MASK) && completed == segCount;
     if (!ok) {
         os_log(OS_LOG_DEFAULT, "XDMADriver: DMA %s failed status=0x%x completed=%u/%u len=%llu",
                hostToCard ? "H2C" : "C2H", status, completed, segCount, length);