Skip to content

SBP-2: carry command data by OHCI physical DMA - #217

Merged
mrmidi merged 2 commits into
mrmidi:mainfrom
mhellevang:perf/sbp2-physical-dma-v2
Oct 23, 2026
Merged

mrmidi merged 2 commits into
mrmidi:mainfrom
mhellevang:perf/sbp2-physical-dma-v2

Conversation

@mhellevang

@mhellevang mhellevang commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Split of #213 (merge order):

  1. Bound SBP-2 max_payload by link speed, log posted-write fault address #214 Standalone fixes (max_payload, posted-write, rcvPhyPkt)
  2. Read bus-reset generation from SelfIDCount, not reserved 0x06C #215 Bus-reset generation from SelfIDCount
  3. SBP-2: re-login/reconnect after rediscovery, not on a 100 ms timer #216 Reconnect after rediscovery
  4. SBP-2: carry command data by OHCI physical DMA #217 Physical DMA, needs 2+3 ← this PR
  5. SBP-2: bus reset only when a logout with a command outstanding fails #218 Recovery/teardown policy, needs 4
  6. SBP-2: DMA HBA task data in place via fBufferIOVMAddr #219 Zero-copy, needs 5

Replaces #213. Draft until #215 and #216 are in and the LS-9000 run is done; the branch carries both of them, so the diff shows their commits until then.

Same mechanism as #213, different structure. The wire behavior is the Linux one: enable the target's node for the current generation at login and reconnect (sbp2.c:1146, :1198; ohci.c:2600-2631), never disable a node, clear the whole filter at bus reset and teardown, PhysicalUpperBound once at init (ohci.c:2348), TYPE_ERROR for software requests below it (core-transaction.c handle_low_memory).

What changed from #213, per review:

  • No policy in Hardware/. HardwareInterface::EnablePhysicalRequests(node, generation) -> std::expected<void, PhysicalDmaError> (InvalidNode, HardwareGone, StaleGeneration) and ClearPhysicalRequestFilters(). No per-node counts, no fence, no revoke. The stale-generation check is the one Linux does in the OHCI driver, so it stays here.
  • No revoke → bus-reset chain. The buffer is kept until the target says it is done (SBP-2 §10.4): status block, LUN-reset status, bus reset, or logout status. CommandExecutor has a phase (Idle/Active/Recovering) in place of the flags; resources leave a phase only through those edges. No new reset escalations: a failed AGENT_RESET after a completed LUN reset changes nothing (§6.4.2); a LUN reset the target never fetches keeps main's existing escalation, and that reset is what frees the buffer.
  • Client release. ReleaseSession/ReleaseOwner no longer free a buffer the target may be writing: the command is parked on the retiring session (ParkedCommand, RAII) and freed when the logout status arrives, at bus reset, or with the registry. No bus reset on client exit.
  • Login/reconnect enable is explicit. LoginSession::EnablePhysicalDma() runs before SetState(LoggedIn); a failure (a reset is already in flight) goes Suspended and reconnects after rediscovery, no silent LoggedIn → Failed.
  • Dropped: per-source-ID restriction of software ranges (not in either reference, no real restriction), the AllocateDMA range re-check (the 32-bit spec already guarantees it), the gate change in RevokeAndDrain, the software-request counter. The 0x06C fix is Read bus-reset generation from SelfIDCount, not reserved 0x06C #215, the reconnect change SBP-2: re-login/reconnect after rediscovery, not on a 100 ms timer #216, max_payload/posted-write/rcvPhyPkt Bound SBP-2 max_payload by link speed, log posted-write fault address #214.
  • AddressSpaceManager: physical ranges are read through std::span over the mapping, no pointer arithmetic; cacheable mapping, std::ranges::fill.

Known gaps, for the recovery/teardown PR (#218): a logout that is rejected synchronously or times out frees the parked buffer without confirmation (main's behavior; your item 1 says reset in that case), and the raw user-client path (SBP2Handler::ReleaseOwner) frees all of an owner's ranges at once, so parking only protects the HBA path, which is what VueScan and Phocus use.

documentation/sbp2-physical-dma.md describes the mechanism, the lifetime table and the exposure (any address below PhysicalUpperBound that the OHCI maps, for admitted nodes; source IDs are not authenticated; same as Apple and Linux).

Host tests: 3808 green, 15 new (filter enable/clear/errors, bus reset clears filter, enable at login/reconnect and failure → Suspended, buffer retained through LUN reset and freed on its status, release parks until logout status, bus reset frees parked, page-table geometry at 60 KiB boundaries and 30 MiB).

HW: LS-9000 scan, cancel, client exit and unplug during READ; then Hasselblad for the capture rate if @un4tun8ly has time.

@mhellevang

Copy link
Copy Markdown
Contributor Author

LS-9000 run on 1a0c78d (this PR with #214, #215, #216 on top):

  1. Full scan: clean. [PhysicalDMA] node=0 gen=1 enabled at login.
  2. Cancelled scan: SCSI-level only (SEND LUT 5/24, START/STOP 9/80, then "becoming ready"); no transport event.
  3. VueScan quit mid-transfer: nothing on the SBP-2 side. The HBA keeps the login, so ReleaseSession never runs on this path; the parked-command code is reached only on dext teardown. Noting it so nobody reads this run as covering it.
  4. Scanner power-cycled mid-READ: LoggedIn → Suspended, the READ completed as aborted (0xe00002eb, sbp=0x0b) and its buffer was freed on the reset edge; rediscovery 650 ms, reconnect rejected (power-cycle), full login, filter re-armed for gen 4, target back after the scanner's warm-up.
  5. Full scan afterwards: clean.

Whole session: no ORB timeout, no LUN reset, no escalation, no posted-write error, no crash report. Still draft until #215/#216 land and this is rebased; Hasselblad capture rate is open to @un4tun8ly if they have time.

@mhellevang

Copy link
Copy Markdown
Contributor Author

Zero-copy probe done (local build on top of this PR, not pushed). With UserGetDMASpecification at numAddressBits = 32, every task's fBufferIOVMAddr landed below 4 GiB (0x801d8000 range) and was reachable by the OHCI: INQUIRY data correct, full LS-9000 scan clean with the target DMAing straight into the kernel's task buffer, no UserGetDataBuffer, no copy. So item 5 is feasible and will replace the bounce buffer for HBA tasks; the physical range in this PR stays for the raw user-client path only, which has no IOVA.

Two things the real zero-copy PR has to carry, found while writing the probe: UserGetDataBuffer invalidates the fBufferIOVMAddr mapping per the SDK, so the two paths are exclusive per task; and the kernel releases that mapping on ParallelTaskCompletion, so for an in-place task the completion itself is the "target has stopped" edge and must wait for it, which the recovery path here already does and the teardown path does not yet.

@mhellevang
mhellevang force-pushed the perf/sbp2-physical-dma-v2 branch from 27c40e9 to d0b2a5b Compare October 23, 2026 07:33
@mhellevang
mhellevang marked this pull request as ready for review October 23, 2026 07:33
@mrmidi
mrmidi merged commit c6d76db into mrmidi:main Oct 23, 2026
2 checks passed
@mhellevang
mhellevang deleted the perf/sbp2-physical-dma-v2 branch October 23, 2026 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants