Skip to content

SBP-2: DMA HBA task data in place via fBufferIOVMAddr - #219

Merged
mrmidi merged 5 commits into
mrmidi:mainfrom
mhellevang:perf/sbp2-zero-copy
Oct 23, 2026
Merged

mrmidi merged 5 commits into
mrmidi:mainfrom
mhellevang:perf/sbp2-zero-copy

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
  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 ← this PR

Zero-copy, on top of #218 and #217; draft until they land. The diff shows their commits until then.

The kernel SCSI shim maps each task's buffer for our PCI device before UserProcessParallelTask and keeps it until ParallelTaskCompletion (fBufferIOVMAddr, one segment). With UserGetDMASpecification at 32 address bits that mapping lies below PhysicalUpperBound, so the SBP-2 target reads and writes it directly, as Apple's SBP-2 does with the task's memory descriptor (IOFireWireSBP2ORB.cpp prepareBufferAddressSpace). CommandRequest::dataIOVA carries the address; the executor builds the page table from it and allocates nothing; CommandResult::inPlaceBytes reports the transfer. UserGetDataBuffer is gone from the HBA: per the SDK it invalidates the fBufferIOVMAddr mapping, so the two are exclusive. The bounce buffer from #217 stays for the raw user client, which has no mapping of its own.

Per READ this removes the bounce allocation, the memset, the copy into a std::vector, the value_or copy and the memcpy into the task buffer, plus one IOBufferMemoryDescriptor per task.

Lifetime: the kernel reuses the task buffer at ParallelTaskCompletion. A result after an unfetched LUN reset (the existing bus-reset escalation) is therefore held until OnBusReset for an in-place command; a bounce buffer is ours and its result still goes out at once. The same holds when no LUN reset can be issued because a client task-management ORB is already in flight: the in-place result waits for that status. Release at dext teardown is covered by #218 clearing the filter first; the other release cause is a device that has left the bus. Normal status, LUN-reset recovery (completion is deferred until its status) and bus reset are unchanged edges. Release at dext teardown completes tasks at once; #218 clears the physical request filter before the bridge shuts down, so the kernel can reuse those buffers safely. The other release cause is a device that left the bus.

Host tests: in-place READ uses the caller's address and allocates no range, data-out needs no payload, page table covers the caller's range at 60 KiB boundaries. HW: the probe build with this exact data path ran a clean full LS-9000 scan with all tasks in place (fBufferIOVMAddr in the 0x801d8000 range, see #217); see the run on this branch below.

Two things stay as they are, stated so they are not missed: the transfer length handed to the target is fRequestedTransferCount, the length the kernel mapped, and there is no second length to check it against without UserGetDataBuffer; and fBytesTransferred reports the full length on GOOD, as the bounce path did (SBP-2 carries no residual), the difference being that a short read now leaves the client's own buffer tail untouched instead of zero-filled.

@mhellevang
mhellevang force-pushed the perf/sbp2-zero-copy branch 2 times, most recently from 2b72ca4 to c04dbe0 Compare October 22, 2026 11:23
@mhellevang

Copy link
Copy Markdown
Contributor Author

LS-9000 run on this branch (986f558 = this + #214 on top, build 39):

  1. Full scan at 2000 dpi: clean, every task in place (no not DMA-mapped fallback, none exists any more), no transport event.
  2. pmset sleepnow mid-scan: the READ in flight completed as aborted (0xe00002eb/sbp=0x0b), bridge shut down, no crash. The failed-logout path from SBP-2: bus reset only when a logout with a command outstanding fails #218 fired during the quiesce ("logout rejected with a command outstanding — requesting bus reset"), which is pointless there; SBP-2: bus reset only when a logout with a command outstanding fails #218 now clears the physical filter before the bridge releases sessions and skips the reset request during teardown.
  3. After wake the driver went through four quiesce/restart cycles (Thunderbolt re-enumeration, 3 s apart), ended Suspended, and from then on the scanner's power-cycle resets were seen only as AR packets (RxPath: Bus reset packet captured), never as Bus reset detected: the bus-reset interrupt path stayed silent until the adapter was re-plugged. The earlier sleep/wake runs (fix(lifecycle): recreate the ROM scanner across runtime rebuilds instead of reusing it against a freed bus #90) had the scanner idle, so wake-with-a-scan-in-flight has no baseline on main; I'll reproduce it on a build without SBP-2: bus reset only when a logout with a command outstanding fails #218/SBP-2: DMA HBA task data in place via fBufferIOVMAddr #219 before attributing it. Not counted as evidence for or against this PR.
  4. Fresh instance after re-plug: login 0.6 s, filter armed, target back, full scan clean.

No crash report in the session. Remaining HW item for this PR: none that this setup can produce; the wake issue is tracked separately.

@mhellevang
mhellevang marked this pull request as ready for review October 23, 2026 08:49
@mrmidi
mrmidi merged commit 9c9a1cb into mrmidi:main Oct 23, 2026
2 checks passed
@mhellevang
mhellevang deleted the perf/sbp2-zero-copy branch October 23, 2026 19:27
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