From 04f2f35d575087f82a3a8a1735642333e199b8fe Mon Sep 17 00:00:00 2001 From: Tim Paine <3105306+timkpaine@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:31:04 -0400 Subject: [PATCH] Serialize DMA transfers on the dext's descriptor ring and check the completed count Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com> --- .../integration/test_macos_xdma_hardware.py | 32 +++++++++++++++++++ kernel/macos-xdma/xdma/XDMADriver.cpp | 23 ++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/dau_driver/tests/integration/test_macos_xdma_hardware.py b/dau_driver/tests/integration/test_macos_xdma_hardware.py index dcbe1bd..c04df72 100644 --- a/dau_driver/tests/integration/test_macos_xdma_hardware.py +++ b/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=, 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)) diff --git a/kernel/macos-xdma/xdma/XDMADriver.cpp b/kernel/macos-xdma/xdma/XDMADriver.cpp index 9af707e..fca3d5a 100644 --- a/kernel/macos-xdma/xdma/XDMADriver.cpp +++ b/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);