fix(isoch): enforce RX completion ordering and clear stopped events

ASFWDriver/Common/BarrierUtils.hpp

@@ -16,6 +16,19 @@ void FullBarrier();    // full fence for rare cases
 // MMIO barrier for programming device registers
 inline void IoBarrier() { OSSynchronizeIO(); }
 
+// DriverKit 27 IOLib.h:235-239 documents OSSynchronizeIO as DMB ST on
+// Apple Silicon (also verified in the installed DriverKit library). It does
+// not order reads. RX needs completion -> payload reads -> status rearm.
+// Use the full system domain for device DMA, not a C++ inner-shareable fence.
+// This orders accesses; it is not cache maintenance or a DMA completion wait.
+inline void DmaOwnershipBarrier() {
+#if defined(__aarch64__)
+    __builtin_arm_dmb(0xf); // DMB SY: all loads/stores, full system domain.
+#else
+    OSSynchronizeIO(); // DriverKit x86_64: MFENCE.
+#endif
+}
+
 // Convenience wrappers for raw MMIO 32-bit accesses
 inline void Write32(volatile uint32_t* addr, uint32_t value) {
     *addr = value;

ASFWDriver/Isoch/Memory/IIsochDMAMemory.hpp

@@ -20,6 +20,12 @@ class IIsochDMAMemory : public Shared::IDMAMemory {
     // Explicitly allocate a payload buffer region (from payload slab)
     virtual std::optional<Shared::DMARegion> AllocatePayloadBuffer(size_t size) = 0;
 
+    /// Orders completion observation, payload reads, and completion rearming.
+    /// Separate from publication: receive ownership also needs load ordering.
+    virtual void ReceiveOwnershipBarrier() const noexcept {
+        ASFW::Driver::DmaOwnershipBarrier();
+    }
+
     /// Orders payload publication before descriptor publication. Kept virtual
     /// so host tests can record the exact DMA visibility sequence.
     virtual void PublishBarrier() const noexcept {

ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp

@@ -210,6 +210,15 @@ kern_return_t IsochReceiveContext::Stop() {
         return failure;
     }
 
+    if (control != UINT32_MAX) {
+        if (auto access = hardware_->TryBeginAccess()) {
+            // Masked context events are invisible to the interrupt drain
+            // (Linux ohci.c:2094-2107). Once ACTIVE is clear, retire only our
+            // stopped context's pending event before a later restart.
+            access.WriteAndFlush(Driver::Register32::kIsoRecvIntEventClear, contextMask);
+        }
+    }
+
     IOLockLock(lock_);
     Transition(IRPolicy::State::Stopped, 0, "Stop");
     IOLockUnlock(lock_);

ASFWDriver/Isoch/Receive/IsochRxDmaRing.hpp

@@ -54,14 +54,21 @@ class IsochRxDmaRing final {
 
             dma.FetchFromDevice(desc, sizeof(*desc));
 
-            const uint16_t xferStatus = Async::HW::AR_xferStatus(*desc);
-            const uint16_t resCount = Async::HW::AR_resCount(*desc);
-            const bool done = (xferStatus != 0) || (resCount != reqCount);
-
-            if (!done) {
+            const uint32_t status = desc->statusWord;
+            const uint16_t xferStatus = static_cast<uint16_t>(status >> 16);
+            const uint16_t resCount = static_cast<uint16_t>(status);
+            // A residual byte count is progress, not ownership transfer.
+            // Cross-checked with Linux ohci.c:2781-2813: packet-per-buffer
+            // retirement waits for transfer_status before accessing payload.
+            if (xferStatus == 0) {
                 break;
             }
 
+            // Completion observation must precede payload reads. Linux
+            // ohci.c:2781-2813 synchronizes RX payload for the CPU here.
+            // FetchFromDevice's OSSynchronizeIO alone is DMB ST on arm64,
+            // even for cache-inhibited mappings; it does not order loads.
+            dma.ReceiveOwnershipBarrier();
             const uint16_t actualLength = (resCount <= reqCount) ? static_cast<uint16_t>(reqCount - resCount) : 0;
             auto* payloadVA = bufferRing_.GetElementVA(idx);
             if (payloadVA && actualLength > 0) {
@@ -77,6 +84,11 @@ class IsochRxDmaRing final {
             };
             onPacket(packet);
 
+            // Finish payload reads before clearing the completion marker.
+            // Linux queues payload back for the device only after consumption
+            // (amdtp-stream.c:1343-1356; ohci.c:3370-3392). This orders CPU
+            // accesses; the circular program still has no hardware stop fence.
+            dma.ReceiveOwnershipBarrier();
             Async::HW::AR_init_status(*desc, reqCount);
             dma.PublishToDevice(desc, sizeof(*desc));
 

ASFWDriver/Isoch/Transmit/IsochTransmitContext.cpp

@@ -461,6 +461,14 @@ kern_return_t IsochTransmitContext::Stop() noexcept {
             return failure;
         }
 
+        if (control != UINT32_MAX) {
+            if (auto stoppedAccess = hardware_->TryBeginAccess()) {
+                // As for RX: retire only this quiesced context's masked event
+                // (Linux ohci.c:2105-2107), leaving other contexts untouched.
+                stoppedAccess.WriteAndFlush(Register32::kIsoXmitIntEventClear, 1u << contextIndex_);
+            }
+        }
+
         if (controlBlock_) {
             controlBlock_->statusWord.store(IsochTxQueueStatus::kStopped, std::memory_order_release);
         }

tests/audio/IsochReceiveContextTests.cpp

@@ -171,9 +171,11 @@ TEST_F(IsochReceiveContextTest, StopRetainsBindingStateWhenActiveNeverClears) {
     const auto controlSet = static_cast<::ASFW::Driver::Register32>(
         ::DMAContextHelpers::IsoRcvContextControlSet(0));
     hardware_->SetTestRegister(controlSet, ::ASFW::Driver::ContextControl::kActive);
+    hardware_->SetTestRegister(::ASFW::Driver::Register32::kIsoRecvIntEventClear, 0);
 
     EXPECT_EQ(context_->Stop(), kIOReturnTimeout);
     EXPECT_EQ(context_->GetState(), IRPolicy::State::Running);
+    EXPECT_EQ(hardware_->GetTestRegister(::ASFW::Driver::Register32::kIsoRecvIntEventClear), 0U);
 }
 
 TEST_F(IsochReceiveContextTest, PollProcessesPackets) {
@@ -227,3 +229,29 @@ TEST_F(IsochReceiveContextTest, BusyPollRetainsBindingUntilStopCanAcquireOwnersh
     EXPECT_EQ(consumer.quiesceCount, 1U);
     context_->SetReceiveConsumer(nullptr);
 }
+
+TEST_F(IsochReceiveContextTest, StopAcknowledgesOnlyItsQuiescedContext) {
+    ASSERT_EQ(context_->Configure(0, 3), kIOReturnSuccess);
+    ASSERT_EQ(context_->Start(), kIOReturnSuccess);
+    const auto controlSet = static_cast<::ASFW::Driver::Register32>(
+        ::DMAContextHelpers::IsoRcvContextControlSet(3));
+    hardware_->SetTestRegister(controlSet, 0);
+    hardware_->SetTestRegister(::ASFW::Driver::Register32::kIsoRecvIntEventClear, 0);
+    hardware_->SetTestRegister(::ASFW::Driver::Register32::kIsoRecvIntEventSet, 0xf);
+
+    EXPECT_EQ(context_->Stop(), kIOReturnSuccess);
+    EXPECT_EQ(hardware_->GetTestRegister(::ASFW::Driver::Register32::kIsoRecvIntEventClear), 8U);
+    EXPECT_EQ(context_->GetState(), IRPolicy::State::Stopped);
+}
+
+TEST_F(IsochReceiveContextTest, StopDoesNotAcknowledgeWhenControllerReadsAllOnes) {
+    ASSERT_EQ(context_->Configure(0, 0), kIOReturnSuccess);
+    ASSERT_EQ(context_->Start(), kIOReturnSuccess);
+    const auto controlSet = static_cast<::ASFW::Driver::Register32>(
+        ::DMAContextHelpers::IsoRcvContextControlSet(0));
+    hardware_->SetTestRegister(controlSet, UINT32_MAX);
+    hardware_->SetTestRegister(::ASFW::Driver::Register32::kIsoRecvIntEventClear, 0);
+
+    EXPECT_EQ(context_->Stop(), kIOReturnSuccess);
+    EXPECT_EQ(hardware_->GetTestRegister(::ASFW::Driver::Register32::kIsoRecvIntEventClear), 0U);
+}

tests/audio/IsochRxDmaRingTests.cpp

@@ -1,4 +1,5 @@
 #include <gtest/gtest.h>
+#include <functional>
 
 #include "Isoch/Receive/IsochRxDmaRing.hpp"
 #include "Isoch/Memory/IsochDMAMemoryManager.hpp"
@@ -27,8 +28,128 @@ std::shared_ptr<IIsochDMAMemory> MakeTestIsochMemory(::ASFW::Driver::HardwareInt
     return concreteMgr;
 }
 
+// Keep the production allocator/IOVA layout; intercept only visibility events.
+// This models ordering obligations, not ARM caches or a physical DMA engine.
+class ObservedReceiveMemory final : public IIsochDMAMemory {
+public:
+    explicit ObservedReceiveMemory(IIsochDMAMemory& backing) : backing_(backing) {}
+    std::function<void()> onOwnership;
+    std::function<void(const std::byte*, size_t)> onFetch;
+    std::function<void(const std::byte*, size_t)> onPublish;
+
+    std::optional<ASFW::Shared::DMARegion> AllocateDescriptor(size_t size) override {
+        return backing_.AllocateDescriptor(size);
+    }
+    std::optional<ASFW::Shared::DMARegion> AllocatePayloadBuffer(size_t size) override {
+        return backing_.AllocatePayloadBuffer(size);
+    }
+    std::optional<ASFW::Shared::DMARegion> AllocateRegion(size_t size, size_t alignment) override {
+        return backing_.AllocateRegion(size, alignment);
+    }
+    uint64_t VirtToIOVA(const std::byte* p) const noexcept override {
+        return backing_.VirtToIOVA(p);
+    }
+    std::byte* IOVAToVirt(uint64_t address) const noexcept override {
+        return backing_.IOVAToVirt(address);
+    }
+    void FetchFromDevice(const std::byte* p, size_t size) const noexcept override {
+        if (onFetch) onFetch(p, size);
+        backing_.FetchFromDevice(p, size);
+    }
+    void PublishToDevice(const std::byte* p, size_t size) const noexcept override {
+        if (onPublish) onPublish(p, size);
+        backing_.PublishToDevice(p, size);
+    }
+    void ReceiveOwnershipBarrier() const noexcept override {
+        if (onOwnership) onOwnership();
+        backing_.ReceiveOwnershipBarrier();
+    }
+    size_t TotalSize() const noexcept override { return backing_.TotalSize(); }
+    size_t AvailableSize() const noexcept override { return backing_.AvailableSize(); }
+private:
+    IIsochDMAMemory& backing_;
+};
+
 } // namespace
 
+TEST(IsochRxDmaRingTests, OrdersCompletedPayloadReadsAndRearmAcrossWraps) {
+    ::ASFW::Driver::HardwareInterface hw;
+    auto backing = MakeTestIsochMemory(hw, 8, 64);
+    ASSERT_TRUE(backing);
+    ObservedReceiveMemory mem(*backing);
+    IsochRxDmaRing ring;
+    ASSERT_EQ(ring.SetupRings(mem, 8, 64), kIOReturnSuccess);
+
+    // Follow the descriptor program built by SetupRings, including its wrap.
+    // Fixed-size INPUT_LAST slots have no allocator padding to skip.
+    uint32_t command = ring.InitialCommandPtrWord();
+    for (uint32_t packet = 0; packet < 40; ++packet) {
+        const uint32_t index = packet % 8;
+        auto* desc = backing->IOVAToPtr<ASFW::Async::HW::OHCIDescriptor>(command & ~0xfu);
+        ASSERT_EQ(desc, ring.DescriptorAt(index));
+        ASSERT_EQ(command & 0xfu, 1u);
+        auto* payload = ring.PayloadVA(index);
+        // Delay modeled payload visibility until the ownership acquire. The
+        // production FetchFromDevice currently supplies only a store barrier.
+        payload[0] = 0xee;
+        desc->statusWord = (0x8451u << 16) | 48u;
+        uint32_t barriers = 0;
+        bool consumed = false;
+        bool published = false;
+        mem.onOwnership = [&] {
+            EXPECT_EQ(desc->statusWord, (0x8451u << 16) | 48u);
+            if (barriers == 0) {
+                EXPECT_FALSE(consumed);
+                payload[0] = static_cast<uint8_t>(packet);
+            } else {
+                EXPECT_TRUE(consumed);
+            }
+            ++barriers;
+        };
+        mem.onFetch = [&](const std::byte* p, size_t) {
+            if (p == reinterpret_cast<const std::byte*>(payload)) EXPECT_EQ(barriers, 1u);
+        };
+        mem.onPublish = [&](const std::byte* p, size_t) {
+            ASSERT_EQ(p, reinterpret_cast<const std::byte*>(desc));
+            EXPECT_TRUE(consumed);
+            EXPECT_EQ(barriers, 2u);
+            EXPECT_EQ(desc->statusWord, 64u);
+            published = true;
+        };
+        EXPECT_EQ(ring.DrainCompleted(mem, [&](const auto& completion) {
+            EXPECT_EQ(completion.descriptorIndex, index);
+            EXPECT_EQ(completion.payload[0], static_cast<uint8_t>(packet));
+            EXPECT_EQ(desc->statusWord, (0x8451u << 16) | 48u);
+            EXPECT_FALSE(published);
+            consumed = true;
+        }), 1u);
+        EXPECT_EQ(barriers, 2u);
+        EXPECT_TRUE(published);
+        command = desc->branchWord;
+        mem.onOwnership = {};
+        mem.onFetch = {};
+        mem.onPublish = {};
+        EXPECT_EQ(ring.DrainCompleted(mem, [](const auto&) { ADD_FAILURE(); }), 0u);
+    }
+}
+
+TEST(IsochRxDmaRingTests, DoesNotAcquireOrReuseWhileOnlyResidualHasChanged) {
+    ::ASFW::Driver::HardwareInterface hw;
+    auto backing = MakeTestIsochMemory(hw, 8, 64);
+    ObservedReceiveMemory mem(*backing);
+    IsochRxDmaRing ring;
+    ASSERT_EQ(ring.SetupRings(mem, 8, 64), kIOReturnSuccess);
+    ring.DescriptorAt(0)->statusWord = 48;
+    mem.onOwnership = [] { ADD_FAILURE() << "incomplete packet has no ownership transfer"; };
+    mem.onPublish = [](const auto*, auto) { ADD_FAILURE() << "must not rearm in-flight DMA"; };
+    mem.onFetch = [&](const std::byte* p, size_t size) {
+        EXPECT_EQ(p, reinterpret_cast<const std::byte*>(ring.DescriptorAt(0)));
+        EXPECT_EQ(size, sizeof(ASFW::Async::HW::OHCIDescriptor));
+    };
+    EXPECT_EQ(ring.DrainCompleted(mem, [](const auto&) { ADD_FAILURE(); }), 0u);
+    EXPECT_EQ(ring.DescriptorAt(0)->statusWord, 48u);
+}
+
 TEST(IsochRxDmaRingTests, InitialCommandPtrWord_SetsZBitAndPointsToDesc0) {
     ::ASFW::Driver::HardwareInterface hw;
     auto mem = MakeTestIsochMemory(hw, 8, 64);
@@ -64,7 +185,7 @@ TEST(IsochRxDmaRingTests, DrainCompleted_ProcessesOneDescriptorAndRearms) {
 
     constexpr uint16_t reqCount = 64;
     constexpr uint16_t actualLength = 16;
-    d0->statusWord = (0x0000u << 16) | static_cast<uint16_t>(reqCount - actualLength);
+    d0->statusWord = (0x8451u << 16) | static_cast<uint16_t>(reqCount - actualLength);
 
     uint32_t calls = 0;
     const uint32_t processed = ring.DrainCompleted(*mem, [&calls, actualLength](const IsochRxDmaRing::CompletedPacket& pkt) {
@@ -86,3 +207,57 @@ TEST(IsochRxDmaRingTests, DrainCompleted_ProcessesOneDescriptorAndRearms) {
     EXPECT_EQ(ASFW::Async::HW::AR_xferStatus(*d0), 0u);
     EXPECT_EQ(ASFW::Async::HW::AR_resCount(*d0), reqCount);
 }
+
+TEST(IsochRxDmaRingTests, ResidualProgressDoesNotRetireAnIncompletePacket) {
+    ::ASFW::Driver::HardwareInterface hw;
+    auto mem = MakeTestIsochMemory(hw, 8, 64);
+    IsochRxDmaRing ring;
+    ASSERT_EQ(ring.SetupRings(*mem, 8, 64), kIOReturnSuccess);
+    auto* first = ring.DescriptorAt(0);
+    first->statusWord = 48;
+    ring.DescriptorAt(1)->statusWord = (0x8451u << 16) | 48u;
+    uint32_t calls = 0;
+    EXPECT_EQ(ring.DrainCompleted(*mem, [&](const auto&) { ++calls; }), 0u);
+    EXPECT_EQ(calls, 0u);
+    EXPECT_EQ(first->statusWord, 48u);
+    EXPECT_EQ(ring.DrainCompleted(*mem, [&](const auto&) { ++calls; }), 0u);
+    EXPECT_EQ(first->statusWord, 48u);
+    first->statusWord = (0x8451u << 16) | 48u;
+    EXPECT_EQ(ring.DrainCompleted(*mem, [&](const auto& packet) {
+        EXPECT_EQ(packet.descriptorIndex, calls++);
+        EXPECT_EQ(packet.actualLength, 16u);
+        EXPECT_EQ(packet.xferStatus, 0x8451u);
+    }), 2u);
+    EXPECT_EQ(calls, 2u);
+}
+
+TEST(IsochRxDmaRingTests, FullRingWrapEmptyTailAndHotAppendPreserveOrder) {
+    ::ASFW::Driver::HardwareInterface hw;
+    auto mem = MakeTestIsochMemory(hw, 8, 64);
+    IsochRxDmaRing ring;
+    ASSERT_EQ(ring.SetupRings(*mem, 8, 64), kIOReturnSuccess);
+    // This production receive allocator has fixed-size slots and no wrap
+    // padding. Exercise its branch from the tail back to descriptor zero.
+    uint32_t expected = 0;
+    auto consume = [&](const auto& packet) {
+        EXPECT_EQ(packet.descriptorIndex, expected++ % 8);
+        EXPECT_EQ(packet.actualLength, 16u);
+    };
+    for (unsigned lap = 0; lap < 4; ++lap) {
+        for (unsigned i = 0; i < 8; ++i)
+            ring.DescriptorAt(i)->statusWord = (0x8451u << 16) | 48u;
+        EXPECT_EQ(ring.DrainCompleted(*mem, consume), 8u);
+        EXPECT_EQ(ring.DrainCompleted(*mem, consume), 0u);
+    }
+    for (unsigned i = 0; i < 7; ++i)
+        ring.DescriptorAt(i)->statusWord = (0x8451u << 16) | 48u;
+    EXPECT_EQ(ring.DrainCompleted(*mem, consume), 7u);
+    EXPECT_EQ(ring.DrainCompleted(*mem, consume), 0u); // empty at tail
+    ring.DescriptorAt(7)->statusWord = (0x8451u << 16) | 48u;
+    EXPECT_EQ(ring.DrainCompleted(*mem, [&](const auto& packet) {
+        consume(packet);
+        if (packet.descriptorIndex == 7)
+            ring.DescriptorAt(0)->statusWord = (0x8451u << 16) | 48u;
+    }), 2u); // completion arrives across the wrap while draining
+    EXPECT_EQ(expected, 41u);
+}

tests/audio/IsochServiceTxPreparationTests.cpp

@@ -248,13 +248,19 @@ TEST(IsochServiceTxPreparation, FaultedTransmitIsQuiescedByStopBeforeReuse) {
                                                   controlDescriptor,
                                                   AudioTimingGeometry::kTxPacketsPerGroup),
               kIOReturnBusy);
+    // A pending sibling event must survive our stop. Neither event may be
+    // acknowledged while this context still owns its DMA program.
+    hardware.SetTestRegister(Register32::kIsoXmitIntEventSet, 0x5u);
+    hardware.SetTestRegister(Register32::kIsoXmitIntEventClear, 0u);
     EXPECT_EQ(service.StopTransmit(), kIOReturnTimeout);
     EXPECT_EQ(context->GetState(), ASFW::Isoch::ITState::Faulted);
+    EXPECT_EQ(hardware.GetTestRegister(Register32::kIsoXmitIntEventClear), 0u);
 
     hardware.SetTestRegister(controlSet, 0);
     EXPECT_EQ(service.StopTransmit(), kIOReturnSuccess);
     EXPECT_EQ(context->GetState(), ASFW::Isoch::ITState::Stopped);
     EXPECT_FALSE(context->NeedsQuiesce());
+    EXPECT_EQ(hardware.GetTestRegister(Register32::kIsoXmitIntEventClear), 1u);
 }
 
 // Start clears every ContextControl bit: cycleMatchEnable (bit 31) has an