diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 9e6ae591c..6c67bf1d7 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -26,19 +26,29 @@ jobs: # ASFW.xcodeproj is generated from project.yml (XcodeGen) and committed. # Fail if they drifted apart — someone edited project.yml or added source # files without regenerating, or hand-edited the pbxproj. If this fails - # after an xcodegen version bump changed the output format, regenerate - # locally with the new version and commit. + # after the pinned xcodegen version changes, regenerate locally with the + # same version and commit. - name: Verify ASFW.xcodeproj matches project.yml (XcodeGen) run: | - brew install xcodegen - xcodegen --version - xcodegen generate --quiet + # Single source of truth, shared with build.sh, so a contributor's + # locally installed xcodegen can never disagree with this check. + . ./.xcodegen-version + XCODEGEN_ARCHIVE="$RUNNER_TEMP/xcodegen.zip" + XCODEGEN_ROOT="$RUNNER_TEMP/xcodegen-release" + curl --fail --location --retry 3 \ + --output "$XCODEGEN_ARCHIVE" \ + "https://github.com/yonaskolb/XcodeGen/releases/download/${XCODEGEN_VERSION}/xcodegen.zip" + echo "${XCODEGEN_SHA256} ${XCODEGEN_ARCHIVE}" | shasum -a 256 -c - + ditto -x -k "$XCODEGEN_ARCHIVE" "$XCODEGEN_ROOT" + XCODEGEN_BIN="$XCODEGEN_ROOT/xcodegen/bin/xcodegen" + "$XCODEGEN_BIN" --version + "$XCODEGEN_BIN" generate --quiet # Diff only the pbxproj: scheme files are cosmetically rewritten by # any open Xcode (version attr, BuildableName flavor, empty blocks), # so they flip-flop between xcodegen and Xcode styles; their semantic # content comes from project.yml either way. if ! git diff --exit-code --stat -- ASFW.xcodeproj/project.pbxproj project.yml; then - echo "::error::ASFW.xcodeproj is out of sync with project.yml. Run 'xcodegen generate' locally and commit the regenerated project (see README → Building)." + echo "::error::ASFW.xcodeproj is out of sync with project.yml. Regenerate it with XcodeGen ${XCODEGEN_VERSION} and commit the result (see README → Building)." exit 1 fi diff --git a/.xcodegen-version b/.xcodegen-version new file mode 100644 index 000000000..dbafa5344 --- /dev/null +++ b/.xcodegen-version @@ -0,0 +1,18 @@ +# XcodeGen release pinned for generating ASFW.xcodeproj from project.yml. +# +# Both CI (.github/workflows/build-and-test.yml) and build.sh read this file, so +# the version that regenerates the committed project is the same everywhere. +# Homebrew's xcodegen floats and its output is NOT byte-identical across +# releases (2.45.4 -> 2.46.0 reorders the pbxproj `targets` list), which is why +# the pin exists and why build.sh refuses to regenerate with a different one. +# +# Upgrading — do all three steps together, or CI's drift check will fail: +# 1. bump XCODEGEN_VERSION below +# 2. curl the release zip, put its `shasum -a 256` in XCODEGEN_SHA256 +# curl -fL -o /tmp/xcodegen.zip \ +# https://github.com/yonaskolb/XcodeGen/releases/download//xcodegen.zip +# shasum -a 256 /tmp/xcodegen.zip +# 3. regenerate ASFW.xcodeproj with that exact version and commit it alongside +# +XCODEGEN_VERSION=2.46.0 +XCODEGEN_SHA256=4d9e34b62172d645eed6457cac13fc222569974098ef4ee9c3368bedf0196806 diff --git a/ASFW.xcodeproj/project.pbxproj b/ASFW.xcodeproj/project.pbxproj index cbbb809ad..44ec75d46 100644 --- a/ASFW.xcodeproj/project.pbxproj +++ b/ASFW.xcodeproj/project.pbxproj @@ -3000,8 +3000,8 @@ projectDirPath = ""; projectRoot = ""; targets = ( - EE5A446B3669D0171F9606EE /* ASFW */, 0EB9A8DA75D08971084A440A /* ASFWDriver */, + EE5A446B3669D0171F9606EE /* ASFW */, 348672D607701677ABCB567C /* ASFWTests */, ); }; diff --git a/ASFW.xcodeproj/xcshareddata/xcschemes/ASFWDriver.xcscheme b/ASFW.xcodeproj/xcshareddata/xcschemes/ASFWDriver.xcscheme index 09c8b9bed..0d79cf893 100644 --- a/ASFW.xcodeproj/xcshareddata/xcschemes/ASFWDriver.xcscheme +++ b/ASFW.xcodeproj/xcshareddata/xcschemes/ASFWDriver.xcscheme @@ -1,10 +1,11 @@ + version = "1.7"> + buildImplicitDependencies = "YES" + runPostActionsOnFailure = "NO"> @@ -26,18 +27,21 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" - shouldUseLaunchSchemeArgsEnv = "YES"> + shouldUseLaunchSchemeArgsEnv = "YES" + onlyGenerateCoverageForSpecifiedTargets = "NO"> + + + + + + diff --git a/ASFWDriver/Async/AsyncSubsystemBusReset.cpp b/ASFWDriver/Async/AsyncSubsystemBusReset.cpp index a60248724..3ada2e5a5 100644 --- a/ASFWDriver/Async/AsyncSubsystemBusReset.cpp +++ b/ASFWDriver/Async/AsyncSubsystemBusReset.cpp @@ -1,12 +1,16 @@ // SPDX-License-Identifier: Apache-2.0 #include "AsyncSubsystem.hpp" +#include "Contexts/ATRequestContext.hpp" +#include "Contexts/ATResponseContext.hpp" #include "Tx/Submitter.hpp" #include "../Logging/Logging.hpp" #include "Track/LabelAllocator.hpp" #include "Track/PayloadRegistry.hpp" +#include + #include namespace ASFW::Async { @@ -167,7 +171,58 @@ void AsyncSubsystem::FlushATContexts() { if (!txnMgr_) { return; } - (void)DrainTxCompletions(nullptr); + + // Callers reach here only after StopATContextsOnly(). Once ACTIVE is clear, + // hardware can no longer write a final descriptor status. OHCI 1.2 draft + // §7.2.3.3 (p. 7-14) permits optional controller behavior that leaves + // outstanding AT descriptors without a final status after a bus reset. + // Complete those descriptors as evt_flushed rather than waiting forever. + // Linux uses the same stop-then-flush ordering and maps zero/no-status + // entries to RCODE_GENERATION (context_stop() then at_context_flush(), + // ohci.c:2000-2010); Apple uses stopDMA() then resetDMA() in + // handleBusResetInt(). + // + // Confirm ACTIVE is clear per context before flushing. Both references stop + // both contexts unconditionally and then flush unconditionally, but neither + // can prove the stop succeeded: OHCI §7.2.3 lets a context keep the ACTIVE + // bit set past the stop timeout. Flushing such a context would report packets + // that are merely in flight and zero descriptor words the controller is still + // traversing, so skip it and leave the ring for the next scan instead. + auto* atRequest = ResolveAtRequestContext(); + auto* atResponse = ResolveAtResponseContext(); + + const auto flushable = [](auto* context, const char* name) { + if (!context) { + return false; + } + if (context->IsActive()) { + ASFW_LOG_ERROR(Async, + "FlushATContexts: %{public}s still ACTIVE after stop; " + "skipping flush to avoid rewriting live descriptors", + name); + return false; + } + return true; + }; + + std::optional requestFlush; + if (flushable(atRequest, "AT request")) { + requestFlush.emplace(*atRequest); + } + std::optional responseFlush; + if (flushable(atResponse, "AT response")) { + responseFlush.emplace(*atResponse); + } + if (!requestFlush && !responseFlush) { + return; + } + + const uint32_t drained = DrainTxCompletions("bus-reset-flush"); + if (drained > 0) { + ASFW_LOG(Async, + "FlushATContexts: drained %u AT completion(s) during bus-reset flush", + drained); + } } void AsyncSubsystem::ConfirmBusGeneration(uint8_t confirmedGeneration) { diff --git a/ASFWDriver/Async/Contexts/ATContextBase.hpp b/ASFWDriver/Async/Contexts/ATContextBase.hpp index f394d9987..b6dcf0d13 100644 --- a/ASFWDriver/Async/Contexts/ATContextBase.hpp +++ b/ASFWDriver/Async/Contexts/ATContextBase.hpp @@ -230,9 +230,9 @@ class ATContextBase : public ContextBase { /** * \brief Scan for completed descriptors and extract completion status. * - * Walks the descriptor ring from head index, checking xferStatus field - * for hardware completion. Extracts event code, timestamp, and tLabel - * on first completed descriptor found. + * Walks the descriptor ring from head index, checking the packet chain's + * OUTPUT_LAST xferStatus for hardware completion. Extracts event code, + * timestamp, and tLabel on the first completed packet found. * * \return TxCompletion if descriptor completed, std::nullopt if none ready * @@ -245,19 +245,26 @@ class ATContextBase : public ContextBase { * 2. Load head index (atomic acquire) * 3. If head == tail, ring is empty → return nullopt * 4. Read descriptor at head index - * 5. If xferStatus == 0, descriptor not yet completed → return nullopt - * 6. Extract event code from xferStatus[4:0] - * 7. Extract timestamp from timeStamp field - * 8. If OUTPUT_LAST_Immediate, extract tLabel from packet header - * 9. Advance head index: (head + N) % capacity, where N = descriptor block count - * 10. Unlock context, return TxCompletion + * 5. For OUTPUT_MORE, check the following OUTPUT_LAST completion status + * 6. If the packet's terminal xferStatus == 0, return nullopt + * 7. Extract event code from xferStatus[4:0] + * 8. Extract timestamp from timeStamp field + * 9. Extract tLabel from the immediate packet header + * 10. Advance head beyond the completed packet chain + * 11. Unlock context, return TxCompletion * * **Apple Pattern** - * ChannelBundle::ScanNextATReqCompletion(): - * - Checks xferStatus != 0 for completion - * - Extracts ack code and event code from status word - * - Extracts tLabel from packet header for response matching - * - Advances completion cursor + * AppleFWOHCI_AsyncTransmit::checkForCompletedElements(): + * - Checks the terminal descriptor status of each pending ATxElement + * - Leaves the element queued while the terminal status is zero + * - Dispatches completion only after the terminal status becomes non-zero + * + * Apple walks a linked list of ATxElements, so a zero-status element only + * clears its "reap the predecessor" flag and iteration continues past it + * (symbol offsets 0xf0e8-0xf10a, 0xf188-0xf195). This ring has a single + * head cursor instead, so a zero-status chain at head necessarily blocks + * everything behind it — hence the quiesced-context escape in + * InspectChainTail(), which Apple gets from resetDMA() instead. * * **Thread Safety** * Serialized via IOLock. Safe to call concurrently with SubmitChain(). @@ -268,6 +275,48 @@ class ATContextBase : public ContextBase { */ [[nodiscard]] std::optional ScanCompletion() noexcept; + /** + * \brief RAII scope that makes ScanCompletion() complete statusless descriptors. + * + * While held, a descriptor whose xferStatus is still zero is reported as + * evt_flushed instead of being treated as pending. OHCI 1.2 draft + * §7.2.3.3 (p. 7-14) permits a controller to leave outstanding AT + * descriptors without a final status after a bus reset. This does not prove + * that the corresponding packet was never transmitted, but the transaction + * must still be completed because hardware can no longer update it once the + * context is stopped. + * + * \warning The context must already be stopped. Entering this scope while + * hardware can still write a status would report packets that are + * merely in flight as flushed. + * + * **Reference Pattern** + * Linux at_context_flush() sets ctx->flushing around ohci_at_context_work() + * so the ordinary handle_at_packet() body drains the queue instead of + * stopping at the first zero status (ohci.c:1331-1343, 1364). AppleFWOHCI + * reaches the same end state through resetDMA(), which frees every pending + * ATxElement after stopDMA() (symbol offsets 0x5fae-0x5fd2). + */ + class [[nodiscard]] FlushScope { + public: + explicit FlushScope(ATContextBase& context) noexcept : context_(&context) { + context_->flushing_ = true; + } + ~FlushScope() { + if (context_) { + context_->flushing_ = false; + } + } + + FlushScope(const FlushScope&) = delete; + FlushScope& operator=(const FlushScope&) = delete; + FlushScope(FlushScope&&) = delete; + FlushScope& operator=(FlushScope&&) = delete; + + private: + ATContextBase* context_{nullptr}; + }; + /** * \brief Get descriptor ring for diagnostics. * @@ -314,6 +363,16 @@ class ATContextBase : public ContextBase { uint16_t timeStamp{0}; }; + enum class ChainTailScanResult : uint8_t { + NotRecognized, + Pending, + Advanced, + }; + + /// Set by FlushScope: report zero-status descriptors as evt_flushed. + /// Touched only under submitLock_ (set/cleared while the context is stopped). + bool flushing_{false}; + /// Descriptor ring for tracking in-flight chains DescriptorRing* ring_{nullptr}; @@ -341,6 +400,8 @@ class ATContextBase : public ContextBase { size_t capacity) noexcept; [[nodiscard]] bool LoadScanState(ScanState& state) noexcept; void FetchScanDescriptor(const ScanState& state) noexcept; + [[nodiscard]] ChainTailScanResult InspectChainTail( + const ScanState& state) noexcept; void HandlePendingDescriptor(const ScanState& state) noexcept; [[nodiscard]] bool IsOrphanedDescriptor(const ScanState& state, uint32_t& commandPtrAddr, @@ -605,7 +666,30 @@ std::optional ATContextBase::ScanCompletion() noexce return std::nullopt; } + // OHCI 1.2 draft §7.2.3.3 (p. 7-14): after a bus reset, optional + // controller behavior may leave outstanding AT descriptors without a + // final status. Reporting zero-status descriptors as evt_flushed + // releases the ring and fails their transactions instead of letting + // them time out; it does not imply that the packets were never sent. + // Linux uses the same flush-mode scan shape, mapping zero/no-status + // entries to RCODE_GENERATION (handle_at_packet() at ohci.c:1364, + // at_context_flush() at 1331-1343). AppleFWOHCI's resetDMA() frees every + // pending ATxElement after stopDMA() (symbol offsets 0x5fae-0x5fd2). + if (state.xferStatus == 0 && flushing_) { + state.xferStatus = static_cast(OHCIEventCode::kEvtFlushed); + } + if (state.xferStatus == 0) { + switch (InspectChainTail(state)) { + case ChainTailScanResult::Advanced: + continue; + case ChainTailScanResult::Pending: + unlock(); + return std::nullopt; + case ChainTailScanResult::NotRecognized: + break; + } + HandlePendingDescriptor(state); unlock(); return std::nullopt; @@ -826,6 +910,71 @@ void ATContextBase::FetchScanDescriptor(const ScanState& state) no } } +template +typename ATContextBase::ChainTailScanResult +ATContextBase::InspectChainTail(const ScanState& state) noexcept { + const uint16_t controlHi = static_cast( + state.desc->control >> HW::OHCIDescriptor::kControlHighShift); + const uint8_t command = static_cast( + (controlHi >> HW::OHCIDescriptor::kCmdShift) & 0xF); + if (command != HW::OHCIDescriptor::kCmdOutputMore) { + return ChainTailScanResult::NotRecognized; + } + + const uint8_t key = static_cast( + (controlHi >> HW::OHCIDescriptor::kKeyShift) & 0x7); + const uint8_t precursorBlocks = + (key == HW::OHCIDescriptor::kKeyImmediate) ? 2 : 1; + const size_t tailIndex = + (state.headIndex + precursorBlocks) % state.capacity; + if (tailIndex == state.tailIndex) { + return ChainTailScanResult::NotRecognized; + } + + ScanState tailState; + tailState.capacity = state.capacity; + tailState.headIndex = tailIndex; + tailState.desc = ring_->At(tailIndex); + if (!tailState.desc) { + return ChainTailScanResult::NotRecognized; + } + tailState.isImmediate = HW::IsImmediate(*tailState.desc); + FetchScanDescriptor(tailState); + const uint16_t tailControlHi = static_cast( + tailState.desc->control >> HW::OHCIDescriptor::kControlHighShift); + const uint8_t tailCommand = static_cast( + (tailControlHi >> HW::OHCIDescriptor::kCmdShift) & 0xF); + if (tailCommand != HW::OHCIDescriptor::kCmdOutputLast) { + return ChainTailScanResult::NotRecognized; + } + + if (HW::AT_xferStatus(*tailState.desc) == 0) { + // A running context has simply not finished this packet yet. The command + // pointer may already reference the pending tail, so the OUTPUT_MORE + // precursor is NOT orphaned and must stay queued. + // AppleFWOHCI 5.5.9 checkForCompletedElements() likewise leaves an + // ATxElement queued while its terminal descriptor status is zero + // (symbol offsets 0xf0e8-0xf10a), and Linux returns 0 from + // handle_at_packet() to stop iteration: + // references/linux-ohci-firewire-low-level-stack/drivers/firewire/ohci.c:1354-1366. + // + // A stopped context never writes a status either, but the chain must + // still be held here: FlushScope is what reports it as evt_flushed and + // fails the owning transaction. Releasing it from this path instead + // would consume the descriptors before the flush could see them. + return ChainTailScanResult::Pending; + } + + // Linux records completion on the packet's OUTPUT_LAST descriptor: + // references/linux-ohci-firewire-low-level-stack/drivers/firewire/ohci.c:1298-1310,1354-1366. + ClearDescriptorBlocks(state.headIndex, precursorBlocks, state.capacity); + ring_->SetHead(tailIndex); + ASFW_LOG_V2(Async, + "ScanCompletion: head %zu→%zu (completed OUTPUT_LAST after OUTPUT_MORE)", + state.headIndex, tailIndex); + return ChainTailScanResult::Advanced; +} + template void ATContextBase::HandlePendingDescriptor(const ScanState& state) noexcept { uint32_t commandPtrAddr = 0; diff --git a/ASFWDriver/Async/Contexts/ContextBase.hpp b/ASFWDriver/Async/Contexts/ContextBase.hpp index ad5170bd2..b3049d670 100644 --- a/ASFWDriver/Async/Contexts/ContextBase.hpp +++ b/ASFWDriver/Async/Contexts/ContextBase.hpp @@ -5,6 +5,7 @@ #include #include "../../Hardware/HardwareInterface.hpp" +#include "../../Hardware/OHCIConstants.hpp" #include "../../Hardware/RegisterMap.hpp" namespace ASFW::Async { @@ -254,8 +255,7 @@ class ContextBase { * Used for polling during context stop sequence. */ [[nodiscard]] bool IsActive() const noexcept { - constexpr uint32_t kActiveBit = 1u << 13; - return (ReadControl() & kActiveBit) != 0; + return (ReadControl() & Driver::kContextControlActiveBit) != 0; } /** @@ -264,8 +264,7 @@ class ContextBase { * \return true if ContextControl.run bit is set */ [[nodiscard]] bool IsRunning() const noexcept { - constexpr uint32_t kRunBit = 1u << 15; - return (ReadControl() & kRunBit) != 0; + return (ReadControl() & Driver::kContextControlRunBit) != 0; } /** diff --git a/ASFWDriver/Async/Engine/ContextManager.cpp b/ASFWDriver/Async/Engine/ContextManager.cpp index e240922de..169a682f9 100644 --- a/ASFWDriver/Async/Engine/ContextManager.cpp +++ b/ASFWDriver/Async/Engine/ContextManager.cpp @@ -350,11 +350,25 @@ kern_return_t ContextManager::stopAT() noexcept { ASFW_LOG(Async, "ContextManager::stopAT - stopping AT contexts"); - kern_return_t kr = state_->atReqCtx.Stop(); - if (kr != kIOReturnSuccess) return kr; + // Stop both contexts unconditionally, then report the first failure. An + // early return would leave the response context running while callers treat + // AT as quiesced — AsyncSubsystem::FlushATContexts() would then rewrite + // descriptors the controller still owns. Linux issues both context_stop() + // calls back to back for the same reason (ohci.c:1999-2000), as does + // AppleFWOHCI's handleBusResetInt() (stopDMA at symbol offsets + // 0x5fae and 0x5fba). + const kern_return_t reqKr = state_->atReqCtx.Stop(); + if (reqKr != kIOReturnSuccess) { + ASFW_LOG(Async, "ContextManager::stopAT - AT req stop failed (kr=0x%08x)", reqKr); + } - kr = state_->atRspCtx.Stop(); - if (kr != kIOReturnSuccess) return kr; + const kern_return_t rspKr = state_->atRspCtx.Stop(); + if (rspKr != kIOReturnSuccess) { + ASFW_LOG(Async, "ContextManager::stopAT - AT rsp stop failed (kr=0x%08x)", rspKr); + } + + if (reqKr != kIOReturnSuccess) return reqKr; + if (rspKr != kIOReturnSuccess) return rspKr; ASFW_LOG(Async, "ContextManager::stopAT - SUCCESS"); return kIOReturnSuccess; diff --git a/CLAUDE.md b/CLAUDE.md index 3b0cbb464..054d891c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,10 +14,12 @@ Two components: **`ASFW.xcodeproj` is GENERATED from the root `project.yml` (XcodeGen).** Never edit the pbxproj or hand-tune settings in Xcode — change `project.yml` and run -`xcodegen generate` (`./build.sh` does it automatically when xcodegen is -installed). After adding/removing/renaming source files, regenerate and commit -the updated `ASFW.xcodeproj` together with `project.yml`. Output is -deterministic; the generated project stays committed so CI builds without +`xcodegen generate` with the version pinned in `.xcodegen-version` +(`./build.sh` checks an installed xcodegen automatically; use +`--no-xcodegen` only to build the committed project as-is). After +adding/removing/renaming source files, regenerate and commit the updated +`ASFW.xcodeproj` together with `project.yml`. Output is deterministic for the +pinned version; the generated project stays committed so CI builds without xcodegen. (`ADKVirtualAudioLab/` has its own separate `project.yml`.) **Primary build (Xcode — required for signing and producing `.dext`):** diff --git a/README.md b/README.md index 3503420c4..50c9e5578 100644 --- a/README.md +++ b/README.md @@ -427,20 +427,57 @@ Build scripts or CMakeLists are for quick testing and creating compile_commands. ### Xcode project is generated (XcodeGen) `ASFW.xcodeproj` is generated from the root [`project.yml`](project.yml) with -[XcodeGen](https://github.com/yonaskolb/XcodeGen) (`brew install xcodegen`). -The generated project is committed, so plain checkouts (and CI) build without -XcodeGen installed — but **never edit the pbxproj or project settings in the -Xcode UI**; change `project.yml` instead. +[XcodeGen](https://github.com/yonaskolb/XcodeGen). The generated project is +committed, so plain checkouts (and CI) build without XcodeGen installed — but +**never edit the pbxproj or project settings in the Xcode UI**; change +`project.yml` instead. + +**The XcodeGen version is pinned** in [`.xcodegen-version`](.xcodegen-version), +because its output is *not* byte-identical across releases (2.45.4 → 2.46.0 +reorders the pbxproj `targets` list) and CI diffs the regenerated project +against the committed one. Homebrew's `xcodegen` floats, so install the pinned +release rather than `brew install xcodegen` — this is the same download, pin, +and checksum CI uses: + +```bash +. ./.xcodegen-version +XCODEGEN_TMP="$(mktemp -d)" +XCODEGEN_PREFIX="$HOME/.local" +curl --fail --location --retry 3 \ + --output "$XCODEGEN_TMP/xcodegen.zip" \ + "https://github.com/yonaskolb/XcodeGen/releases/download/${XCODEGEN_VERSION}/xcodegen.zip" +echo "${XCODEGEN_SHA256} ${XCODEGEN_TMP}/xcodegen.zip" | shasum -a 256 -c - +ditto -x -k "$XCODEGEN_TMP/xcodegen.zip" "$XCODEGEN_TMP/release" +mkdir -p "$XCODEGEN_PREFIX" +"$XCODEGEN_TMP/release/xcodegen/install.sh" "$XCODEGEN_PREFIX" +export PATH="$XCODEGEN_PREFIX/bin:$PATH" +hash -r +xcodegen --version +``` + +The `PATH` update deliberately places the pinned binary before a Homebrew +installation. Add the same `export` command to your shell startup file if you +want future terminal sessions to use the pinned version. + +If `xcodegen` isn't on your `PATH` at all, nothing above applies — `build.sh` +skips regeneration entirely and builds the committed project. The pin only +matters once you have *some* `xcodegen` installed, because `build.sh` +regenerates on every build; if that version doesn't match the pin it stops with +an error rather than writing a pbxproj CI would reject. Use `--no-xcodegen` to +build anyway. After **adding, removing, or renaming source files**, regenerate the project and commit it together with your change: ```bash -xcodegen generate # ./build.sh does this automatically when xcodegen is installed +xcodegen generate # ./build.sh does this automatically, and refuses to run + # if your xcodegen doesn't match the pin ``` -Output is deterministic — regenerating with no changes produces an identical -pbxproj. +Output is deterministic *for a given XcodeGen version* — regenerating with no +changes produces an identical pbxproj. If your machine has a different version +and you only need to build, pass `./build.sh --no-xcodegen` to use the committed +project as-is (added or removed sources will not be picked up). NOTE: You need an Apple Developer account (paid) and appropriate entitlements — or a free account plus SIP disabled — to build/load the driver on your machine. See Apple's documentation for details: https://developer.apple.com/documentation/driverkit/debugging-and-testing-system-extensions diff --git a/build.sh b/build.sh index 08ec15ab5..fb98c1f91 100755 --- a/build.sh +++ b/build.sh @@ -60,6 +60,10 @@ SWIFT_COVERAGE_LCOV="${BUILD_DIR}/swift_coverage.lcov" # See README "SCSI HBA — opt-in". ENABLE_SCSI=false +# Skip the XcodeGen regeneration in preflight() and build the committed project +# as-is. Escape hatch for a machine whose xcodegen differs from the pin. +NO_XCODEGEN=false + usage() { cat </dev/null 2>&1; then + # source globs pick up added/removed files; output is deterministic *for a + # given xcodegen version*, so this is a no-op when nothing changed. Falls + # through to the committed project when xcodegen isn't installed (e.g. CI + # runners, which use the pinned release directly). + if [[ -f "project.yml" ]] && ! $NO_XCODEGEN && command -v xcodegen >/dev/null 2>&1; then + # XcodeGen output is not byte-identical across releases, and CI diffs the + # regenerated pbxproj against the committed one. Regenerating with an + # unpinned version silently produces a project that CI will reject, so + # stop here instead — a build missing a newly added source file is much + # harder to diagnose than this message. + if [[ ! -r ".xcodegen-version" ]]; then + err "xcodegen is installed, but the required .xcodegen-version pin is missing or unreadable." + err "Refusing to regenerate ${PROJECT_NAME}.xcodeproj with an unpinned version." + exit 1 + fi + unset XCODEGEN_VERSION + . ./.xcodegen-version + if [[ -z "${XCODEGEN_VERSION:-}" ]]; then + err ".xcodegen-version does not define XCODEGEN_VERSION." + err "Refusing to regenerate ${PROJECT_NAME}.xcodeproj with an unpinned version." + exit 1 + fi + xcodegen_actual="$(xcodegen --version 2>/dev/null | awk '{print $NF}')" + if [[ "$xcodegen_actual" != "$XCODEGEN_VERSION" ]]; then + err "xcodegen ${xcodegen_actual:-} is installed, but this repo pins ${XCODEGEN_VERSION}." + err "Regenerating would produce a pbxproj that CI's drift check rejects." + err "Either:" + err " - install the pinned release (copy-paste block in README -> Building ->" + err " 'Xcode project is generated (XcodeGen)'), or" + err " - re-run with --no-xcodegen to build the committed project as-is" + err " (added/removed source files will NOT be picked up)." + exit 1 + fi log "Regenerating ${PROJECT_NAME}.xcodeproj from project.yml..." xcodegen generate --quiet || { err "xcodegen generate failed"; exit 1; } fi @@ -231,7 +270,7 @@ run_swift_tests() { if $VERBOSE; then xcodebuild "${XCODEBUILD_ARGS[@]}" 2>&1 else - xcodebuild "${XCODEBUILD_ARGS[@]}" 2>&1 | grep -E '(Test Case|passed|failed|error:)' || true + xcodebuild "${XCODEBUILD_ARGS[@]}" 2>&1 | grep -E '(Test Case|passed|failed|error:)' fi local test_status=${PIPESTATUS[0]} set -e diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d0eb481a0..ba76bfb64 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -50,6 +50,8 @@ if (NOT GTest_FOUND) endif() include(GoogleTest) +# Avoid parallel post-build discovery racing on empty GoogleTest JSON output. +set(CMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE PRE_TEST) # Define common test settings interface library add_library(asfw_test_common_interface INTERFACE) diff --git a/tests/async/ATContextCompletionTests.cpp b/tests/async/ATContextCompletionTests.cpp new file mode 100644 index 000000000..7cfee63a5 --- /dev/null +++ b/tests/async/ATContextCompletionTests.cpp @@ -0,0 +1,259 @@ +#include +#include + +#include + +#include "ASFWDriver/Async/Contexts/ATRequestContext.hpp" +#include "ASFWDriver/Hardware/OHCIConstants.hpp" +#include "ASFWDriver/Hardware/OHCIDescriptors.hpp" +#include "ASFWDriver/Hardware/RegisterMap.hpp" +#include "ASFWDriver/Shared/Memory/DMAMemoryManager.hpp" +#include "ASFWDriver/Shared/Rings/DescriptorRing.hpp" + +namespace ASFW::Testing { +namespace { + +constexpr size_t kDescriptorCount = 8; +constexpr uint8_t kTLabel = 45; +constexpr uint16_t kTimestamp = 0x1234; + +class ATContextCompletionTest : public ::testing::Test { +protected: + Driver::HardwareInterface hardware_; + Shared::DMAMemoryManager dma_; + Shared::DescriptorRing ring_; + Async::ATRequestContext context_; + + void SetUp() override { + ASSERT_TRUE(dma_.Initialize( + hardware_, kDescriptorCount * sizeof(Async::HW::OHCIDescriptor))); + + auto region = dma_.AllocateRegion( + kDescriptorCount * sizeof(Async::HW::OHCIDescriptor)); + ASSERT_TRUE(region.has_value()); + + auto* descriptors = + reinterpret_cast(region->virtualBase); + ASSERT_TRUE(ring_.Initialize( + std::span{descriptors, kDescriptorCount})); + ASSERT_TRUE(ring_.Finalize(region->deviceBase)); + ASSERT_EQ(context_.Initialize(hardware_, ring_, dma_), kIOReturnSuccess); + } + + void PrepareBlockWriteChain( + uint16_t payloadStatus = + static_cast(Async::OHCIEventCode::kAckComplete)) { + auto* header = reinterpret_cast(ring_.At(0)); + ASSERT_NE(header, nullptr); + header->common.control = Async::HW::OHCIDescriptor::BuildControl({ + .reqCount = 16, + .command = Async::HW::OHCIDescriptor::kCmdOutputMore, + .key = Async::HW::OHCIDescriptor::kKeyImmediate, + .interruptBits = Async::HW::OHCIDescriptor::kIntNever, + .branchBits = Async::HW::OHCIDescriptor::kBranchAlways, + }); + header->immediateData[0] = static_cast(kTLabel) << 10; + + auto* payload = ring_.At(2); + ASSERT_NE(payload, nullptr); + payload->control = Async::HW::OHCIDescriptor::BuildControl({ + .reqCount = 8, + .command = Async::HW::OHCIDescriptor::kCmdOutputLast, + .key = Async::HW::OHCIDescriptor::kKeyStandard, + .interruptBits = Async::HW::OHCIDescriptor::kIntAlways, + .branchBits = Async::HW::OHCIDescriptor::kBranchNever, + }); + payload->timeStamp = kTimestamp; + payload->xferStatus = payloadStatus; + + ring_.SetTail(3); + hardware_.SetTestRegister( + Async::ATRequestTag::kControlSetReg, + Driver::kContextControlRunBit); + hardware_.SetTestRegister( + Async::ATRequestTag::kCommandPtrReg, + ring_.CommandPtrWordTo(ring_.At(0), 3)); + } +}; + +TEST_F(ATContextCompletionTest, + UsesOutputLastStatusWhenOutputMorePrecursorHasNoStatus) { + PrepareBlockWriteChain(); + + const auto completion = context_.ScanCompletion(); + + ASSERT_TRUE(completion.has_value()); + EXPECT_EQ(completion->eventCode, Async::OHCIEventCode::kAckComplete); + EXPECT_EQ(completion->timeStamp, kTimestamp); + EXPECT_EQ(completion->tLabel, kTLabel); + EXPECT_EQ(completion->descriptor, ring_.At(2)); + EXPECT_EQ(ring_.Head(), 3u); + EXPECT_TRUE(ring_.IsEmpty()); +} + +TEST_F(ATContextCompletionTest, + WaitsWhenOutputMoreAndOutputLastBothHaveNoStatus) { + PrepareBlockWriteChain(0); + + const auto completion = context_.ScanCompletion(); + + EXPECT_FALSE(completion.has_value()); + EXPECT_EQ(ring_.Head(), 0u); + EXPECT_FALSE(ring_.IsEmpty()); +} + +TEST_F(ATContextCompletionTest, + PreservesRecognizedChainWhenCommandPtrAdvancesToPendingOutputLast) { + PrepareBlockWriteChain(0); + hardware_.SetTestRegister( + Async::ATRequestTag::kCommandPtrReg, + ring_.CommandPtrWordTo(ring_.At(2), 1)); + + const auto pendingCompletion = context_.ScanCompletion(); + + EXPECT_FALSE(pendingCompletion.has_value()); + EXPECT_EQ(ring_.Head(), 0u); + EXPECT_FALSE(ring_.IsEmpty()); + + auto* payload = ring_.At(2); + ASSERT_NE(payload, nullptr); + payload->xferStatus = + static_cast(Async::OHCIEventCode::kAckComplete); + + const auto completion = context_.ScanCompletion(); + + ASSERT_TRUE(completion.has_value()); + EXPECT_EQ(completion->eventCode, Async::OHCIEventCode::kAckComplete); + EXPECT_EQ(completion->timeStamp, kTimestamp); + EXPECT_EQ(completion->tLabel, kTLabel); + EXPECT_EQ(completion->descriptor, payload); + EXPECT_EQ(ring_.Head(), 3u); + EXPECT_TRUE(ring_.IsEmpty()); +} + +TEST_F(ATContextCompletionTest, AdvancesTrulyOrphanedPendingDescriptor) { + auto* descriptor = ring_.At(0); + ASSERT_NE(descriptor, nullptr); + descriptor->control = Async::HW::OHCIDescriptor::BuildControl({ + .reqCount = 8, + .command = Async::HW::OHCIDescriptor::kCmdOutputLast, + .key = Async::HW::OHCIDescriptor::kKeyStandard, + .interruptBits = Async::HW::OHCIDescriptor::kIntAlways, + .branchBits = Async::HW::OHCIDescriptor::kBranchNever, + }); + descriptor->xferStatus = 0; + ring_.SetTail(1); + hardware_.SetTestRegister(Async::ATRequestTag::kControlSetReg, 0); + hardware_.SetTestRegister( + Async::ATRequestTag::kCommandPtrReg, + ring_.CommandPtrWordTo(descriptor, 1)); + + const auto completion = context_.ScanCompletion(); + + EXPECT_FALSE(completion.has_value()); + EXPECT_EQ(ring_.Head(), 1u); + EXPECT_TRUE(ring_.IsEmpty()); +} + +// Once a context has stopped, hardware can no longer write a final status, but +// an ordinary scan must still leave the chain alone: FlushScope is what reports +// it and fails the owning transaction. A scan that landed between +// StopATContextsOnly() and FlushATContexts() (the watchdog drain runs on a timer) +// would otherwise consume the descriptors first, leaving the transaction to +// time out instead. +TEST_F(ATContextCompletionTest, HoldsQuiescedChainSoTheFlushCanReportIt) { + PrepareBlockWriteChain(0); + hardware_.SetTestRegister(Async::ATRequestTag::kControlSetReg, 0); + + for (int i = 0; i < 4; ++i) { + const auto completion = context_.ScanCompletion(); + EXPECT_FALSE(completion.has_value()); + } + + EXPECT_EQ(ring_.Head(), 0u); + EXPECT_FALSE(ring_.IsEmpty()); + + Async::ATRequestContext::FlushScope flush(context_); + const auto flushed = context_.ScanCompletion(); + + ASSERT_TRUE(flushed.has_value()); + EXPECT_EQ(flushed->eventCode, Async::OHCIEventCode::kEvtFlushed); + EXPECT_EQ(flushed->tLabel, kTLabel); + EXPECT_TRUE(ring_.IsEmpty()); +} + +// The quiesced-context drain above releases the ring but reports nothing, so the +// owning transaction only fails once it times out. Inside a FlushScope the same +// chain is reported as evt_flushed, which TransactionCompletionHandler maps to +// kIOReturnAborted — Linux calls the equivalent RCODE_GENERATION "the same error +// as when we try to use a stale generation count" (ohci.c:1387-1393). +TEST_F(ATContextCompletionTest, FlushScopeReportsStatuslessChainAsFlushed) { + PrepareBlockWriteChain(0); + hardware_.SetTestRegister(Async::ATRequestTag::kControlSetReg, 0); + + Async::ATRequestContext::FlushScope flush(context_); + + const auto completion = context_.ScanCompletion(); + + ASSERT_TRUE(completion.has_value()); + EXPECT_EQ(completion->eventCode, Async::OHCIEventCode::kEvtFlushed); + EXPECT_EQ(completion->tLabel, kTLabel); + EXPECT_EQ(completion->descriptor, ring_.At(2)); + EXPECT_EQ(ring_.Head(), 3u); + EXPECT_TRUE(ring_.IsEmpty()); +} + +// A descriptor with a real completion keeps that status while flushing; only +// zero-status descriptors are synthesized. +TEST_F(ATContextCompletionTest, FlushScopePreservesRealCompletionStatus) { + PrepareBlockWriteChain(); + + Async::ATRequestContext::FlushScope flush(context_); + + const auto completion = context_.ScanCompletion(); + + ASSERT_TRUE(completion.has_value()); + EXPECT_EQ(completion->eventCode, Async::OHCIEventCode::kAckComplete); + EXPECT_EQ(completion->timeStamp, kTimestamp); + EXPECT_EQ(completion->tLabel, kTLabel); + EXPECT_TRUE(ring_.IsEmpty()); +} + +// FlushATContexts() gates FlushScope on IsActive(). This pins the property that +// gate depends on: a context whose ACTIVE bit is still set must not be flushed, +// because its queued packets may still be in flight, and clearing their +// descriptor words would cut the chain the controller is traversing. +TEST_F(ATContextCompletionTest, ActiveContextIsNotEligibleForFlush) { + PrepareBlockWriteChain(0); + hardware_.SetTestRegister( + Async::ATRequestTag::kControlSetReg, + Driver::kContextControlRunBit | Driver::kContextControlActiveBit); + + ASSERT_TRUE(context_.IsActive()); + + // What FlushATContexts() does when IsActive() reports true: no FlushScope. + const auto completion = context_.ScanCompletion(); + + EXPECT_FALSE(completion.has_value()); + EXPECT_EQ(ring_.Head(), 0u); + EXPECT_FALSE(ring_.IsEmpty()); + EXPECT_EQ(ring_.At(2)->xferStatus, 0); +} + +// Leaving the scope must restore normal pending semantics, otherwise a live +// context would start reporting in-flight packets as flushed. +TEST_F(ATContextCompletionTest, FlushScopeRestoresPendingSemanticsOnExit) { + PrepareBlockWriteChain(0); + { + Async::ATRequestContext::FlushScope flush(context_); + } + + const auto completion = context_.ScanCompletion(); + + EXPECT_FALSE(completion.has_value()); + EXPECT_EQ(ring_.Head(), 0u); + EXPECT_FALSE(ring_.IsEmpty()); +} + +} // namespace +} // namespace ASFW::Testing diff --git a/tests/async/CMakeLists.txt b/tests/async/CMakeLists.txt index 19bd8ae20..54f5f2dad 100644 --- a/tests/async/CMakeLists.txt +++ b/tests/async/CMakeLists.txt @@ -68,6 +68,14 @@ add_async_test(ATDescriptorTests ATDescriptorTests.cpp ) +# AT Context Completion Tests +add_async_test(ATContextCompletionTests + ATContextCompletionTests.cpp + "${ASFW_DRIVER_DIR}/Shared/Memory/DMAMemoryManager.cpp" + "${ASFW_DRIVER_DIR}/Shared/Rings/DescriptorRing.cpp" + "${ASFW_DRIVER_DIR}/Common/BarrierUtils.cpp" +) + # Buffer Ring DMA Tests add_async_test(BufferRingDMATests BufferRingDMATests.cpp