diff --git a/ASFW.xcodeproj/project.pbxproj b/ASFW.xcodeproj/project.pbxproj index 89824f2dc..bae1f9e36 100644 --- a/ASFW.xcodeproj/project.pbxproj +++ b/ASFW.xcodeproj/project.pbxproj @@ -178,6 +178,7 @@ 46EAC460C610E97D83C42F43 /* AVCMusicCapabilities.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 38B33C0D51014568208DBB2A /* AVCMusicCapabilities.cpp */; }; 47C7D2797F3974490450A45A /* AVCNestedChannelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB92C50A509F45F51EFD3F03 /* AVCNestedChannelTests.swift */; }; 47F41CED6EB2EE793A31B6E5 /* ASFWAudioDriverControls.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 018751CF8CA051FDDE9B2B06 /* ASFWAudioDriverControls.cpp */; }; + 47F7CC1D374745AF715C8AB2 /* PhysicalRequestFilter.cpp in Sources */ = {isa = PBXBuildFile; fileRef = AE9C2F9E30033532DA95E435 /* PhysicalRequestFilter.cpp */; }; 48359CD69DF9A8F5868E12C0 /* ASFWMCPLiveDriverControl.swift in Sources */ = {isa = PBXBuildFile; fileRef = D1391A71639068D8291B32AF /* ASFWMCPLiveDriverControl.swift */; }; 4A7F808CC9DBBBED16B2F07A /* ConfigROMBuilder.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 290CF8A5D9D6CABD1B3BF37A /* ConfigROMBuilder.cpp */; }; 4A8DB4DE00645A2458BAF883 /* MidiNubPublisher.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2376C0878D2E3E97CF4DD0CE /* MidiNubPublisher.cpp */; }; @@ -1401,6 +1402,7 @@ AE208856E70C5C9AC1A54F71 /* ASFW-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "ASFW-Bridging-Header.h"; sourceTree = ""; }; AE3316AEFE76C1A5D9641992 /* MidiTxReservation.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = MidiTxReservation.hpp; sourceTree = ""; }; AE6917642BC190CEA2F5E7E3 /* BusManagerRuntimeState.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = BusManagerRuntimeState.hpp; sourceTree = ""; }; + AE9C2F9E30033532DA95E435 /* PhysicalRequestFilter.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = PhysicalRequestFilter.cpp; sourceTree = ""; }; AEC88EDC2EC1E38DC23B8916 /* AudioCoordinator.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = AudioCoordinator.hpp; sourceTree = ""; }; AED113166DE4061F3A41FC54 /* BoundedDequeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BoundedDequeTests.swift; sourceTree = ""; }; AED823683733E47EADBB95EB /* DriverConnector+IRM.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DriverConnector+IRM.swift"; sourceTree = ""; }; @@ -3554,6 +3556,7 @@ A418B8B548743D3952A8089B /* OHCIConstants.hpp */, C197F16F4A4D565C13DA4286 /* OHCIDescriptors.hpp */, 2AEC69885AF2551714DE1936 /* OHCIEventCodes.hpp */, + AE9C2F9E30033532DA95E435 /* PhysicalRequestFilter.cpp */, E90F7A6B5C08D9C94CD93034 /* RegisterMap.hpp */, ); path = Hardware; @@ -4562,6 +4565,7 @@ E8E793E5929460B3A42C034D /* PayloadRegistry.cpp in Sources */, 3C130CEE2A5FE017326B568E /* PcmSlotCodec.cpp in Sources */, 836FE3225BA2D368808EFC48 /* PhyCommand.cpp in Sources */, + 47F7CC1D374745AF715C8AB2 /* PhysicalRequestFilter.cpp in Sources */, 3ECDDF5924BE520E1750A8FF /* PostResetTimingCoordinator.cpp in Sources */, 270027C27155433C83E72998 /* PowerLinkPolicyCoordinator.cpp in Sources */, 3455F76072D6ED0ED7E7B77E /* ROMReader.cpp in Sources */, diff --git a/ASFWDriver/ASFWDriver.cpp b/ASFWDriver/ASFWDriver.cpp index 384bc2aad..3f9641f65 100644 --- a/ASFWDriver/ASFWDriver.cpp +++ b/ASFWDriver/ASFWDriver.cpp @@ -212,6 +212,16 @@ void ExecuteRuntimeTeardown(ServiceContext& ctx, const QuiescePlan& plan) { ctx.deps.avcDiscovery->Shutdown(); } ASFW::Protocols::SBP2::SBP2BridgeHub::Clear(); + // The bridge releases sessions whose commands the target may still be + // serving by DMA, and their logouts may fail on a bus that is going down. + // Shut the physical response unit first: then no target can reach a + // released buffer, and a failed logout needs no bus reset. + if (ctx.deps.hardware) { + ctx.deps.hardware->ClearPhysicalRequestFilters(); + } + if (ctx.deps.sbp2SessionRegistry) { + ctx.deps.sbp2SessionRegistry->PrepareForTeardown(); + } if (ctx.sbp2Bridge) { ctx.sbp2Bridge->Shutdown(); ctx.sbp2Bridge.reset(); diff --git a/ASFWDriver/Async/AsyncSubsystemLifecycle.cpp b/ASFWDriver/Async/AsyncSubsystemLifecycle.cpp index d1fb87de7..67fa11f50 100644 --- a/ASFWDriver/Async/AsyncSubsystemLifecycle.cpp +++ b/ASFWDriver/Async/AsyncSubsystemLifecycle.cpp @@ -472,6 +472,7 @@ void AsyncSubsystem::Teardown(bool disableHardware) { if (disableHardware && hardware_) { hardware_->SetInterruptMask(0xFFFFFFFFu, false); hardware_->ClearLinkControlBits(Driver::LinkControlBits::kRcvPhyPkt); + hardware_->ClearPhysicalRequestFilters(); } // CRITICAL: Deactivate completion queue BEFORE stopping contexts diff --git a/ASFWDriver/Controller/ControllerCoreInterrupts.cpp b/ASFWDriver/Controller/ControllerCoreInterrupts.cpp index de4161f9c..1ef2980d1 100644 --- a/ASFWDriver/Controller/ControllerCoreInterrupts.cpp +++ b/ASFWDriver/Controller/ControllerCoreInterrupts.cpp @@ -60,6 +60,9 @@ IsochContextEvents ControllerCore::HandleInterrupt(const InterruptSnapshot& snap HandleFaultInterrupts(events); NotifyBusResetCoordinator(events, snapshot.timestamp); if ((events & IntEventBits::kBusReset) != 0U) { + // Node numbers are reassigned by the reset; no node stays admitted to + // physical DMA until its session re-enables it for the new generation. + hw.ClearPhysicalRequestFilters(); auto access = hw.TryBeginAccess(); if (!access) return {}; // SelfIDCount.selfIDGeneration already holds the new generation at the diff --git a/ASFWDriver/Controller/ControllerCoreLifecycle.cpp b/ASFWDriver/Controller/ControllerCoreLifecycle.cpp index 3ed2c05a3..b2040aa3e 100644 --- a/ASFWDriver/Controller/ControllerCoreLifecycle.cpp +++ b/ASFWDriver/Controller/ControllerCoreLifecycle.cpp @@ -725,8 +725,11 @@ kern_return_t ControllerCore::InitialiseHardware(IOService* provider) { return configRomStatus; } - // Step 7: Set Physical Upper Bound (256MB CSR address range) - // TODO(ASFW-DMA): Confirm whether remote DMA still requires this register programming. + // Step 7: bound physical responses to the 4 GiB every DMA IOVA lives in + // (AllocateDMA asks for 32-bit addresses), as Linux ohci_enable does. + if (auto access = hw.TryBeginAccess()) { + access.WriteAndFlush(Register32::kPhyUpperBound, ASFW::Driver::kPhysicalUpperBound); + } // Per Linux ohci_enable(): Don't pre-write NodeID; bus reset will assign it from Self-ID // The kProvisionalNodeId value would be immediately overwritten anyway hw.SetLinkControlBits(ASFW::Driver::kDefaultLinkControl); diff --git a/ASFWDriver/Hardware/HardwareInterface.hpp b/ASFWDriver/Hardware/HardwareInterface.hpp index 3e56f9925..820914258 100644 --- a/ASFWDriver/Hardware/HardwareInterface.hpp +++ b/ASFWDriver/Hardware/HardwareInterface.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include // for std::pair @@ -32,6 +33,8 @@ class IAsyncControllerPort; namespace ASFW::Driver { +enum class PhysicalDmaError : uint8_t { InvalidNode, HardwareGone, StaleGeneration }; + /// Result of a local autonomous IRM CSR compare-swap operation (OHCI §5.5). struct LocalCSRLockResult { enum class Status : uint8_t { @@ -110,6 +113,12 @@ class HardwareInterface { [[nodiscard]] PciInterruptState ReadPciInterruptState() const noexcept; void SetLinkControlBits(uint32_t bits); void ClearLinkControlBits(uint32_t bits); + // Admit one local node's physical requests for the current generation, the + // way Linux ohci_enable_phys_dma does. Nothing disables a node; the filter + // is cleared as a whole at bus reset and teardown. + [[nodiscard]] std::expected + EnablePhysicalRequests(uint16_t node, uint16_t generation) noexcept; + void ClearPhysicalRequestFilters() noexcept; void ClearIntEvents(uint32_t mask); // Acknowledges the caller-selected global bits, then reads and clears each // signalled isoch direction. One readback completes the entire batch before diff --git a/ASFWDriver/Hardware/OHCIConstants.hpp b/ASFWDriver/Hardware/OHCIConstants.hpp index d7e794124..a51f6a3c3 100644 --- a/ASFWDriver/Hardware/OHCIConstants.hpp +++ b/ASFWDriver/Hardware/OHCIConstants.hpp @@ -12,6 +12,9 @@ namespace ASFW::Driver { // AR Filter Constants (OHCI §7.4) // Bit 31 in AsReqFilterHiSet = accept all async requests constexpr uint32_t kAsReqAcceptAllMask = 0x80000000u; +// PhysicalUpperBound: physical responses serve the first 4 GiB, where every +// IOVA this driver hands a target lives (Linux ohci.c:2348, core.h:289). +constexpr uint32_t kPhysicalUpperBound = 0x00010000u; // Default link control configuration used during controller initialization. // Keep the local cycle timer running for cycleLost/cycleSynch accounting, but diff --git a/ASFWDriver/Hardware/PhysicalRequestFilter.cpp b/ASFWDriver/Hardware/PhysicalRequestFilter.cpp new file mode 100644 index 000000000..1476842bd --- /dev/null +++ b/ASFWDriver/Hardware/PhysicalRequestFilter.cpp @@ -0,0 +1,52 @@ +#include "HardwareInterface.hpp" +#include "OHCIConstants.hpp" +#include "RegisterMap.hpp" +#include "../Logging/Logging.hpp" + +namespace ASFW::Driver { + +std::expected +HardwareInterface::EnablePhysicalRequests(uint16_t node, uint16_t generation) noexcept { + const uint16_t bus = node & 0xFFC0u; + const uint16_t phy = node & 0x3Fu; + if ((bus != 0 && bus != 0xFFC0u) || phy == 63) { + return std::unexpected(PhysicalDmaError::InvalidNode); + } + auto access = TryBeginAccess(); + if (!access) { + return std::unexpected(PhysicalDmaError::HardwareGone); + } + // Linux ohci.c:2615: a stale generation must not arm a node number the + // reset may have handed to another device. Linux checks under the lock its + // reset work also takes; we have no such lock against the interrupt path, + // so the check is repeated after the write and the bit taken back if a + // reset slipped in between. + const auto stale = [&access, generation]() { + const uint32_t current = (access.Read(Register32::kSelfIDCount) & + SelfIDCountBits::kGenerationMask) >> SelfIDCountBits::kGenerationShift; + return current != (generation & 0xFFu) || + (access.Read(Register32::kIntEvent) & IntEventBits::kBusReset) != 0; + }; + if (stale()) { + return std::unexpected(PhysicalDmaError::StaleGeneration); + } + const Register32 set = phy < 32 ? Register32::kPhyReqFilterLoSet : Register32::kPhyReqFilterHiSet; + const Register32 clear = phy < 32 ? Register32::kPhyReqFilterLoClear : Register32::kPhyReqFilterHiClear; + const uint32_t bit = 1u << (phy % 32); + access.WriteAndFlush(set, bit); + if (stale()) { + access.WriteAndFlush(clear, bit); + return std::unexpected(PhysicalDmaError::StaleGeneration); + } + ASFW_LOG(Hardware, "[PhysicalDMA] node=%u gen=%u enabled", phy, generation); + return {}; +} + +void HardwareInterface::ClearPhysicalRequestFilters() noexcept { + if (auto access = TryBeginAccess()) { + access.Write(Register32::kPhyReqFilterHiClear, 0xFFFFFFFFu); + access.WriteAndFlush(Register32::kPhyReqFilterLoClear, 0xFFFFFFFFu); + } +} + +} // namespace ASFW::Driver diff --git a/ASFWDriver/Protocols/SBP2/AddressSpaceManager.hpp b/ASFWDriver/Protocols/SBP2/AddressSpaceManager.hpp index bfdd5394c..417cbc7b7 100644 --- a/ASFWDriver/Protocols/SBP2/AddressSpaceManager.hpp +++ b/ASFWDriver/Protocols/SBP2/AddressSpaceManager.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -152,6 +153,64 @@ class AddressSpaceManager { return kr; } + // A buffer the target reaches through the OHCI physical response unit: its + // 1394 address is the buffer's own IOVA (below 4 GiB, addressHi 0), so the + // controller serves the target's reads and writes by DMA with no software + // in the path. Apple (IOFireWireSBP2ORB.cpp:1002-1041) and Linux + // (sbp2_map_scatterlist) carry SBP-2 data this way. The range stays in the + // table so the CPU side can read it by handle; it has no shadow copy. + kern_return_t AllocatePhysicalRange(void* owner, + uint32_t length, + uint64_t* outHandle, + AddressRangeMeta* outMeta = nullptr) { + if (!lock_ || !outHandle || length == 0) { + return kIOReturnBadArgument; + } + + AddressRange range{}; + range.owner = owner; + range.physical = true; + range.meta.length = length; + + IOLockLock(lock_); + const kern_return_t kr = AllocateBacking(range); + if (kr != kIOReturnSuccess) { + IOLockUnlock(lock_); + return kr; + } + uint64_t address = range.deviceAddress; + if (!range.hasBacking) { + if (hardware_) { + IOLockUnlock(lock_); + return kIOReturnNoMemory; + } + // Host tests have no DMA: stand in with a unique sub-4 GiB address. + address = kHostTestPhysicalBase + hostTestPhysicalCursor_; + hostTestPhysicalCursor_ += AlignUp(length, 4096); + range.buffer.resize(length, 0); + } + for (const auto& [handle, existing] : ranges_) { + if (RangesOverlap(address, length, existing.meta.address, existing.meta.length)) { + CleanupBacking(range); + IOLockUnlock(lock_); + return kIOReturnNoSpace; + } + } + range.meta.handle = nextHandle_++; + range.meta.address = address; + range.meta.addressHi = static_cast(address >> 32); + range.meta.addressLo = static_cast(address & 0xFFFF'FFFFULL); + + const uint64_t handle = range.meta.handle; + if (outMeta) { + *outMeta = range.meta; + } + ranges_.emplace(handle, std::move(range)); + IOLockUnlock(lock_); + *outHandle = handle; + return kIOReturnSuccess; + } + kern_return_t DeallocateAddressRange(void* owner, uint64_t handle) { if (!lock_ || handle == 0) { return kIOReturnBadArgument; @@ -216,8 +275,8 @@ class AddressSpaceManager { return kIOReturnNoSpace; } - outData->assign(range.buffer.begin() + static_cast(offset), - range.buffer.begin() + static_cast(offset + length)); + const auto bytes = Storage(range).subspan(offset, length); + outData->assign(bytes.begin(), bytes.end()); IOLockUnlock(lock_); return kIOReturnSuccess; } @@ -349,8 +408,7 @@ class AddressSpaceManager { } const uint32_t offset = static_cast(address - range->meta.address); - std::memcpy(outValue, - range->buffer.data() + static_cast(offset), + std::memcpy(outValue, Storage(*range).subspan(offset, sizeof(uint32_t)).data(), sizeof(uint32_t)); IOLockUnlock(lock_); @@ -429,6 +487,7 @@ class AddressSpaceManager { static constexpr uint32_t kAutoAddressWindowStartLo = 0x0010'0000u; static constexpr uint32_t kAutoAddressWindowEndLo = 0x0FFF'FFFFu; static constexpr uint64_t kAutoAddressAlignment = 8ULL; + static constexpr uint64_t kHostTestPhysicalBase = 0x4000'0000ULL; static constexpr std::size_t kDebugLabelCapacity = 32; @@ -445,8 +504,18 @@ class AddressSpaceManager { uint8_t* mappedBytes{nullptr}; uint64_t deviceAddress{0}; bool hasBacking{false}; + bool physical{false}; }; + // Where the bytes live: the DMA mapping for a physical range (the target + // writes it directly), the shadow copy for a software-served range. + static std::span Storage(const AddressRange& range) { + if (range.physical && range.hasBacking) { + return {range.mappedBytes, range.meta.length}; + } + return range.buffer; + } + static void CopyDebugLabel(AddressRange& range, const char* label) { range.debugLabel.fill('\0'); if (!label) { @@ -606,9 +675,11 @@ class AddressSpaceManager { #endif } + // Software-served ranges are written by the CPU per request and stay + // uncached; a physical range is filled by the device and read once. IOMemoryMap* mapping = nullptr; const kern_return_t kr = dma->descriptor->CreateMapping( - kIOMemoryMapCacheModeInhibit, + range.physical ? 0 : kIOMemoryMapCacheModeInhibit, 0, 0, size, @@ -632,7 +703,11 @@ class AddressSpaceManager { return kIOReturnNoMemory; } - ZeroMappedBytes(mapped, size); + if (range.physical) { + std::ranges::fill(std::span{mapped, size}, uint8_t{0}); + } else { + ZeroMappedBytes(mapped, size); + } OSSynchronizeIO(); range.descriptor = std::move(dma->descriptor); @@ -744,7 +819,9 @@ class AddressSpaceManager { static_cast(reinterpret_cast(data.data()) & 0x7ULL), static_cast(reinterpret_cast(range.buffer.data()) & 0x7ULL), static_cast(reinterpret_cast(range.mappedBytes) & 0x7ULL)); - CopyPayloadBytes(range.buffer.data() + static_cast(offset), data); + if (!range.buffer.empty()) { + CopyPayloadBytes(range.buffer.data() + static_cast(offset), data); + } if (range.hasBacking && range.mappedBytes) { CopyPayloadBytes(range.mappedBytes + static_cast(offset), data); @@ -756,6 +833,7 @@ class AddressSpaceManager { ASFW::Driver::HardwareInterface* hardware_{nullptr}; IOLock* lock_{nullptr}; uint64_t nextHandle_{1}; + uint64_t hostTestPhysicalCursor_{0}; std::unordered_map ranges_; }; diff --git a/ASFWDriver/Protocols/SBP2/SCSICommandSet.hpp b/ASFWDriver/Protocols/SBP2/SCSICommandSet.hpp index ded495b24..5cad293b3 100644 --- a/ASFWDriver/Protocols/SBP2/SCSICommandSet.hpp +++ b/ASFWDriver/Protocols/SBP2/SCSICommandSet.hpp @@ -22,6 +22,10 @@ struct CommandRequest { std::vector outgoingPayload{}; uint32_t timeoutMs{2000}; bool captureSenseData{false}; + // Bus address of the caller's own DMA-mapped buffer, below 4 GiB. When set, + // the target moves the data there directly: no bounce buffer, no copy, and + // outgoingPayload stays empty for data-out. + uint64_t dataIOVA{0}; [[nodiscard]] bool HasTransferBuffer() const noexcept { return transferLength > 0; @@ -39,6 +43,9 @@ struct CommandResult { bool scsiStatusValid{false}; std::vector payload{}; std::vector senseData{}; + // Bytes the target moved straight into the caller's buffer (dataIOVA); + // payload stays empty for those commands. + uint32_t inPlaceBytes{0}; }; // SBP-2 Annex B packs autosense into the status block's command-set-dependent diff --git a/ASFWDriver/Protocols/SBP2/Session/CommandExecutor.cpp b/ASFWDriver/Protocols/SBP2/Session/CommandExecutor.cpp index 54d8553fb..78252118c 100644 --- a/ASFWDriver/Protocols/SBP2/Session/CommandExecutor.cpp +++ b/ASFWDriver/Protocols/SBP2/Session/CommandExecutor.cpp @@ -33,6 +33,48 @@ constexpr uint32_t kManagementTimeoutGraceMs = 1000; } // namespace +ParkedCommand::ParkedCommand(AddressSpaceManager& addrSpaceMgr, + void* owner, + uint64_t bufferHandle, + std::unique_ptr orb, + std::unique_ptr pageTable) noexcept + : addrSpaceMgr_(&addrSpaceMgr) + , owner_(owner) + , bufferHandle_(bufferHandle) + , orb_(std::move(orb)) + , pageTable_(std::move(pageTable)) {} + +ParkedCommand::ParkedCommand(ParkedCommand&& other) noexcept + : addrSpaceMgr_(other.addrSpaceMgr_) + , owner_(other.owner_) + , bufferHandle_(std::exchange(other.bufferHandle_, 0)) + , orb_(std::move(other.orb_)) + , pageTable_(std::move(other.pageTable_)) {} + +ParkedCommand& ParkedCommand::operator=(ParkedCommand&& other) noexcept { + if (this != &other) { + Release(); + addrSpaceMgr_ = other.addrSpaceMgr_; + owner_ = other.owner_; + bufferHandle_ = std::exchange(other.bufferHandle_, 0); + orb_ = std::move(other.orb_); + pageTable_ = std::move(other.pageTable_); + } + return *this; +} + +ParkedCommand::~ParkedCommand() { + Release(); +} + +void ParkedCommand::Release() noexcept { + orb_.reset(); + pageTable_.reset(); + if (bufferHandle_ != 0) { + addrSpaceMgr_->DeallocateAddressRange(owner_, std::exchange(bufferHandle_, 0)); + } +} + CommandExecutor::CommandExecutor(Async::IFireWireBus& bus, Async::IFireWireBusInfo& busInfo, AddressSpaceManager& addrSpaceMgr, @@ -50,7 +92,7 @@ CommandExecutor::CommandExecutor(Async::IFireWireBus& bus, CommandExecutor::~CommandExecutor() { lifetimeToken_.reset(); - if (commandInFlight_ && resultCallback_) { + if (phase_ == CommandPhase::Active && resultCallback_) { FailActiveCommand(static_cast(kIOReturnAborted), Wire::SBPStatus::kRequestAborted); } @@ -84,7 +126,7 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, if (request.cdb.empty()) { return false; } - if (session_.State() != LoginState::LoggedIn || commandInFlight_) { + if (session_.State() != LoginState::LoggedIn || phase_ != CommandPhase::Idle) { return false; } if (request.transferLength > 0 && request.direction == SCSI::DataDirection::None) { @@ -93,7 +135,7 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, if (request.direction == SCSI::DataDirection::FromTarget && !request.outgoingPayload.empty()) { return false; } - if (request.direction == SCSI::DataDirection::ToTarget && + if (request.direction == SCSI::DataDirection::ToTarget && request.dataIOVA == 0 && request.outgoingPayload.size() != request.transferLength) { return false; } @@ -103,11 +145,18 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, return false; } + // The target moves the data itself through the OHCI physical response unit; + // LoginSession admitted its node when the login completed. A caller that + // owns a DMA mapping (the HBA's task buffer) names it directly; a caller + // without one (the user client) gets a bounce buffer in the same space. uint64_t bufferHandle = 0; AddressSpaceManager::AddressRangeMeta bufferMeta{}; - if (request.transferLength > 0) { - const kern_return_t kr = addrSpaceMgr_.AllocateAddressRangeAuto( - owner_, 0xFFFF, request.transferLength, &bufferHandle, &bufferMeta); + const bool inPlace = request.dataIOVA != 0; + if (inPlace) { + bufferMeta.address = request.dataIOVA; + } else if (request.transferLength > 0) { + const kern_return_t kr = addrSpaceMgr_.AllocatePhysicalRange( + owner_, request.transferLength, &bufferHandle, &bufferMeta); if (kr != kIOReturnSuccess) { ASFW_LOG(Async, "CommandExecutor: failed to allocate command buffer: 0x%08x", kr); return false; @@ -116,7 +165,7 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, std::unique_ptr pageTable; if (request.transferLength > 0) { - if (request.direction == SCSI::DataDirection::ToTarget) { + if (!inPlace && request.direction == SCSI::DataDirection::ToTarget) { const kern_return_t writeKr = addrSpaceMgr_.WriteLocalData( owner_, bufferHandle, 0, std::span{request.outgoingPayload.data(), @@ -156,11 +205,10 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, if (weak.expired()) { return; } - if (!commandInFlight_ || commandORB_.get() != submittedORB) { + if (phase_ != CommandPhase::Active || commandORB_.get() != submittedORB) { return; } - commandInFlight_ = false; commandReady_ = true; lastCompletedCommandOpcode_ = activeCommandOpcode_; activeCommandOpcode_.reset(); @@ -169,8 +217,13 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, result.transportStatus = transportStatus; result.sbpStatus = sbpStatus; - if (transportStatus == 0 && sbpStatus == Wire::SBPStatus::kNoAdditionalInfo && - activeCommandRequest_.has_value() && + const bool completed = transportStatus == 0 && + sbpStatus == Wire::SBPStatus::kNoAdditionalInfo && + activeCommandRequest_.has_value(); + if (completed && activeCommandRequest_->dataIOVA != 0) { + // SBP-2 status carries no residual; the whole transfer is reported. + result.inPlaceBytes = activeCommandRequest_->transferLength; + } else if (completed && activeCommandRequest_->direction == SCSI::DataDirection::FromTarget && activeCommandRequest_->transferLength > 0 && commandBufferHandle_ != 0) { std::vector payload; @@ -219,29 +272,29 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, NotifyResultCallback(); return; } - // Transport failure → agent gets reset before retry. - CleanupCommandResources(); - // ORB timeout means the target may be wedged beyond what AGENT_RESET - // revives (fetches ORBs, never executes). Apple's transport resets the - // LUN before completing the task - // (IOFireWireSerialBusProtocolTransport.cpp:1554), so the initiator's - // retry runs against a freshly reset execution engine. Delivery of the - // failed result is deferred until the LUN reset completes. + // Transport failure: no status block, so the target may still be + // moving data into the buffer. SBP-2 §10.4 reuses the memory only once + // a task-management status says the target stopped, so the ORB, page + // table and buffer stay mapped through the LUN reset. Apple's transport + // resets the LUN before completing the task + // (IOFireWireSerialBusProtocolTransport.cpp:1554); delivery of the + // failed result waits for that too. + phase_ = CommandPhase::Recovering; + session_.ClearCommandTracking(); + recoveryResetInFlight_ = true; if (SubmitTaskManagement(SBP2ManagementORB::Function::LogicalUnitReset, [this]() { - // lastError_ carries the management ORB outcome (set by - // the mgmt completion before onComplete). Nonzero means - // the target never fetched the LUN-reset ORB — its fetch - // engine is dead and no management function can reach it. - // Apple's transport stops here and just fails the task - // (IOFireWireSerialBusProtocolTransport.cpp:2030 ignores - // the LUN-reset status), leaving the device wedged until - // power cycle; its user-client path instead resets the - // bus (IOFireWireSBP2UserClient.cpp:427). A target - // whose fetch engine is this dead (observed on the - // LS-9000, but not a model-specific behavior) is only - // recoverable via a bus reset, so escalate — reconnect/ - // re-login runs via the normal reset flow. + recoveryResetInFlight_ = false; + // lastError_ carries the management ORB + // outcome. Nonzero means the target never + // fetched the LUN-reset ORB — its fetch + // engine is dead and no management + // function can reach it. Apple's user + // client resets the bus for that + // (IOFireWireSBP2UserClient.cpp:427), and + // only the reset ends the target's claim + // on the buffer: it stays mapped until + // OnBusReset. if (lastError_ != 0) { if (busResetRequester_) { ASFW_LOG(Async, @@ -249,15 +302,28 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, "escalating to bus reset", lastError_); busResetRequester_(); } - NotifyResultCallback(); + // A caller-owned buffer is reused once + // the result is delivered, so an + // in-place result waits for the reset + // (OnBusReset flushes it); a bounce + // buffer is ours to keep. + if (!commandInPlace_) { + NotifyResultCallback(); + } return; } - // A target may leave its fetch agent dead after a LUN - // reset (status dead bit, seen on a Hasselblad H camera), - // and a dead agent ignores ORB_POINTER. Apple resets the - // fetch agent from the LUN-reset completion and completes - // the task only after that - // (IOFireWireSerialBusProtocolTransport.cpp LunResetComplete). + // LUN reset status: every task is gone and + // the buffer is ours again (§10.4.3). + CleanupCommandResources(); + // A target may leave its fetch agent dead + // after a LUN reset (status dead bit, seen + // on a Hasselblad H camera). Apple resets + // the agent from the LUN-reset completion + // and completes the task only after that + // (IOFireWireSerialBusProtocolTransport.cpp + // LunResetComplete). AGENT_RESET has no + // status (§6.4.2); its outcome changes + // nothing here. const std::weak_ptr weakSelf = lifetimeToken_; session_.ResetFetchAgentNoWait([this, weakSelf]() { if (weakSelf.expired()) { @@ -269,15 +335,29 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, ASFW_LOG(Async, "CommandExecutor: transport failure — LUN reset before completing task"); return; } + recoveryResetInFlight_ = false; + // No LUN reset could be issued: the session is no longer LoggedIn (a + // reset cleared the filter and OnBusReset owns the rest) or a client + // task-management ORB is in flight, whose status is the stop point. A + // caller-owned buffer waits for that status (the management completion + // below frees and delivers); a bounce buffer is ours, so match Linux's + // abort (sbp2.c:1550-1551): release and complete. + if (commandInPlace_ && session_.State() == LoginState::LoggedIn) { + ASFW_LOG(Async, "CommandExecutor: transport failure, holding in-place result " + "for the task-management status"); + return; + } + CleanupCommandResources(); NotifyResultCallback(); }); - commandInFlight_ = true; + phase_ = CommandPhase::Active; commandReady_ = false; pendingCommandResult_.reset(); activeCommandRequest_ = request; activeCommandOpcode_ = request.cdb.front(); commandBufferHandle_ = bufferHandle; + commandInPlace_ = inPlace; commandORB_ = std::move(orb); commandPageTable_ = std::move(pageTable); resultCallback_ = std::move(callback); @@ -287,14 +367,13 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, } // SubmitORB returned false. Two distinct cases, told apart by whether the - // ORB completion callback ran synchronously during the call — it resets - // commandORB_ via CleanupCommandResources when it does. - if (commandORB_.get() == submittedORB) { + // ORB completion callback ran synchronously during the call — it leaves + // Active when it does. + if (phase_ == CommandPhase::Active && commandORB_.get() == submittedORB) { // Completion did NOT run: a genuine synchronous rejection (e.g. the // session rejected on state before touching the fetch agent). Roll back // and report false; the callback is dropped unfired, so the caller // completes the task itself, exactly once. - commandInFlight_ = false; commandReady_ = false; pendingCommandResult_.reset(); activeCommandRequest_.reset(); @@ -304,12 +383,12 @@ bool CommandExecutor::SubmitCommand(const SCSI::CommandRequest& request, return false; } - // commandORB_ was reset, so FetchAgent::AppendImmediate fired the ORB - // completion synchronously (WriteBlock failed on submit). The result was - // already delivered, or deferred behind a LUN-reset ORB — either way the - // executor now owns resultCallback_. Report success so the caller does NOT - // fire its own fallback completion: doing so would complete the SCSI task - // twice (double ParallelTaskCompletion + OSAction/controller over-release). + // FetchAgent::AppendImmediate fired the ORB completion synchronously + // (WriteBlock failed on submit). The result was already delivered, or + // deferred behind a LUN-reset ORB — either way the executor now owns + // resultCallback_. Report success so the caller does NOT fire its own + // fallback completion: doing so would complete the SCSI task twice (double + // ParallelTaskCompletion + OSAction/controller over-release). return true; } @@ -329,11 +408,7 @@ std::optional CommandExecutor::GetInquiryResult() { lastCompletedCommandOpcode_ != kInquiryOpcode) { return std::nullopt; } - SCSI::CommandResult result = std::move(*pendingCommandResult_); - pendingCommandResult_.reset(); - lastCompletedCommandOpcode_.reset(); - commandReady_ = false; - return result; + return GetCommandResult(); } bool CommandExecutor::SubmitTaskManagement(SBP2ManagementORB::Function function, @@ -373,19 +448,24 @@ bool CommandExecutor::SubmitTaskManagement(SBP2ManagementORB::Function function, return; } lastError_ = static_cast(status); - if (status == 0 && IsSupportedTaskManagementFunction(function)) { + const bool ownsCommand = phase_ == CommandPhase::Active || + (phase_ == CommandPhase::Recovering && !recoveryResetInFlight_); + if (status == 0 && IsSupportedTaskManagementFunction(function) && ownsCommand) { // A push-mode command wiped by task management must still complete - // (the bridge holds an open SCSI task on it). - if (commandInFlight_ && resultCallback_) { + // (the bridge holds an open SCSI task on it); a result held back by + // the timeout path goes out now. The status says the target dropped + // the task set, so its buffer is free (§10.4). + const bool pushMode = static_cast(resultCallback_); + if (pushMode) { FailActiveCommand(static_cast(kIOReturnAborted), Wire::SBPStatus::kRequestAborted); } CleanupCommandResources(); activeCommandRequest_.reset(); activeCommandOpcode_.reset(); - // Don't wipe a pending result awaiting deferred delivery (the - // timeout→LUN-reset escalation delivers it via onComplete below). - if (!resultCallback_) { + if (pushMode) { + NotifyResultCallback(); + } else { pendingCommandResult_.reset(); lastCompletedCommandOpcode_.reset(); commandReady_ = false; @@ -406,29 +486,43 @@ bool CommandExecutor::SubmitTaskManagement(SBP2ManagementORB::Function function, } void CommandExecutor::OnBusReset() { + // The reset ends the target's claim on everything (its node ID and the + // physical filter are gone with the generation), as it does for Linux + // (sbp2.c complete_command_orb: unmap on a NULL status). CleanupManagementResources(); - if (commandInFlight_ || commandORB_) { + if (phase_ == CommandPhase::Active) { FailActiveCommand(static_cast(kIOReturnAborted), Wire::SBPStatus::kRequestAborted); } + CleanupCommandResources(); // A result deferred behind a management ORB (the timeout→LUN-reset - // escalation) clears commandInFlight_/commandORB_ before deferring delivery - // to that ORB's onComplete. CleanupManagementResources above dropped the - // ORB without firing onComplete, so the guard misses it — flush any - // still-armed result now. NotifyResultCallback moves the callback out, so - // this is a no-op when FailActiveCommand already fired it. + // escalation) left Active before deferring delivery to that ORB's + // onComplete. CleanupManagementResources above dropped the ORB without + // firing onComplete, so the guard misses it — flush any still-armed result + // now. NotifyResultCallback moves the callback out, so this is a no-op when + // FailActiveCommand already fired it. if (resultCallback_) { NotifyResultCallback(); } } -void CommandExecutor::Cleanup() { +std::optional CommandExecutor::Cleanup() { // Release-time teardown: a pending push-mode command must complete (aborted) // so its task is not leaked. May run under the registry lock — the bridge's // callback defers any registry re-entry. - if (commandInFlight_ && resultCallback_) { + if (phase_ == CommandPhase::Active && resultCallback_) { FailActiveCommand(static_cast(kIOReturnAborted), Wire::SBPStatus::kRequestAborted); } + // The target still owns the mapping until its logout status (§8.4 aborts + // the task set): hand the resources to whoever waits for that instead of + // unmapping under a running transfer. + std::optional parked; + if (phase_ != CommandPhase::Idle) { + session_.ClearCommandTracking(); + parked.emplace(addrSpaceMgr_, owner_, std::exchange(commandBufferHandle_, 0), + std::move(commandORB_), std::move(commandPageTable_)); + phase_ = CommandPhase::Idle; + } CleanupCommandResources(); CleanupManagementResources(); // Flush a result deferred behind a (now-dropped) management ORB; see @@ -436,14 +530,14 @@ void CommandExecutor::Cleanup() { if (resultCallback_) { NotifyResultCallback(); } + return parked; } void CommandExecutor::FailActiveCommand(int transportStatus, uint8_t sbpStatus) noexcept { - if (!commandInFlight_) { + if (phase_ != CommandPhase::Active) { return; } - commandInFlight_ = false; commandReady_ = true; lastCompletedCommandOpcode_ = activeCommandOpcode_; activeCommandOpcode_.reset(); @@ -456,17 +550,12 @@ void CommandExecutor::FailActiveCommand(int transportStatus, uint8_t sbpStatus) : static_cast(result.transportStatus); pendingCommandResult_ = std::move(result); activeCommandRequest_.reset(); - commandPageTable_.reset(); - if (commandBufferHandle_ != 0) { - addrSpaceMgr_.DeallocateAddressRange(owner_, commandBufferHandle_); - commandBufferHandle_ = 0; - } if (commandORB_) { commandORB_->SetAppended(false); } - - CleanupCommandResources(); - NotifyResultCallback(); + // Resources and phase are left to the caller: it frees or parks them + // first, then delivers the result, so a callback that submits the next + // command inline finds the executor Idle. } void CommandExecutor::NotifyResultCallback() { @@ -486,15 +575,15 @@ void CommandExecutor::NotifyResultCallback() { void CommandExecutor::RetireCommand() { // Normal completion: the target has posted status and is done with the ORB - // (one ORB_POINTER write per command — no chain to keep alive). Free the - // data buffer, page table and ORB. + // and the buffer (one ORB_POINTER write per command — no chain to keep + // alive). Free the data buffer, page table and ORB. if (commandBufferHandle_ != 0) { addrSpaceMgr_.DeallocateAddressRange(owner_, commandBufferHandle_); commandBufferHandle_ = 0; } commandORB_.reset(); commandPageTable_.reset(); - commandInFlight_ = false; + phase_ = CommandPhase::Idle; } void CommandExecutor::CleanupCommandResources() { @@ -507,7 +596,7 @@ void CommandExecutor::CleanupCommandResources() { } commandORB_.reset(); commandPageTable_.reset(); - commandInFlight_ = false; + phase_ = CommandPhase::Idle; } void CommandExecutor::CleanupManagementResources() { diff --git a/ASFWDriver/Protocols/SBP2/Session/CommandExecutor.hpp b/ASFWDriver/Protocols/SBP2/Session/CommandExecutor.hpp index 5a18271a6..944406ce8 100644 --- a/ASFWDriver/Protocols/SBP2/Session/CommandExecutor.hpp +++ b/ASFWDriver/Protocols/SBP2/Session/CommandExecutor.hpp @@ -33,6 +33,7 @@ #include #include #include +#include namespace ASFW::Async { class IFireWireBus; @@ -41,6 +42,43 @@ class IFireWireBusInfo; namespace ASFW::Protocols::SBP2 { +// Where a command's ORB, page table and data buffer are in their life. The +// target moves the data itself (OHCI physical DMA) and may still be doing so +// until a status block or a completed task-management function says it has +// stopped (SBP-2 §10.4), so resources leave Active/Recovering only through +// those edges, a bus reset, or by being parked on the session's logout. +enum class CommandPhase : uint8_t { + Idle, // no command resources held + Active, // ORB submitted, status pending + Recovering, // transport failure; LUN reset in flight, resources retained +}; + +// A command's resources after its session was released while the target still +// owned them. Freed when dropped: after the logout status (the target has +// aborted the task set, §8.4), at bus reset, or with the registry. +class ParkedCommand { +public: + ParkedCommand(AddressSpaceManager& addrSpaceMgr, + void* owner, + uint64_t bufferHandle, + std::unique_ptr orb, + std::unique_ptr pageTable) noexcept; + ParkedCommand(ParkedCommand&& other) noexcept; + ParkedCommand& operator=(ParkedCommand&& other) noexcept; + ParkedCommand(const ParkedCommand&) = delete; + ParkedCommand& operator=(const ParkedCommand&) = delete; + ~ParkedCommand(); + +private: + void Release() noexcept; + + AddressSpaceManager* addrSpaceMgr_; + void* owner_; + uint64_t bufferHandle_; + std::unique_ptr orb_; + std::unique_ptr pageTable_; +}; + class CommandExecutor { public: CommandExecutor(Async::IFireWireBus& bus, @@ -96,8 +134,10 @@ class CommandExecutor { busResetRequester_ = std::move(requester); } - // Release-time cleanup: drop command + management resources. - void Cleanup(); + // Release-time cleanup: complete the active command (aborted) and drop + // management resources. Resources the target may still be using come back + // parked; the caller keeps them until the logout status or a bus reset. + [[nodiscard]] std::optional Cleanup(); [[nodiscard]] static bool IsSupportedTaskManagementFunction( SBP2ManagementORB::Function function) noexcept; @@ -123,10 +163,16 @@ class CommandExecutor { std::optional activeCommandOpcode_; std::optional lastCompletedCommandOpcode_; bool commandReady_{false}; - bool commandInFlight_{false}; + CommandPhase phase_{CommandPhase::Idle}; std::unique_ptr commandORB_; std::unique_ptr commandPageTable_; uint64_t commandBufferHandle_{0}; + // The data buffer is the caller's own mapping (request.dataIOVA); it may be + // reused the moment the result is delivered. + bool commandInPlace_{false}; + // The timeout recovery's own LUN reset is in flight (its completion owns + // the retained resources); a client task-management ORB is not. + bool recoveryResetInFlight_{false}; ResultCallback resultCallback_; std::unique_ptr managementORB_; diff --git a/ASFWDriver/Protocols/SBP2/Session/LoginSession.cpp b/ASFWDriver/Protocols/SBP2/Session/LoginSession.cpp index a0a963f36..c323b5928 100644 --- a/ASFWDriver/Protocols/SBP2/Session/LoginSession.cpp +++ b/ASFWDriver/Protocols/SBP2/Session/LoginSession.cpp @@ -828,6 +828,9 @@ void LoginSession::CompleteLoginFromStatusBlock(const Wire::StatusBlock& block, .nodeID = loginNodeID_}}; loginRetryCount_ = 0; + if (!EnablePhysicalDma()) { + return; + } SetState(LoginState::LoggedIn); BindFetchAgent(); @@ -865,6 +868,9 @@ void LoginSession::CompleteReconnectFromStatusBlock(const Wire::StatusBlock& blo return; } + if (!EnablePhysicalDma()) { + return; + } SetState(LoginState::LoggedIn); BindFetchAgent(); ASFW_LOG_SBP2( "LoginSession: reconnect successful — loginID=%u", loginID_.load()); @@ -887,6 +893,24 @@ void LoginSession::CompleteReconnectFromStatusBlock(const Wire::StatusBlock& blo } } +// Command data moves by physical DMA, so a login is usable only once the OHCI +// admits the target's node for this generation; Linux enables at login +// (sbp2.c:1146) and reconnect (sbp2.c:1198) the same way. A stale generation +// means a reset is already in flight: wait for rediscovery like any reset. +bool LoginSession::EnablePhysicalDma() noexcept { + if (!physicalDma_) { + return true; + } + const auto enabled = physicalDma_(loginNodeID_, loginGeneration_.load()); + if (enabled) { + return true; + } + ASFW_LOG_SBP2("LoginSession: physical DMA not enabled for node=0x%04x gen=%u (err=%u), suspending", + loginNodeID_, loginGeneration_.load(), static_cast(enabled.error())); + SetState(LoginState::Suspended); + return false; +} + void LoginSession::CompleteLogoutFromStatusBlock(const Wire::StatusBlock& /*block*/, uint32_t /*length*/) noexcept { const uint16_t oldLoginID = loginID_; diff --git a/ASFWDriver/Protocols/SBP2/Session/LoginSession.hpp b/ASFWDriver/Protocols/SBP2/Session/LoginSession.hpp index 93f9975de..ff026bb80 100644 --- a/ASFWDriver/Protocols/SBP2/Session/LoginSession.hpp +++ b/ASFWDriver/Protocols/SBP2/Session/LoginSession.hpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -123,6 +124,9 @@ class LoginSession : public std::enable_shared_from_this { using LoginCallback = std::function; using LogoutCallback = std::function; using StatusCallback = std::function; + // Admits the target's node to OHCI physical DMA for one generation. + using PhysicalDmaEnabler = std::function( + uint16_t node, uint16_t generation)>; LoginSession(Async::IFireWireBus& bus, Async::IFireWireBusInfo& busInfo, @@ -141,6 +145,7 @@ class LoginSession : public std::enable_shared_from_this { void SetLoginCallback(LoginCallback cb) noexcept { loginCallback_ = std::move(cb); } void SetLogoutCallback(LogoutCallback cb) noexcept { logoutCallback_ = std::move(cb); } void SetStatusCallback(StatusCallback cb) noexcept { statusCallback_ = std::move(cb); } + void SetPhysicalDmaEnabler(PhysicalDmaEnabler fn) noexcept { physicalDma_ = std::move(fn); } // ----------------------------------------------------------------------- // Session operations @@ -233,6 +238,7 @@ class LoginSession : public std::enable_shared_from_this { void OnStatusBlockRemoteWrite(uint32_t offset, std::span payload) noexcept; void ProcessStatusBlock(const Wire::StatusBlock& block, uint32_t length) noexcept; void CompleteLoginFromStatusBlock(const Wire::StatusBlock& block, uint32_t length) noexcept; + [[nodiscard]] bool EnablePhysicalDma() noexcept; void CompleteReconnectFromStatusBlock(const Wire::StatusBlock& block, uint32_t length) noexcept; void CompleteLogoutFromStatusBlock(const Wire::StatusBlock& block, uint32_t length) noexcept; @@ -265,6 +271,7 @@ class LoginSession : public std::enable_shared_from_this { // Configuration SBP2TargetInfo targetInfo_{}; + PhysicalDmaEnabler physicalDma_; bool configured_{false}; uint16_t maxPayloadSize_{4096}; // default, clipped by login response diff --git a/ASFWDriver/Protocols/SBP2/Session/SessionRegistry.cpp b/ASFWDriver/Protocols/SBP2/Session/SessionRegistry.cpp index 33d0b71f1..4897264b1 100644 --- a/ASFWDriver/Protocols/SBP2/Session/SessionRegistry.cpp +++ b/ASFWDriver/Protocols/SBP2/Session/SessionRegistry.cpp @@ -97,7 +97,7 @@ SessionRegistry::~SessionRegistry() { (void)record.session->Logout(); } if (record.executor) { - record.executor->Cleanup(); + (void)record.executor->Cleanup(); } } sessions_.clear(); @@ -164,6 +164,7 @@ std::expected SessionRegistry::CreateSession(void* owner, record.session = std::make_shared(bus_, busInfo_, addrSpaceMgr_, scheduler_); record.session->Configure(targetInfo); record.session->SetMaxPayloadSize(maxPayloadSize_); + record.session->SetPhysicalDmaEnabler(physicalDmaEnabler_); // record.lastError lives at a stable address (map node), so the executor may // bind a reference to it. record.executor = std::make_unique( @@ -289,8 +290,9 @@ bool SessionRegistry::ReleaseSession(void* owner, uint64_t handle) { } auto& record = it->second; + std::optional parked; if (record.executor) { - record.executor->Cleanup(); + parked = record.executor->Cleanup(); } const LoginState state = record.session ? record.session->State() : LoginState::Idle; @@ -298,11 +300,11 @@ bool SessionRegistry::ReleaseSession(void* owner, uint64_t handle) { (state == LoginState::LoggedIn || state == LoginState::Suspended)) { SetReleaseLogoutCallbackLocked(handle, record.guid, record.session); // Retire optimistically; un-retired below if Logout() is rejected. - RetireSessionLocked(record); + RetireSessionLocked(record, std::move(parked)); toLogout = record.session; } else if (record.session && state == LoginState::LoggingOut) { SetReleaseLogoutCallbackLocked(handle, record.guid, record.session); - RetireSessionLocked(record); + RetireSessionLocked(record, std::move(parked)); } sessions_.erase(it); } @@ -312,7 +314,7 @@ bool SessionRegistry::ReleaseSession(void* owner, uint64_t handle) { // when the device vanished during teardown). if (toLogout && !toLogout->Logout()) { IOLockGuard lock(lock_); - EraseRetiredSessionLocked(toLogout); + OnLogoutFailedLocked(toLogout, "rejected"); } return true; } @@ -329,20 +331,20 @@ void SessionRegistry::ReleaseOwner(void* owner) { auto& record = it->second; const LoginState state = record.session ? record.session->State() : LoginState::Idle; + std::optional parked; + if (record.executor) { + parked = record.executor->Cleanup(); + } if (record.session && (state == LoginState::LoggedIn || state == LoginState::Suspended)) { SetReleaseLogoutCallbackLocked(record.handle, record.guid, record.session); // Retire optimistically; un-retired below if Logout() is rejected. - RetireSessionLocked(record); + RetireSessionLocked(record, std::move(parked)); toLogout.push_back(record.session); } else if (record.session && state == LoginState::LoggingOut) { SetReleaseLogoutCallbackLocked(record.handle, record.guid, record.session); - RetireSessionLocked(record); - } - - if (record.executor) { - record.executor->Cleanup(); + RetireSessionLocked(record, std::move(parked)); } it = sessions_.erase(it); } @@ -352,11 +354,26 @@ void SessionRegistry::ReleaseOwner(void* owner) { for (auto& session : toLogout) { if (!session->Logout()) { IOLockGuard lock(lock_); - EraseRetiredSessionLocked(session); + OnLogoutFailedLocked(session, "rejected"); } } } +void SessionRegistry::SetPhysicalDmaEnabler(LoginSession::PhysicalDmaEnabler enabler) { + IOLockGuard lock(lock_); + physicalDmaEnabler_ = std::move(enabler); + for (auto& [handle, record] : sessions_) { + if (record.session) { + record.session->SetPhysicalDmaEnabler(physicalDmaEnabler_); + } + } +} + +void SessionRegistry::PrepareForTeardown() { + IOLockGuard lock(lock_); + tearingDown_ = true; +} + void SessionRegistry::SetBusResetRequester(std::function requester) { IOLockGuard lock(lock_); busResetRequester_ = std::move(requester); @@ -401,6 +418,44 @@ void SessionRegistry::OnBusReset(uint16_t newGeneration) { record.executor->OnBusReset(); } } + // The reset ends the target's claim on a parked command's buffer. An entry + // kept only for that (its logout had failed) goes with it; one still + // logging out is erased by its own logout callback. + for (auto& retired : retiringSessions_) { + retired.parked.reset(); + } + std::erase_if(retiringSessions_, [](const RetiringSession& retired) { + return retired.session && retired.session->State() != LoginState::LoggingOut; + }); +} + +// A logout that was rejected or never answered leaves the target's claim on a +// parked command unresolved; SBP-2 gives no other way to end it than a bus +// reset (main's policy for an unreachable target), after which OnBusReset +// frees the command. Without a parked command there is nothing to protect and +// the entry is simply dropped. +void SessionRegistry::OnLogoutFailedLocked(const std::shared_ptr& session, + const char* reason) { + const auto it = std::find_if( + retiringSessions_.begin(), retiringSessions_.end(), + [&session](const RetiringSession& retired) { return retired.session == session; }); + if (it == retiringSessions_.end()) { + return; + } + if (!it->parked) { + retiringSessions_.erase(it); + return; + } + if (tearingDown_) { + // The teardown already shut the physical response unit before it + // released the sessions; the parked command goes with the registry. + return; + } + ASFW_LOG(Async, "SessionRegistry: logout %{public}s with a command outstanding — " + "requesting bus reset to free it", reason); + if (busResetRequester_) { + busResetRequester_(); + } } void SessionRegistry::RefreshTargets(Discovery::Generation gen) { @@ -527,7 +582,8 @@ bool SessionRegistry::HasSessionForTargetLocked(uint64_t guid, uint32_t romOffse }); } -void SessionRegistry::RetireSessionLocked(const SessionRecord& record) { +void SessionRegistry::RetireSessionLocked(const SessionRecord& record, + std::optional parked) { if (record.session == nullptr) { return; } @@ -539,7 +595,10 @@ void SessionRegistry::RetireSessionLocked(const SessionRecord& record) { .guid = record.guid, .romOffset = record.romOffset, .session = record.session, + .parked = std::move(parked), }); + } else if (!it->parked) { + it->parked = std::move(parked); } } @@ -566,7 +625,7 @@ void SessionRegistry::SetReleaseLogoutCallbackLocked( std::weak_ptr weakSession = session; std::weak_ptr alive = lifetimeToken_; session->SetLogoutCallback( - [this, handle, guid, weakSession, alive](const LogoutCompleteParams&) { + [this, handle, guid, weakSession, alive](const LogoutCompleteParams& params) { if (alive.expired()) { return; // registry destroyed; session outlived it } @@ -577,12 +636,16 @@ void SessionRegistry::SetReleaseLogoutCallbackLocked( if (it != sessions_.end() && (completedSession == nullptr || it->second.session == completedSession)) { if (it->second.executor) { - it->second.executor->Cleanup(); + (void)it->second.executor->Cleanup(); } sessions_.erase(it); } - EraseRetiredSessionLocked(completedSession); + if (params.status == 0 || completedSession == nullptr) { + EraseRetiredSessionLocked(completedSession); + } else { + OnLogoutFailedLocked(completedSession, params.status == -2 ? "timed out" : "failed"); + } EmitLoginStateLocked(guid, false); }); } diff --git a/ASFWDriver/Protocols/SBP2/Session/SessionRegistry.hpp b/ASFWDriver/Protocols/SBP2/Session/SessionRegistry.hpp index be48a0d14..b2ac1d4a8 100644 --- a/ASFWDriver/Protocols/SBP2/Session/SessionRegistry.hpp +++ b/ASFWDriver/Protocols/SBP2/Session/SessionRegistry.hpp @@ -108,6 +108,12 @@ class SessionRegistry { // CommandExecutor::SetBusResetRequester). Set once during driver wiring, // before any session exists. void SetBusResetRequester(std::function requester); + // Handed to every LoginSession (LoginSession::SetPhysicalDmaEnabler). + void SetPhysicalDmaEnabler(LoginSession::PhysicalDmaEnabler enabler); + // Driver teardown is about to release every session with the physical + // response unit already shut: a failed logout then needs no bus reset, and + // a parked command is freed with the registry. + void PrepareForTeardown(); // Push channel for login state. Fires observer(guid, true) when a session // reaches LoggedIn (fresh login or reconnect) and observer(guid, false) on @@ -130,8 +136,10 @@ class SessionRegistry { std::shared_ptr ResolveUnit(uint64_t guid, uint32_t romOffset) const; [[nodiscard]] bool HasSessionForTargetLocked(uint64_t guid, uint32_t romOffset) const; - void RetireSessionLocked(const SessionRecord& record); + void RetireSessionLocked(const SessionRecord& record, + std::optional parked = std::nullopt); void EraseRetiredSessionLocked(const std::shared_ptr& session); + void OnLogoutFailedLocked(const std::shared_ptr& session, const char* reason); void SetReleaseLogoutCallbackLocked(uint64_t handle, uint64_t guid, const std::shared_ptr& session); @@ -150,12 +158,15 @@ class SessionRegistry { IOLock* lock_{nullptr}; std::function busResetRequester_; + LoginSession::PhysicalDmaEnabler physicalDmaEnabler_; std::function loginStateObserver_; std::map sessions_; struct RetiringSession { uint64_t guid{0}; uint32_t romOffset{0}; std::shared_ptr session; + // A command the target still owned at release; freed with the entry. + std::optional parked; }; // Hidden from registry clients, but retained until async logout finishes/times out. std::vector retiringSessions_; @@ -165,6 +176,7 @@ class SessionRegistry { // they are dispatched to workQueue_ — a management write can fail INLINE // while the caller still holds lock_, and os_unfair_lock recursion aborts). std::shared_ptr lifetimeToken_{std::make_shared(0)}; + bool tearingDown_{false}; }; } // namespace ASFW::Protocols::SBP2 diff --git a/ASFWDriver/SCSIController/ASFWSCSIController.cpp b/ASFWDriver/SCSIController/ASFWSCSIController.cpp index c2b024f24..dbc5901e2 100644 --- a/ASFWDriver/SCSIController/ASFWSCSIController.cpp +++ b/ASFWDriver/SCSIController/ASFWSCSIController.cpp @@ -297,10 +297,9 @@ kern_return_t IMPL(ASFWSCSIController, UserInitializeController) // and rejects any task larger than a small threshold before it reaches the // dext: VueScan's SEND LUT (32 KB out) and image READ (~510 KB in) failed // instantly while everything ≤197 B went through (LS-9000 traffic, but the - // constraint applies to any scanner in this class). The dext never touches - // fBufferIOVMAddr segments (data is copied via UserGetDataBuffer), so the - // values only need to be permissive and consistent with - // UserGetDMASpecification (Apple's ~30 MB SBP-2 ceiling, 64-bit, 4-byte alignment). + // constraint applies to any scanner in this class). The values need to be + // permissive and consistent with UserGetDMASpecification (Apple's ~30 MB + // SBP-2 ceiling, 4-byte alignment). OSDictionary* constraints = OSDictionary::withCapacity(7); if (constraints == nullptr) { return kIOReturnNoMemory; @@ -440,14 +439,17 @@ kern_return_t IMPL(ASFWSCSIController, UserInitializeTargetForID) kern_return_t IMPL(ASFWSCSIController, UserGetDMASpecification) { // This spec drives the kernel shim's IODMACommand for the task buffer it - // maps before UserProcessParallelTask (fBufferIOVMAddr). Big-endian segment - // output byte-swaps those segment addresses — with kDMAOutputSegmentBig32 - // the first data-carrying command (INQUIRY) died in kernel DMA prep and - // never reached the dext ("InitializeDeviceSupport error"). Wire endianness - // is irrelevant here; SBP-2 data movement uses our own DMA path anyway. + // maps before UserProcessParallelTask (fBufferIOVMAddr). That mapping is + // what the SBP-2 target reads and writes through the OHCI physical response + // unit, which serves the first 4 GiB (PhysicalUpperBound), so the address + // width is 32 bits: the kernel bounces anything that would land higher. + // Big-endian segment output byte-swaps the segment addresses — with + // kDMAOutputSegmentBig32 the first data-carrying command (INQUIRY) died in + // kernel DMA prep and never reached the dext ("InitializeDeviceSupport + // error"). *maxTransferSize = kMaxTransferPerTask; *alignment = 4; - *numAddressBits = 64; + *numAddressBits = 32; *segmentType = kDMAOutputSegmentHost64; return kIOReturnSuccess; } @@ -577,29 +579,20 @@ kern_return_t IMPL(ASFWSCSIController, UserProcessParallelTask) ? parallelRequest.fTimeoutInMilliSec : kDefaultTaskTimeoutMs; - // Resolve the task's data buffer HERE — the SDK contract is that - // UserGetDataBuffer is called inside UserProcessParallelTask. The mapping - // stays valid until the task completes, so the completion lambda may write - // through the captured address for data-IN. - // - // Like every DriverKit `OSObject**` out-param, the descriptor comes back - // +1 retained and must be released by us. Leaking it cost one dext-side - // object (and its mach ports) per data-carrying task; a long VueScan - // session (~57k tasks) exhausted the port space and the kernel killed the - // dext with EXC_RESOURCE/PORTS. Hold it until the task completes (the - // mapping must outlive the data-IN copy) and release it there. - IOAddressSegment dataSeg{}; - IOBufferMemoryDescriptor* buffer = nullptr; + // The kernel mapped the task's buffer for our PCI device before this call + // (fBufferIOVMAddr, one segment, below 4 GiB per UserGetDMASpecification), + // and keeps it until ParallelTaskCompletion. The SBP-2 target reads and + // writes that mapping directly, as Apple's SBP-2 does + // (IOFireWireSBP2ORB.cpp prepareBufferAddressSpace). UserGetDataBuffer is + // not called: per the SDK it invalidates this mapping. The mapping covers + // fRequestedTransferCount bytes, which is exactly what request.transferLength + // carries and what the target is told; there is no second length to check + // it against without that call. if (request.transferLength > 0) { - kern_return_t kr = UserGetDataBuffer(parallelRequest.fTargetID, - parallelRequest.fControllerTaskIdentifier, - &buffer); - if (kr != kIOReturnSuccess || buffer == nullptr || - buffer->GetAddressRange(&dataSeg) != kIOReturnSuccess || dataSeg.address == 0 || - dataSeg.length < request.transferLength) { - ASFW_LOG(Controller, "[SCSIHBA] task data buffer unavailable (kr=0x%x len=%llu/%u)", - kr, dataSeg.length, request.transferLength); - OSSafeReleaseNULL(buffer); + if (parallelRequest.fBufferIOVMAddr == 0 || + parallelRequest.fBufferIOVMAddr + request.transferLength > (uint64_t{1} << 32)) { + ASFW_LOG(Controller, "[SCSIHBA] task buffer not DMA-mapped below 4 GiB (iova=0x%llx len=%u)", + (uint64_t)parallelRequest.fBufferIOVMAddr, request.transferLength); resp.fServiceResponse = kSCSIServiceResponse_SERVICE_DELIVERY_OR_TARGET_FAILURE; ParallelTaskCompletion(completion, resp); if (response != nullptr) { @@ -607,15 +600,11 @@ kern_return_t IMPL(ASFWSCSIController, UserProcessParallelTask) } return kIOReturnSuccess; } - if (request.direction == SBP2::SCSI::DataDirection::ToTarget) { - const auto* src = reinterpret_cast(dataSeg.address); - request.outgoingPayload.assign(src, src + request.transferLength); - } + request.dataIOVA = parallelRequest.fBufferIOVMAddr; } // The completion fires later, from the FireWire driver's work queue — keep - // the OSAction and this service alive across the async gap. `buffer` - // (already +1) rides along and is released in the same place. + // the OSAction and this service alive across the async gap. completion->retain(); this->retain(); @@ -623,24 +612,19 @@ kern_return_t IMPL(ASFWSCSIController, UserProcessParallelTask) const uint64_t taskID = parallelRequest.fControllerTaskIdentifier; const bool dataIn = (request.direction == SBP2::SCSI::DataDirection::FromTarget); const uint32_t requestedLength = request.transferLength; - const uint64_t dataAddress = dataSeg.address; - const uint64_t dataLength = dataSeg.length; bridge->SubmitTask(std::move(request), - [this, completion, buffer, targetID, taskID, dataIn, requestedLength, dataAddress, - dataLength, opcode](const SBP2::SCSI::CommandResult& result) { + [this, completion, targetID, taskID, dataIn, requestedLength, + opcode](const SBP2::SCSI::CommandResult& result) { SCSIUserParallelResponse asyncResp{}; asyncResp.version = kScsiUserParallelTaskResponseCurrentVersion1; asyncResp.fTargetID = targetID; asyncResp.fControllerTaskIdentifier = taskID; FillResponseFromResult(asyncResp, result); - if (dataIn && asyncResp.fServiceResponse == kSCSIServiceResponse_TASK_COMPLETE && - !result.payload.empty() && dataAddress != 0) { - uint64_t n = std::min(result.payload.size(), requestedLength); - n = std::min(n, dataLength); - memcpy(reinterpret_cast(dataAddress), result.payload.data(), n); - asyncResp.fBytesTransferred = n; + if (dataIn && asyncResp.fServiceResponse == kSCSIServiceResponse_TASK_COMPLETE) { + asyncResp.fBytesTransferred = + std::min(result.inPlaceBytes, requestedLength); } else if (!dataIn && asyncResp.fServiceResponse == kSCSIServiceResponse_TASK_COMPLETE && asyncResp.fCompletionStatus == kSCSITaskStatus_GOOD) { @@ -665,9 +649,6 @@ kern_return_t IMPL(ASFWSCSIController, UserProcessParallelTask) } ParallelTaskCompletion(completion, asyncResp); - if (buffer != nullptr) { - buffer->release(); - } completion->release(); this->release(); }); diff --git a/ASFWDriver/Service/DriverContext.cpp b/ASFWDriver/Service/DriverContext.cpp index bdfd72d65..c841e1b52 100644 --- a/ASFWDriver/Service/DriverContext.cpp +++ b/ASFWDriver/Service/DriverContext.cpp @@ -370,6 +370,17 @@ kern_return_t DriverWiring::EnsureSbp2Deps(ASFWDriver& service, ::ServiceContext } }); } + if (d.hardware) { + std::weak_ptr weakHardware = d.hardware; + d.sbp2SessionRegistry->SetPhysicalDmaEnabler( + [weakHardware](uint16_t node, uint16_t generation) + -> std::expected { + if (auto hardware = weakHardware.lock()) { + return hardware->EnablePhysicalRequests(node, generation); + } + return std::unexpected(PhysicalDmaError::HardwareGone); + }); + } ASFW_LOG(Controller, "[Controller] SBP2 SessionRegistry initialized"); } diff --git a/ASFWDriver/Service/LocalRequestWiring.cpp b/ASFWDriver/Service/LocalRequestWiring.cpp index 36ccefcc1..0cc5acb9e 100644 --- a/ASFWDriver/Service/LocalRequestWiring.cpp +++ b/ASFWDriver/Service/LocalRequestWiring.cpp @@ -19,6 +19,7 @@ #include "../Bus/IRM/LocalIRMResourceCSRHandler.hpp" #include "../Controller/ControllerCore.hpp" #include "../Hardware/IEEE1394.hpp" +#include "../Hardware/OHCIConstants.hpp" #include "../Logging/Logging.hpp" #include "../Protocols/AVC/FCPResponseRouter.hpp" #include "../Audio/Protocols/DICE/Core/DiceNotificationRouter.hpp" @@ -224,6 +225,8 @@ class FireworksEfcLocalHandler final : public ILocalAddressHandler { }; // --- SBP-2: dynamically allocated device address ranges ------------------------ +constexpr uint64_t kPhysicalResponseLimit = uint64_t{ASFW::Driver::kPhysicalUpperBound} << 16; + class Sbp2LocalHandler final : public ILocalAddressHandler { public: explicit Sbp2LocalHandler(ASFW::Protocols::SBP2::AddressSpaceManager* mgr) noexcept @@ -234,6 +237,14 @@ class Sbp2LocalHandler final : public ILocalAddressHandler { if (mgr_ == nullptr) { return LocalRequestResult::NotMine(); } + // Below PhysicalUpperBound the OHCI serves admitted nodes by DMA; a + // request that still reaches software came from a node without the + // filter bit or with a transaction type the physical unit does not + // take. Linux answers those with TYPE_ERROR (core-transaction.c + // handle_low_memory); so do we, instead of serving them from here. + if (ctx.destOffset < kPhysicalResponseLimit) { + return LocalRequestResult{.claimed = true, .rcode = ResponseCode::TypeError}; + } switch (ctx.tCode) { case AReq::kTcodeWriteQuad: case AReq::kTcodeWriteBlock: { diff --git a/documentation/sbp2-physical-dma.md b/documentation/sbp2-physical-dma.md new file mode 100644 index 000000000..67a572f06 --- /dev/null +++ b/documentation/sbp2-physical-dma.md @@ -0,0 +1,75 @@ +# SBP-2 command data over OHCI physical DMA + +Command data (the buffer a READ fills or a WRITE drains) is carried by the +OHCI physical response unit. The target reads and writes the buffer's own IOVA +directly; no software runs per packet. Apple (`IOFireWireSBP2ORB.cpp:1002-1041`) +and Linux (`sbp2.c` `sbp2_map_scatterlist`, `ohci.c` `ohci_enable_phys_dma`) +carry SBP-2 data the same way. ORBs, page tables, status FIFOs and the login +response stay in the software-served `0xFFFF:xxxx` window. + +For HBA tasks the buffer is the kernel's own: the SCSI shim maps the task +buffer for our PCI device before `UserProcessParallelTask` (`fBufferIOVMAddr`, +one segment, below 4 GiB because `UserGetDMASpecification` asks for 32-bit +addresses) and keeps it until `ParallelTaskCompletion`. `CommandRequest::dataIOVA` +hands that address to SBP-2 and nothing is copied. `UserGetDataBuffer` is never +called; per the SDK it invalidates the mapping. The raw user client has no +mapping of its own and gets a bounce buffer in the same address space. + +## Mechanism + +- `AddressSpaceManager::AllocatePhysicalRange` allocates a 32-bit IOVA bounce + buffer (cacheable mapping, no shadow copy) for callers without a mapping. +- `HardwareInterface::EnablePhysicalRequests(node, generation)` sets the node's + bit in `PhysicalRequestFilter` after checking that `SelfIDCount` still shows + that generation and no `busReset` is pending (`ohci.c:2615`). Nothing clears a + single node: the filter is cleared as a whole at the bus-reset interrupt and + at async teardown. `PhysicalUpperBound` is programmed once at controller init + (`ohci.c:2348`). +- `LoginSession` enables the target's node when a login or reconnect completes + (`sbp2.c:1146`, `:1198`). A stale generation at that point means a reset is in + flight; the session goes `Suspended` and reconnects after rediscovery. +- Requests below `PhysicalUpperBound` that still reach software are answered + `TYPE_ERROR` (`core-transaction.c` `handle_low_memory`): they came from a node + without the filter bit, or with a transaction type the physical unit does not + take. A transfer can therefore not silently fall back to the software path. + +## Buffer lifetime + +The target may DMA into the buffer until it says it has stopped (SBP-2 §10.4). +`CommandExecutor` tracks this as a phase (`Idle`, `Active`, `Recovering`) and +frees the ORB, page table and buffer only at: + +| Edge | Why the target is done | +|---|---| +| Status block for the ORB | the command completed | +| LUN-reset status after a transport failure | the task set is aborted (§10.4.3) | +| Bus reset | the node ID and the filter bit are gone with the generation | +| Logout status after the client released the session | logout aborts the task set (§8.4) | + +An HBA task's buffer is reused by the kernel at `ParallelTaskCompletion`, which +the bridge issues at release. The dext teardown path clears the physical filter +before the bridge shuts down (`ExecuteRuntimeTeardown`), so no target can reach +a completed task's buffer; the other release cause is a device that has left +the bus. + +A command released by its client is parked on the retiring session and freed +with it. If the logout is rejected or never answered, the target's claim is +unresolved and a bus reset is requested; the reset is then the edge that frees +the command, and the only reset the release path ever asks for. AGENT_RESET has +no status (§6.4.2); its outcome does not change the lifetime. A LUN reset that +the target never fetches keeps the existing escalation to a bus reset, which is +then the edge that frees the buffer. + +## Exposure + +A node with its filter bit set can read and write any address below +`PhysicalUpperBound` that the OHCI's IOMMU context maps: command buffers, but +also DMA rings, the Self-ID buffer, the Config ROM image and audio buffers. +Source node IDs are not authenticated. This is the same exposure as Apple's and +Linux's SBP-2 stacks, limited to nodes we logged in to, for the generations we +were logged in. + +## Hardware evidence + +- LS-9000 (software path → physical): no regression in scan, cancel, unplug. +- Hasselblad H4D-50MS: capture 22.9 → 48.0 MB/s (PR #213 test run). diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e9360eb2d..ffce10ddb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -116,6 +116,7 @@ endif() add_library(asfw_test_support STATIC "support/LoggingStubs.cpp" "support/HardwareInterfaceStub.cpp" + "${ASFW_DRIVER_DIR}/Hardware/PhysicalRequestFilter.cpp" "${ASFW_DRIVER_DIR}/Logging/LogRing.cpp" ) target_link_libraries(asfw_test_support PUBLIC asfw_test_common_interface) diff --git a/tests/common/AddressSpaceManagerTests.cpp b/tests/common/AddressSpaceManagerTests.cpp index 114b22c93..b568c62dd 100644 --- a/tests/common/AddressSpaceManagerTests.cpp +++ b/tests/common/AddressSpaceManagerTests.cpp @@ -314,3 +314,58 @@ TEST(AddressSpaceManagerTests, SetDebugLabelToleratesBadInputAndPreservesRange) EXPECT_EQ(0xDE, readback[0]); EXPECT_EQ(0xEF, readback[3]); } + +TEST(AddressSpaceManagerTests, PhysicalRangeLivesBelow4GBAndRoundTrips) { + ASFW::Protocols::SBP2::AddressSpaceManager manager(nullptr); + void* owner = reinterpret_cast(0x1); + + uint64_t handle = 0; + ASFW::Protocols::SBP2::AddressSpaceManager::AddressRangeMeta meta{}; + ASSERT_EQ(kIOReturnSuccess, manager.AllocatePhysicalRange(owner, 32, &handle, &meta)); + EXPECT_EQ(0u, meta.addressHi); + EXPECT_LT(meta.address, 1ULL << 32); + EXPECT_EQ(32u, meta.length); + + // A second buffer must not alias the first. + uint64_t handle2 = 0; + ASFW::Protocols::SBP2::AddressSpaceManager::AddressRangeMeta meta2{}; + ASSERT_EQ(kIOReturnSuccess, manager.AllocatePhysicalRange(owner, 32, &handle2, &meta2)); + EXPECT_NE(meta.address, meta2.address); + + // The CPU side reads what the device wrote, by handle and by address. + const std::array payload{0xDE, 0xAD, 0xBE, 0xEF}; + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, + manager.ApplyRemoteWrite(meta.address + 8, + std::span(payload.data(), payload.size()))); + std::vector readback; + ASSERT_EQ(kIOReturnSuccess, manager.ReadIncomingData(owner, handle, 8, 4, &readback)); + EXPECT_EQ(std::vector(payload.begin(), payload.end()), readback); + uint32_t quadlet = 0; + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, manager.ReadQuadlet(meta.address + 8, &quadlet)); + EXPECT_EQ(0xDEADBEEFu, OSSwapBigToHostInt32(quadlet)); + + EXPECT_EQ(kIOReturnSuccess, manager.DeallocateAddressRange(owner, handle)); + EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, + manager.ApplyRemoteWrite(meta.address + 8, + std::span(payload.data(), payload.size()))); + EXPECT_EQ(kIOReturnSuccess, manager.DeallocateAddressRange(owner, handle2)); +} + +TEST(AddressSpaceManagerTests, BackedPhysicalRangeIsReadThroughItsMapping) { + ASFW::Driver::HardwareInterface hardware; + ASFW::Protocols::SBP2::AddressSpaceManager manager(&hardware); + auto* owner = reinterpret_cast(0x42); + uint64_t handle = 0; + ASFW::Protocols::SBP2::AddressSpaceManager::AddressRangeMeta meta{}; + ASSERT_EQ(kIOReturnSuccess, manager.AllocatePhysicalRange(owner, 70000, &handle, &meta)); + EXPECT_EQ(0u, meta.addressHi); + const std::array bytes{1, 2, 3, 4}; + ASSERT_EQ(kIOReturnSuccess, manager.WriteLocalData(owner, handle, 69996, bytes)); + std::vector readback; + ASSERT_EQ(kIOReturnSuccess, manager.ReadIncomingData(owner, handle, 69996, 4, &readback)); + EXPECT_EQ(std::vector(bytes.begin(), bytes.end()), readback); + uint64_t overlapping = 0; + EXPECT_EQ(kIOReturnNoSpace, + manager.AllocateAddressRange(owner, 0, meta.addressLo, 4, &overlapping)); + EXPECT_EQ(kIOReturnSuccess, manager.DeallocateAddressRange(owner, handle)); +} diff --git a/tests/core/CMakeLists.txt b/tests/core/CMakeLists.txt index 107f4d4b7..b0dd9d20b 100644 --- a/tests/core/CMakeLists.txt +++ b/tests/core/CMakeLists.txt @@ -49,6 +49,10 @@ add_core_test(SelfIDSequenceTests SelfIDSequenceTests.cpp ) +add_core_test(PhysicalRequestFilterTests + PhysicalRequestFilterTests.cpp +) + # SelfID Capture Tests add_core_test(SelfIDCaptureTests SelfIDCaptureTests.cpp diff --git a/tests/core/ControllerCoreInterruptTests.cpp b/tests/core/ControllerCoreInterruptTests.cpp index 4fbc42977..31f849ad4 100644 --- a/tests/core/ControllerCoreInterruptTests.cpp +++ b/tests/core/ControllerCoreInterruptTests.cpp @@ -269,4 +269,14 @@ TEST(InterruptDrainTest, GivesUpOnAnEventThatNeverClears) { EXPECT_EQ(passes, kMaxInterruptPasses); } +TEST_F(ControllerCoreInterruptTest, BusResetClearsEveryPhysicalRequestFilter) { + hardware_->SetTestRegister(Register32::kSelfIDCount, kGeneration << 16); + ASSERT_TRUE(hardware_->EnablePhysicalRequests(5, kGeneration).has_value()); + ASSERT_TRUE(hardware_->EnablePhysicalRequests(40, kGeneration).has_value()); + hardware_->SetTestRegister(Register32::kIntEvent, IntEventBits::kBusReset); + (void)core_->HandleInterrupt(InterruptSnapshot{.intEvent = IntEventBits::kBusReset}); + EXPECT_EQ(0u, hardware_->GetTestRegister(Register32::kPhyReqFilterLoSet)); + EXPECT_EQ(0u, hardware_->GetTestRegister(Register32::kPhyReqFilterHiSet)); +} + } // namespace ASFW::Driver diff --git a/tests/core/HardwareInterfaceOrderTests.cpp b/tests/core/HardwareInterfaceOrderTests.cpp index 97a84cbd9..0ee824f58 100644 --- a/tests/core/HardwareInterfaceOrderTests.cpp +++ b/tests/core/HardwareInterfaceOrderTests.cpp @@ -481,3 +481,33 @@ TEST_F(HardwareInterfaceOrderTests, SetRootHoldOffTrueSetsRhbPreservingGap) { } } // namespace + +TEST_F(HardwareInterfaceOrderTests, EnablePhysicalRequestsChecksGenerationThenSetsAndFlushes) { + InSequence seq; + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kSelfIDCount), _)) + .WillOnce([](uint8_t, uint64_t, uint32_t* value) { *value = 7u << 16; }); + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kIntEvent), _)) + .WillOnce([](uint8_t, uint64_t, uint32_t* value) { *value = 0; }); + EXPECT_CALL(*mockDevice_, MemoryWrite32(0, uint64_t(Register32::kPhyReqFilterHiSet), 1u << 8)); + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kHCControl), _)); + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kSelfIDCount), _)) + .WillOnce([](uint8_t, uint64_t, uint32_t* value) { *value = 7u << 16; }); + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kIntEvent), _)) + .WillOnce([](uint8_t, uint64_t, uint32_t* value) { *value = 0; }); + EXPECT_TRUE(hardware_.EnablePhysicalRequests(40, 7).has_value()); +} + +TEST_F(HardwareInterfaceOrderTests, EnablePhysicalRequestsTakesTheBitBackIfAResetSlipsIn) { + InSequence seq; + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kSelfIDCount), _)) + .WillOnce([](uint8_t, uint64_t, uint32_t* value) { *value = 7u << 16; }); + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kIntEvent), _)) + .WillOnce([](uint8_t, uint64_t, uint32_t* value) { *value = 0; }); + EXPECT_CALL(*mockDevice_, MemoryWrite32(0, uint64_t(Register32::kPhyReqFilterLoSet), 1u << 5)); + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kHCControl), _)); + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kSelfIDCount), _)) + .WillOnce([](uint8_t, uint64_t, uint32_t* value) { *value = 8u << 16; }); + EXPECT_CALL(*mockDevice_, MemoryWrite32(0, uint64_t(Register32::kPhyReqFilterLoClear), 1u << 5)); + EXPECT_CALL(*mockDevice_, MemoryRead32(0, uint64_t(Register32::kHCControl), _)); + EXPECT_EQ(PhysicalDmaError::StaleGeneration, hardware_.EnablePhysicalRequests(5, 7).error()); +} diff --git a/tests/core/PhysicalRequestFilterTests.cpp b/tests/core/PhysicalRequestFilterTests.cpp new file mode 100644 index 000000000..8b43b7a78 --- /dev/null +++ b/tests/core/PhysicalRequestFilterTests.cpp @@ -0,0 +1,35 @@ +#include + +#include "ASFWDriver/Hardware/HardwareInterface.hpp" +#include "ASFWDriver/Hardware/RegisterMap.hpp" + +using namespace ASFW::Driver; + +TEST(PhysicalRequestFilterTests, EnableSetsTheNodeBitForTheCurrentGeneration) { + HardwareInterface hardware; + hardware.SetTestRegister(Register32::kSelfIDCount, 7u << 16); + for (const uint16_t node : {uint16_t{5}, uint16_t{31}, uint16_t{32}, uint16_t{0xFFC0 | 40}, uint16_t{62}}) { + ASSERT_TRUE(hardware.EnablePhysicalRequests(node, 7).has_value()) << node; + const uint16_t phy = node & 0x3Fu; + const auto reg = phy < 32 ? Register32::kPhyReqFilterLoSet : Register32::kPhyReqFilterHiSet; + EXPECT_NE(0u, hardware.GetTestRegister(reg) & (1u << (phy % 32))) << node; + } + hardware.ClearPhysicalRequestFilters(); + EXPECT_EQ(0u, hardware.GetTestRegister(Register32::kPhyReqFilterLoSet)); + EXPECT_EQ(0u, hardware.GetTestRegister(Register32::kPhyReqFilterHiSet)); +} + +TEST(PhysicalRequestFilterTests, RejectsStaleGenerationPendingResetBroadcastAndRemoteBus) { + HardwareInterface hardware; + hardware.SetTestRegister(Register32::kSelfIDCount, 7u << 16); + EXPECT_EQ(PhysicalDmaError::StaleGeneration, hardware.EnablePhysicalRequests(5, 6).error()); + EXPECT_EQ(PhysicalDmaError::InvalidNode, hardware.EnablePhysicalRequests(63, 7).error()); + EXPECT_EQ(PhysicalDmaError::InvalidNode, hardware.EnablePhysicalRequests(0xFFFF, 7).error()); + EXPECT_EQ(PhysicalDmaError::InvalidNode, hardware.EnablePhysicalRequests(0x0045, 7).error()); + hardware.SetTestRegister(Register32::kIntEvent, IntEventBits::kBusReset); + EXPECT_EQ(PhysicalDmaError::StaleGeneration, hardware.EnablePhysicalRequests(5, 7).error()); + EXPECT_EQ(0u, hardware.GetTestRegister(Register32::kPhyReqFilterLoSet)); + EXPECT_EQ(0u, hardware.GetTestRegister(Register32::kPhyReqFilterHiSet)); + hardware.Detach(); + EXPECT_EQ(PhysicalDmaError::HardwareGone, hardware.EnablePhysicalRequests(5, 7).error()); +} diff --git a/tests/protocols/LoginSessionTests.cpp b/tests/protocols/LoginSessionTests.cpp index 50b3c63e6..1a3d060cb 100644 --- a/tests/protocols/LoginSessionTests.cpp +++ b/tests/protocols/LoginSessionTests.cpp @@ -703,3 +703,83 @@ TEST(LoginSessionTests, } } // namespace + +// The target's node is admitted to physical DMA when the login completes and +// again, for the new generation and node, when a reconnect completes. +TEST(LoginSessionTests, PhysicalDmaIsEnabledOnLoginAndReconnect) { + SessionRig rig; + std::vector> enabled; + rig.session.SetPhysicalDmaEnabler( + [&](uint16_t node, uint16_t generation) -> std::expected { + enabled.emplace_back(node, generation); + return {}; + }); + + rig.LoginSuccessfully(); + ASSERT_EQ(1u, enabled.size()); + EXPECT_EQ(std::make_pair(kTargetNodeID, uint16_t{1}), enabled[0]); + + rig.bus.SetGeneration(ASFW::FW::Generation{2}); + rig.session.HandleBusReset(2); + auto refreshed = rig.session.TargetInfo(); + refreshed.targetNodeId = kTargetNodeID + 1; + rig.session.Configure(refreshed); + ASSERT_TRUE(rig.session.Reconnect()); + const size_t reconnectWriteIndex = rig.bus.WriteCount() - 1; + const uint64_t reconnectOrbAddress = + DecodeOrbAddressFromPayload(rig.bus.WriteAt(reconnectWriteIndex).data); + const uint64_t reconnectStatusAddress = + ReadORBAddress(rig.addressManager, reconnectOrbAddress, + offsetof(ReconnectORB, statusFIFOAddressHi), + offsetof(ReconnectORB, statusFIFOAddressLo)); + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + StatusBlock status{}; + status.sbpStatus = SBPStatus::kNoAdditionalInfo; + rig.addressManager.ApplyRemoteWrite( + reconnectStatusAddress, + std::span{reinterpret_cast(&status), sizeof(status)}); + ASSERT_EQ(LoginState::LoggedIn, rig.session.State()); + ASSERT_EQ(2u, enabled.size()); + EXPECT_EQ(std::make_pair(uint16_t(kTargetNodeID + 1), uint16_t{2}), enabled[1]); +} + +// A stale generation at login completion means a reset is in flight: the +// session waits for rediscovery instead of reporting a login it cannot use. +TEST(LoginSessionTests, PhysicalDmaFailureSuspendsInsteadOfLoggingIn) { + SessionRig rig; + rig.session.SetPhysicalDmaEnabler( + [](uint16_t, uint16_t) -> std::expected { + return std::unexpected(ASFW::Driver::PhysicalDmaError::StaleGeneration); + }); + int loginCallbacks = 0; + rig.session.SetLoginCallback([&](const ASFW::Protocols::SBP2::LoginCompleteParams&) { ++loginCallbacks; }); + + ASSERT_TRUE(rig.session.Login()); + const uint64_t loginOrbAddress = DecodeOrbAddressFromPayload(rig.bus.WriteAt(0).data); + const uint64_t loginResponseAddress = + ReadORBAddress(rig.addressManager, loginOrbAddress, + offsetof(LoginORB, loginResponseAddressHi), + offsetof(LoginORB, loginResponseAddressLo)); + const uint64_t statusAddress = + ReadORBAddress(rig.addressManager, loginOrbAddress, + offsetof(LoginORB, statusFIFOAddressHi), + offsetof(LoginORB, statusFIFOAddressLo)); + LoginResponse response{}; + response.length = OSSwapHostToBigInt16(LoginResponse::kSize); + response.loginID = OSSwapHostToBigInt16(0x0042); + response.commandBlockAgentAddressHi = OSSwapHostToBigInt32(0x0000'FFFFu); + response.commandBlockAgentAddressLo = OSSwapHostToBigInt32(0x0020'0000); + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + rig.addressManager.ApplyRemoteWrite( + loginResponseAddress, + std::span{reinterpret_cast(&response), sizeof(response)}); + StatusBlock status{}; + status.sbpStatus = SBPStatus::kNoAdditionalInfo; + rig.addressManager.ApplyRemoteWrite( + statusAddress, + std::span{reinterpret_cast(&status), sizeof(status)}); + + EXPECT_EQ(LoginState::Suspended, rig.session.State()); + EXPECT_EQ(0, loginCallbacks); + EXPECT_EQ(0u, rig.bus.PendingWriteCount()); // no busy-timeout write +} diff --git a/tests/protocols/SessionRegistryTests.cpp b/tests/protocols/SessionRegistryTests.cpp index 45df95273..3b2ec3bf0 100644 --- a/tests/protocols/SessionRegistryTests.cpp +++ b/tests/protocols/SessionRegistryTests.cpp @@ -276,6 +276,9 @@ TEST(SessionRegistryTests, SubmitRequestSenseCapturesPayloadAndSenseData) { const auto& write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); const uint64_t commandOrbAddress = DecodeAddressFromWritePayload(write.data); const uint64_t dataBufferAddress = ReadDataBufferAddress(rig.addressManager, commandOrbAddress); + // Command data is carried by OHCI physical DMA: the descriptor names a + // sub-4 GB bus address, not a slot in the software-served 0xFFFF window. + EXPECT_LT(dataBufferAddress, 1ULL << 32); const std::vector sensePayload{ 0x70, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x0A, @@ -391,6 +394,8 @@ TEST(SessionRegistryTests, InquiryFailureResultPreservesSBPStatus) { TEST(SessionRegistryTests, ActiveCommandFailsOnceAfterFetchAgentRetryExhaustion) { SessionRegistryRig rig; + int busResets = 0; + rig.registry.SetBusResetRequester([&]() { ++busResets; }); const uint64_t handle = rig.CreateSession(); rig.LoginSuccessfully(handle); @@ -426,11 +431,17 @@ TEST(SessionRegistryTests, ActiveCommandFailsOnceAfterFetchAgentRetryExhaustion) EXPECT_EQ(8u, lunResetWrite.data.size()); ASSERT_TRUE(rig.bus.CompleteWrite(agentResetWrite.handle, ASFW::Async::AsyncStatus::kSuccess)); - // Fail the management write so the LUN-reset ORB retires and frees its - // allocation (it may have reused the command ORB's freed address slot). + // The LUN-reset write fails: nothing confirmed the target stopped, so the + // command's ORB and buffer stay mapped and a bus reset is requested. The + // reset is what ends the target's claim on them. ASSERT_TRUE(rig.bus.CompleteWrite(lunResetWrite.handle, ASFW::Async::AsyncStatus::kTimeout)); - + EXPECT_EQ(1, busResets); uint32_t ignored = 0; + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, + rig.addressManager.ReadQuadlet(commandOrbAddress, &ignored)); + EXPECT_FALSE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, request)); + + rig.registry.OnBusReset(2); EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, rig.addressManager.ReadQuadlet(commandOrbAddress, &ignored)); } @@ -1066,7 +1077,7 @@ TEST_P(LunResetAgentResetTest, OrbTimeoutResetsFetchAgentAfterLunResetBeforeDeli ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); } - auto request = SCSI::BuildTestUnitReadyRequest(); + auto request = SCSI::BuildInquiryRequest(36); request.timeoutMs = 1000; int callbacks = 0; ASSERT_TRUE(rig.registry.SubmitCommand( @@ -1076,9 +1087,17 @@ TEST_P(LunResetAgentResetTest, OrbTimeoutResetsFetchAgentAfterLunResetBeforeDeli const auto orbPointerWrite = rig.bus.WriteAt(rig.bus.WriteCount() - 1); ASSERT_TRUE(rig.bus.CompleteWrite(orbPointerWrite.handle, ASFW::Async::AsyncStatus::kSuccess)); + const uint64_t bufferAddress = ReadDataBufferAddress( + rig.addressManager, DecodeAddressFromWritePayload(orbPointerWrite.data)); + uint32_t retained = 0; rig.AdvanceMs(1001); // ORB timeout → AGENT_RESET (no-wait) + LUN-reset mgmt write ASSERT_EQ(2u, rig.bus.PendingWriteCount()); + // The target may still be writing the buffer: it stays mapped and the + // session takes no new command until the LUN reset says it stopped. + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, + rig.addressManager.ReadQuadlet(bufferAddress, &retained)); + EXPECT_FALSE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, request)); auto mgmtWrite = rig.bus.WriteAt(rig.bus.WriteCount() - 1); if (mgmtWrite.data.size() != 8u) { mgmtWrite = rig.bus.WriteAt(rig.bus.WriteCount() - 2); @@ -1103,6 +1122,9 @@ TEST_P(LunResetAgentResetTest, OrbTimeoutResetsFetchAgentAfterLunResetBeforeDeli EXPECT_EQ(4u, agentResetWrite.data.size()); EXPECT_EQ(0x0020'0004u, agentResetWrite.address.addressLo); EXPECT_EQ(0, callbacks) << "task completes only once the fetch agent is reset"; + // LUN-reset status: the task set is gone, the buffer is ours (§10.4.3). + EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, + rig.addressManager.ReadQuadlet(bufferAddress, &retained)); if (GetParam()) { // A bus reset while the AGENT_RESET is in flight flushes the result; @@ -1313,4 +1335,370 @@ TEST(SessionRegistryTests, BusResetDuringLoginLogsInAgainAfterRediscovery) { EXPECT_EQ(ASFW::Protocols::SBP2::LoginState::LoggingIn, state->loginState); } +class ReleaseParksCommandTest : public ::testing::TestWithParam {}; + +// Quitting a client mid-transfer completes the task at once, but the target +// still owns the buffer until its logout status (§8.4 aborts the task set). +TEST_P(ReleaseParksCommandTest, ReleaseKeepsCommandBufferUntilLogoutStatus) { + SessionRegistryRig rig; + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + int callbacks = 0; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, + SCSI::BuildInquiryRequest(36), [&](const SCSI::CommandResult&) { ++callbacks; })); + const auto write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + const uint64_t dataAddress = + ReadDataBufferAddress(rig.addressManager, DecodeAddressFromWritePayload(write.data)); + + const size_t writesBeforeRelease = rig.bus.WriteCount(); + if (GetParam()) { + rig.registry.ReleaseOwner(SessionRegistryRig::Owner()); + } else { + ASSERT_TRUE(rig.registry.ReleaseSession(SessionRegistryRig::Owner(), handle)); + } + // The task completes at once (our buffer is parked, not the caller's); the + // buffer itself waits for the target to let go. + EXPECT_EQ(1, callbacks); + uint32_t value = 0; + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, + rig.addressManager.ReadQuadlet(dataAddress, &value)); + + const auto& logoutWrite = rig.bus.WriteAt(writesBeforeRelease); + ASSERT_TRUE(rig.bus.CompleteWrite(logoutWrite.handle, ASFW::Async::AsyncStatus::kSuccess)); + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, + rig.addressManager.ReadQuadlet(dataAddress, &value)); + StatusBlock status{}; + status.sbpStatus = SBPStatus::kNoAdditionalInfo; + rig.addressManager.ApplyRemoteWrite( + rig.sessionStatusAddress, + std::span{reinterpret_cast(&status), sizeof(status)}); + EXPECT_EQ(1, callbacks); + EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, + rig.addressManager.ReadQuadlet(dataAddress, &value)); +} + +INSTANTIATE_TEST_SUITE_P(SessionAndOwner, ReleaseParksCommandTest, ::testing::Bool()); + +TEST(SessionRegistryTests, BusResetFreesParkedCommand) { + SessionRegistryRig rig; + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + int callbacks = 0; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, + SCSI::BuildInquiryRequest(36), [&](const SCSI::CommandResult&) { ++callbacks; })); + const auto write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + const uint64_t dataAddress = + ReadDataBufferAddress(rig.addressManager, DecodeAddressFromWritePayload(write.data)); + ASSERT_TRUE(rig.registry.ReleaseSession(SessionRegistryRig::Owner(), handle)); + EXPECT_EQ(1, callbacks); + uint32_t value = 0; + ASSERT_EQ(ASFW::Async::ResponseCode::Complete, + rig.addressManager.ReadQuadlet(dataAddress, &value)); + rig.registry.OnBusReset(2); + EXPECT_EQ(1, callbacks); + EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, + rig.addressManager.ReadQuadlet(dataAddress, &value)); +} + +// A logout the target never answers leaves its claim on the buffer open; +// only a bus reset ends it (your item 1 in #213: reset only when logout fails +// or times out). +class LogoutFailureTest : public ::testing::TestWithParam {}; + +TEST_P(LogoutFailureTest, FailedLogoutKeepsParkedCommandAndRequestsBusReset) { + SessionRegistryRig rig; + int resets = 0; + rig.registry.SetBusResetRequester([&]() { ++resets; }); + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + std::weak_ptr weakSession = rig.registry.GetSessionWeakForTesting(handle); + int callbacks = 0; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, + SCSI::BuildInquiryRequest(36), [&](const SCSI::CommandResult&) { ++callbacks; })); + const auto write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + const uint64_t dataAddress = + ReadDataBufferAddress(rig.addressManager, DecodeAddressFromWritePayload(write.data)); + + const bool rejected = GetParam(); + if (rejected) { + rig.bus.FailNextWriteBlock(); + } + const size_t writesBeforeRelease = rig.bus.WriteCount(); + ASSERT_TRUE(rig.registry.ReleaseSession(SessionRegistryRig::Owner(), handle)); + if (!rejected) { + ASSERT_TRUE(rig.bus.CompleteWrite(rig.bus.WriteAt(writesBeforeRelease).handle, + ASFW::Async::AsyncStatus::kSuccess)); + rig.AdvanceMs(3'000); // logout timeout + } + EXPECT_EQ(1, resets); + EXPECT_EQ(1, callbacks); + EXPECT_FALSE(weakSession.expired()); + uint32_t value = 0; + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, + rig.addressManager.ReadQuadlet(dataAddress, &value)); + + rig.registry.OnBusReset(2); + EXPECT_EQ(1, callbacks); + EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, + rig.addressManager.ReadQuadlet(dataAddress, &value)); + EXPECT_TRUE(weakSession.expired()); +} + +INSTANTIATE_TEST_SUITE_P(RejectedAndTimedOut, LogoutFailureTest, ::testing::Bool()); + +TEST(SessionRegistryTests, TeardownReleaseWithRejectedLogoutRequestsNoBusReset) { + SessionRegistryRig rig; + int resets = 0; + rig.registry.SetBusResetRequester([&]() { ++resets; }); + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + std::weak_ptr weakSession = rig.registry.GetSessionWeakForTesting(handle); + int callbacks = 0; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, + SCSI::BuildInquiryRequest(36), [&](const SCSI::CommandResult&) { ++callbacks; })); + const auto write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + const uint64_t dataAddress = + ReadDataBufferAddress(rig.addressManager, DecodeAddressFromWritePayload(write.data)); + + rig.registry.PrepareForTeardown(); + rig.bus.FailNextWriteBlock(); + rig.registry.ReleaseOwner(SessionRegistryRig::Owner()); + // No reset on a bus that is going down; the command stays parked and goes + // with the registry (the teardown shut the physical filter before this). + EXPECT_EQ(0, resets); + EXPECT_EQ(1, callbacks); + EXPECT_FALSE(weakSession.expired()); + uint32_t value = 0; + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, + rig.addressManager.ReadQuadlet(dataAddress, &value)); +} + +class PhysicalDmaSizeTest : public ::testing::TestWithParam {}; + +TEST_P(PhysicalDmaSizeTest, DescribesTheWholeBufferBelow4GB) { + SessionRegistryRig rig; + const auto handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + SCSI::CommandRequest request{}; + request.cdb = std::vector(10, 0); + request.cdb[0] = 0x28; + request.direction = SCSI::DataDirection::FromTarget; + request.transferLength = GetParam(); + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, request)); + const auto write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + const uint64_t orbAddress = DecodeAddressFromWritePayload(write.data); + const uint64_t descriptor = ReadDataBufferAddress(rig.addressManager, orbAddress); + const uint32_t options = OSSwapBigToHostInt32( + ReadQuadlet(rig.addressManager, orbAddress + offsetof(NormalORB, options))); + if (GetParam() <= 0xF000) { + EXPECT_EQ(GetParam(), options & 0xFFFFu); + EXPECT_LT(descriptor, uint64_t{1} << 32); + } else { + const uint32_t entries = options & 0xFFFFu; + EXPECT_EQ((GetParam() + 0xEFFFu) / 0xF000u, entries); + uint64_t expectedAddress = 0; + uint32_t total = 0; + for (uint32_t index = 0; index < entries; ++index) { + const uint32_t hi = OSSwapBigToHostInt32( + ReadQuadlet(rig.addressManager, descriptor + index * 8)); + const uint32_t lo = OSSwapBigToHostInt32( + ReadQuadlet(rig.addressManager, descriptor + index * 8 + 4)); + const uint64_t address = ComposeAddress(hi & 0xFFFFu, lo); + const uint32_t length = hi >> 16; + if (index != 0) EXPECT_EQ(expectedAddress, address); + EXPECT_GT(length, 0u); + EXPECT_LE(length, 0xF000u); + EXPECT_LE(address + length, uint64_t{1} << 32); + expectedAddress = address + length; + total += length; + } + EXPECT_EQ(GetParam(), total); + } + rig.registry.ReleaseOwner(SessionRegistryRig::Owner()); +} + +INSTANTIATE_TEST_SUITE_P(DescriptorBoundaries, PhysicalDmaSizeTest, + ::testing::Values(0xF000u, 0xF001u, 1024u * 1024u, 30u * 1024u * 1024u)); + +// An HBA task names its own DMA-mapped buffer: SBP-2 points the target at it +// and allocates nothing of its own; the result reports the bytes moved there. +TEST(SessionRegistryTests, InPlaceCommandUsesTheCallersBufferAndAllocatesNone) { + SessionRegistryRig rig; + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + constexpr uint64_t kIOVA = 0x8010'0000ULL; + SCSI::CommandRequest request{}; + request.cdb = std::vector(10, 0); + request.cdb[0] = 0x28; + request.direction = SCSI::DataDirection::FromTarget; + request.transferLength = 0x1000; + request.dataIOVA = kIOVA; + std::optional delivered; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, request, + [&](const SCSI::CommandResult& result) { delivered = result; })); + const auto write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + const uint64_t orbAddress = DecodeAddressFromWritePayload(write.data); + EXPECT_EQ(kIOVA, ReadDataBufferAddress(rig.addressManager, orbAddress)); + uint32_t value = 0; + EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, + rig.addressManager.ReadQuadlet(kIOVA, &value)); + ASSERT_TRUE(rig.bus.CompleteWrite(write.handle, ASFW::Async::AsyncStatus::kSuccess)); + + CompleteTaskManagementStatus(rig.addressManager, rig.sessionStatusAddress, orbAddress); + ASSERT_TRUE(delivered.has_value()); + EXPECT_EQ(0, delivered->transportStatus); + EXPECT_EQ(0x1000u, delivered->inPlaceBytes); + EXPECT_TRUE(delivered->payload.empty()); + EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, + rig.addressManager.ReadQuadlet(orbAddress, &value)); +} + +// With the caller's own buffer, a result after an unanswered LUN reset must not +// go out before the bus reset that ends the target's claim on that buffer. +TEST(SessionRegistryTests, InPlaceResultAfterFailedLunResetWaitsForTheBusReset) { + SessionRegistryRig rig; + int resets = 0; + rig.registry.SetBusResetRequester([&]() { ++resets; }); + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + SCSI::CommandRequest request{}; + request.cdb = std::vector(10, 0); + request.cdb[0] = 0x28; + request.direction = SCSI::DataDirection::FromTarget; + request.transferLength = 0x1000; + request.dataIOVA = 0x8040'0000ULL; + request.timeoutMs = 1000; + int callbacks = 0; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, request, + [&](const SCSI::CommandResult&) { ++callbacks; })); + ASSERT_TRUE(rig.bus.CompleteWrite(rig.bus.WriteAt(rig.bus.WriteCount() - 1).handle, + ASFW::Async::AsyncStatus::kSuccess)); + rig.AdvanceMs(1001); // ORB timeout → AGENT_RESET + LUN-reset management write + auto mgmtWrite = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + if (mgmtWrite.data.size() != 8u) { + mgmtWrite = rig.bus.WriteAt(rig.bus.WriteCount() - 2); + } + ASSERT_EQ(8u, mgmtWrite.data.size()); + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + rig.AdvanceMs(3001); // LUN reset never answered + EXPECT_EQ(1, resets); + EXPECT_EQ(0, callbacks); + rig.registry.OnBusReset(2); + EXPECT_EQ(1, callbacks); +} + +// Timeout while a client ABORT TASK SET is already in flight: no LUN reset can +// be issued, and the caller's buffer is handed back only with that status. +TEST(SessionRegistryTests, InPlaceResultWithoutLunResetWaitsForTheClientTaskManagement) { + SessionRegistryRig rig; + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + SCSI::CommandRequest request{}; + request.cdb = std::vector(10, 0); + request.cdb[0] = 0x28; + request.direction = SCSI::DataDirection::FromTarget; + request.transferLength = 0x1000; + request.dataIOVA = 0x8050'0000ULL; + request.timeoutMs = 1000; + int callbacks = 0; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, request, + [&](const SCSI::CommandResult&) { ++callbacks; })); + const auto orbWrite = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + const uint64_t orbAddress = DecodeAddressFromWritePayload(orbWrite.data); + ASSERT_TRUE(rig.bus.CompleteWrite(orbWrite.handle, ASFW::Async::AsyncStatus::kSuccess)); + + ASSERT_TRUE(rig.registry.SubmitTaskManagement(SessionRegistryRig::Owner(), handle, + SBP2ManagementORB::Function::AbortTaskSet)); + const auto mgmtWrite = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + ASSERT_EQ(8u, mgmtWrite.data.size()); + const uint64_t mgmtOrbAddress = DecodeAddressFromWritePayload(mgmtWrite.data); + ASSERT_TRUE(rig.bus.CompleteWrite(mgmtWrite.handle, ASFW::Async::AsyncStatus::kSuccess)); + + rig.AdvanceMs(1001); // ORB timeout; the LUN reset cannot be submitted + while (rig.bus.PendingWriteCount() > 0) { + ASSERT_TRUE(rig.bus.CompleteNextWrite(ASFW::Async::AsyncStatus::kSuccess)); + } + EXPECT_EQ(0, callbacks); + uint32_t value = 0; + EXPECT_EQ(ASFW::Async::ResponseCode::Complete, + rig.addressManager.ReadQuadlet(orbAddress, &value)); + + CompleteTaskManagementStatus(rig.addressManager, + ReadTaskManagementStatusAddress(rig.addressManager, mgmtOrbAddress), + mgmtOrbAddress); + EXPECT_EQ(1, callbacks); + EXPECT_EQ(ASFW::Async::ResponseCode::AddressError, + rig.addressManager.ReadQuadlet(orbAddress, &value)); +} + +TEST(SessionRegistryTests, InPlaceDataOutCarriesNoPayload) { + SessionRegistryRig rig; + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + SCSI::CommandRequest request{}; + request.cdb = std::vector(10, 0); + request.cdb[0] = 0x2a; + request.direction = SCSI::DataDirection::ToTarget; + request.transferLength = 512; + request.dataIOVA = 0x8020'0000ULL; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, request)); + const auto write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + EXPECT_EQ(0x8020'0000ULL, + ReadDataBufferAddress(rig.addressManager, DecodeAddressFromWritePayload(write.data))); +} + +TEST(SessionRegistryTests, InPlacePageTableCoversTheCallersRange) { + SessionRegistryRig rig; + const uint64_t handle = rig.CreateSession(); + rig.LoginSuccessfully(handle); + constexpr uint64_t kIOVA = 0x8030'0000ULL; + constexpr uint32_t kLength = 1024u * 1024u; + SCSI::CommandRequest request{}; + request.cdb = std::vector(10, 0); + request.cdb[0] = 0x28; + request.direction = SCSI::DataDirection::FromTarget; + request.transferLength = kLength; + request.dataIOVA = kIOVA; + ASSERT_TRUE(rig.registry.SubmitCommand(SessionRegistryRig::Owner(), handle, request)); + const auto write = rig.bus.WriteAt(rig.bus.WriteCount() - 1); + const uint64_t orbAddress = DecodeAddressFromWritePayload(write.data); + const uint64_t descriptor = ReadDataBufferAddress(rig.addressManager, orbAddress); + const uint32_t entries = OSSwapBigToHostInt32( + ReadQuadlet(rig.addressManager, orbAddress + offsetof(NormalORB, options))) & 0xFFFFu; + EXPECT_EQ((kLength + 0xEFFFu) / 0xF000u, entries); + uint64_t expected = kIOVA; + uint32_t total = 0; + for (uint32_t index = 0; index < entries; ++index) { + const uint32_t hi = OSSwapBigToHostInt32(ReadQuadlet(rig.addressManager, descriptor + index * 8)); + const uint32_t lo = OSSwapBigToHostInt32(ReadQuadlet(rig.addressManager, descriptor + index * 8 + 4)); + EXPECT_EQ(expected, ComposeAddress(hi & 0xFFFFu, lo)); + expected += hi >> 16; + total += hi >> 16; + } + EXPECT_EQ(kLength, total); +} + } // namespace diff --git a/tests/support/HardwareInterfaceStub.cpp b/tests/support/HardwareInterfaceStub.cpp index a95d4d0d5..8a56c2c50 100644 --- a/tests/support/HardwareInterfaceStub.cpp +++ b/tests/support/HardwareInterfaceStub.cpp @@ -169,10 +169,17 @@ void HardwareInterface::WriteScoped(Register32 reg, uint32_t value) const noexce if (!state.available) { return; } + const uint32_t previous = state.registers[KeyFor(reg)]; state.registers[KeyFor(reg)] = value; state.operations.push_back(TestOperation::Write); - if (reg == Register32::kIntEventClear) { + if (reg == Register32::kPhyReqFilterHiSet || reg == Register32::kPhyReqFilterLoSet) { + state.registers[KeyFor(reg)] = previous | value; + } else if (reg == Register32::kPhyReqFilterHiClear) { + state.registers[KeyFor(Register32::kPhyReqFilterHiSet)] &= ~value; + } else if (reg == Register32::kPhyReqFilterLoClear) { + state.registers[KeyFor(Register32::kPhyReqFilterLoSet)] &= ~value; + } else if (reg == Register32::kIntEventClear) { state.registers[KeyFor(Register32::kIntEvent)] &= ~value; } else if (reg == Register32::kIntMaskSet) { state.registers[KeyFor(Register32::kIntMaskSet)] |= value;