[mhellevang]

Background

Hasselblad H40d user un4tun8ly reported slow transfers over SBP-2.

Previously, each block transfer went through the software async-request handler. This moves command data into OHCI physical memory so the controller can handle the transfers directly. ORBs, page tables and status FIFOs still use software address space.

What changed

Physical access is restricted to the logged-in target and current bus generation, with register readback to check that the filter was applied. LUNs on the same node share access. Software requests into physical buffers return TYPE_ERROR, so transfers can't quietly fall back to the old path.

Command buffers stay mapped until timeout recovery has reset the LUN and fetch agent. If recovery fails or an active command is released, access is revoked before the buffers are freed and a bus reset is requested. This also fixes related MMIO/reset ordering, filter cleanup, payload limits and DMA address validation.

Testing

I've tested with my CoolScan 9000 as a regression check. It still scans fine, and the logs confirm that physical DMA is enabled.

In the main transfer phase on October 20, 1,212 larger commands completed in 40.58 seconds with successful SBP-2 status and no logged transport errors, timeouts or fetch-agent resets. Median command time was 25 ms; 95% completed within 89 ms.

That works out to roughly 3.7–5.5 MB/s. It's a range because the logs record page-table counts rather than exact payload lengths: each command used three entries of at most 60 KiB. The timing includes scanner waits and command processing. These figures describe the CoolScan regression test.

The scanner may not saturate FireWire, so a camera or hard drive is better suited to benchmarking throughput.

Hasselblad H4D-50MS (un4tun8ly, Phocus 3.8.4, macOS 27.0.1, build 6922e922): capture transfer went from 22.9 to 48.0 MB/s (~2.1×) compared with the #207 baseline. Two Single captures and a four-shot MultiShot completed and produced viewable images, and the log shows physical DMA being enabled. CF import and thumbnail loading were unchanged. They look latency-bound rather than bandwidth-bound and are out of scope here.

Other checks:

• C++ tests: 3,805 passed, six existing tests skipped, no failures. Coverage includes access filters, generations, shared LUNs, recovery ordering and transfers up to 30 MiB.
• Unsigned Debug driver build passes for x86_64 and arm64e.
• Full unsigned Release build passes after installing the Metal Toolchain. Analyzer and Swift warnings remain.
• XcodeGen regeneration and git diff --check pass.
• I checked the behavior against the Linux and Apple stacks. References and the remaining checklist are in SBP-2 physical DMA validation.

Still to test

Data-out, cancellation, client exit, unplug during a read, timeout recovery, node-ID changes after reset and multi-LUN operation still need hardware testing. An abort that can't be confirmed blocks every LUN on that node until reset. Host tests also can't prove DMA coherence or that outstanding writes have drained after access is revoked.

[un4tun8ly]

PhocusPocus — PR #213 hardware testing

Testing performed on 21 October 2026, approximately 01:18–02:55 London BST (UTC+1), before the separate experimental investigation.

Setup

• Apple-silicon MacBook Air, macOS 27.0.1 (26A434), SIP disabled.
• Hasselblad H4D-50MS, firmware R366.
• FireWire 800 → Apple FireWire-to-Thunderbolt adapter → Apple Thunderbolt 2-to-3 adapter → MacBook Air.
• Unmodified Phocus 3.8.4.
• Baseline: PR fix(ohci): acknowledge IntEvent before servicing AR rings #207 commit 07889eb, local build 25.
• Candidate: PR Use OHCI physical DMA for SBP-2 command data #213 commit 6922e922d78cb1d256bda99712edfe87b74da64c, local build 34.
• Active candidate driver CDHash verified in IORegistry:
  d98e265b735f43df4b48a0a3b11f81f60eb918f0.

Results

Test	Baseline build 25	Candidate build 34
Phocus-reported capture transfer rate	22.9 MB/s	48.0 MB/s
Same CF image import, A9423717	Approximately 5 seconds	Approximately 4–5 seconds
Folder/thumbnail loading	Approximately 46–50 seconds	Reported similar, approximately 50 seconds
Single capture	Successful transfer recorded; later hangs/stalls also occurred	Two completed; images confirmed viewable
Four-shot MultiShot	No matched successful baseline in this run	Completed; four shots combined into one viewable image

The logged capture-transfer rate increased by approximately 2.10× in these samples. Import and thumbnail timings were user estimates and did not show a comparable doubling of the complete workflow.

The driver logged physical DMA activation at 02:42:05.019 BST:

[PhysicalDMA] node=0 gen=5 enable=1 filter=0x00000001 applied=1

Remaining observations

• Initial candidate discovery was delayed. Recognition followed reconnect/refresh/ROM-read attempts; the effective action was not isolated.
• Calibration/header, metadata and black-standard-deviation warnings persisted.
• Baseline hangs and a stalled capture occurred before PR Use OHCI physical DMA for SBP-2 command data #213 was installed; their causes remain unresolved.

Interpretation and limitations

The speed result is encouraging but preliminary. The candidate was 63 commits ahead of the old baseline, exposure/image content differed, and repeat counts and cache controls were limited. This does not isolate the physical-DMA change alone.

The candidate CF import also landed in the baseline destination folder rather than the planned fresh candidate folder.

Independent speed lines were not recorded for the second Single or four-shot capture in these snapshots. Viewable output was confirmed, but full pixel-integrity and image-quality validation was not performed.

Normal logging was requested but not independently confirmed, and the baseline stream reported dropped messages. The attachment is retained evidence, not a complete bus trace or ASFW in-app ring export.

Data-out, cancellation, unplug-during-transfer, timeout recovery, node-ID changes and multi-LUN safety were not validated by this run.

Attachment

The attached ZIP contains scoped driver/Phocus logs, observations, a hang sample, disk records, build identity/configuration, capture-file metadata and SHA256 checksums. Later experimental work and original photographs are excluded.
PhocusPocus-PR213-6922e92-2026-10-21-pre-six-shot.zip

[mhellevang]

Thanks, this is really useful. The capture speedup and a completed MultiShot are exactly what this PR was meant to show.

A few notes from the logs:

• Import and thumbnails: these don't move much data per command. During thumbnail loading Phocus issues about one small read per second, on top of its regular status polling, and waits for each command to finish before sending the next. Faster DMA only shortens the transfer part of that, so an unchanged time is expected. The 64 MB import at ~4–5 s is also well below the capture rate, which points at the camera reading from its CF card rather than the bus. I'm treating both as out of scope for this PR.
• Delayed discovery: the driver restarted a few times between 02:32 and 02:42, which lines up with the adapter chain being reconnected. In the one start where Self-ID was logged (02:36:15), only our own node was on the bus, so the camera wasn't connected yet. Login happened right after it appeared (02:42:05). Your log filter didn't include Discovery/ROMScanner lines, so I can't confirm this for every restart, but nothing points at a driver issue.

If you get a chance on the iMac/Sequoia setup, timing the same folder's thumbnail load and the same CF import there would tell us whether there's anything to gain on our side. That's optional and doesn't block this PR.

[mrmidi]

Thanks @mhellevang, and thanks @un4tun8ly for the careful test run. 22.9 → 48 MB/s on the H4D is a great result, and physical DMA is the right direction: it's how both Apple and Linux carry SBP-2 data.

I'm not ready to merge it as is, though. Mostly not because of single bugs, but because of how the PR solves problems.

Main concern: mechanisms copied, reasons not asked

The wire behavior is right: per-node filter, generation check, PhyUpperBound, TYPE_ERROR for low memory, max_payload limits. But a lot of the structure around it is copied too, and each issue is fixed in place where it showed up: a check, a flag, a counter, a reset. Our rule (CLAUDE.md) is to take wire behavior from Linux/Apple and write the structure fresh for DriverKit.

The clearest example is the reset chain:

1. The command buffer is freed as soon as the command ends (the old shape).
2. That's unsafe if the target may still write into it, so the node gets revoked.
3. Revoking blocks every LUN on that node, so a bus reset is requested to clear it.
4. The bus reset hits every device on the bus, including audio interfaces.

Asking "why free it now?" at step 1 makes steps 2–4 unnecessary. SBP-2 already says to keep the memory until a stop point (§10.4.1–10.4.3: "the initiator shall not reuse the system memory … until completion status is returned"). With the IOMMU, keeping a buffer mapped a bit longer is cheap and safe.

A smaller example: length - 1 > 0xFFFFFFFFULL - mappedAddress in AllocateDMA. AllocateDMA already asks the kernel for 32-bit addresses (kDefaultDMAMaxAddressBits = 32) and rejects multi-segment results, so this case can't happen. State the limit once (static_assert / assert), or use std::add_sat if a runtime check has to stay.

Second concern: scope

This PR changes behavior well outside SBP-2:

• bus-reset generation for the whole controller (IRM, cycle master, gap/root policy, AV/C, speed map);
• the hardware access gate on teardown (RevokeAndDrain);
• the DMA allocator used by every ring, isoch buffer and the Config ROM;
• how every incoming request below 4 GB is answered (TYPE_ERROR);
• link control and fault-interrupt handling.

The test evidence covers one SBP-2 camera and one scanner. Each of those areas needs its own review against its own reference and its own test. That's the main reason I'm asking for the split below. The generation change is a good example. It turned out to be correct (see below), but confirming that took the OHCI spec and Apple's reset ordering. And it changes stale-work handling for the whole bus. That shouldn't ride along in an SBP-2 perf PR.

Blocking

1. Client release resets the whole bus. CommandExecutor::Cleanup() revokes and calls busResetRequester_() (CommandExecutor.cpp:428), and ReleaseSession calls it before sending logout (SessionRegistry.cpp:294). Quitting Phocus or VueScan mid-transfer will drop any audio interface on the same chain. Logout already aborts the task set (§8.4, §10.5). Please keep the buffer until the logout status arrives, and reset only if logout fails or times out. Neither Apple nor Linux resets the bus on client exit.
2. New reset escalations: a failed agent-reset write after a completed LUN reset (:250), and failing to submit the LUN reset (:257). Once the LUN-reset status arrives, the memory is reusable by spec (§10.4.3). AGENT_RESET has no status (§6.4.2), so its failure says nothing about DMA.
3. Access policy lives in HardwareInterface (per-node counts, revoke fence, blocked mask). Neither reference has this. Linux enables per node and generation and never disables (sbp2.c:1147,1199). Apple enables for every scanned node (IOFireWireController.cpp:3190). Hardware/ should stay raw MMIO. If we need a lifetime, make it an RAII lease owned by the session, returned as std::expected, with the generation inside. That replaces Acquire/Release inside SetState and the destructor. SetState quietly turning LoggedIn into Failed is hard to follow.
4. Reconnect now waits for rediscovery (the 100 ms timer is gone). The target only holds the login for reconnect_hold + 1 s, which is 1 s when it reports 0 (§8.3, §10.5). The delayed discovery @un4tun8ly saw is in exactly this area, and the log filter didn't include Discovery/ROMScanner lines, so I'd rather not call it out of scope yet. One run with those lines, measuring bus reset → Reconnect write, would settle it.

Should fix

5. ControlPhysicalRequests(node, gen, {Acquire, Release, Revoke, Check}) -> bool: five different failures (stale generation, reset pending, fenced, readback mismatch, gate closed) all come back as false. Separate calls + std::expected.
6. CommandExecutor gets two more flags (recovering_, commandDMAQuiesced_) on top of the existing ones. An explicit phase enum would make the bad combinations impossible.
7. New raw pointer arithmetic: StorageBytes(range) + offset (AddressSpaceManager.hpp:302-303, replacing the old iterator form), memcpy at :450, memset at :751. DriverKit 27's libc++ already runs in hardened fast mode, so std::span::subspan gives bounds checks for free. -Wunsafe-buffer-usage flags all of these.
8. LUN reset is optional per spec (§10.4.3). ABORT TASK SET is mandatory, and it's the one that explicitly waits for pending data transfers (§10.4.2 c). With this PR, a target that rejects LUN reset gets its node fenced plus a bus reset on every timeout. main already resets in that case (following Apple), so I'm not asking to switch, but let's not make it worse.
9. Docs: "restricted to the logged-in target" overstates it. Source IDs aren't authenticated, and a node with the filter set can reach everything the OHCI is mapped for (rings, Config ROM, audio buffers). That's the same exposure as Apple and Linux; just say it plainly. Also, documentation/sbp2-physical-dma.md reads as a review log, and its Linux line numbers come from a newer tree than our references.

Left halfway (not blocking, but this is a perf PR)

The software per-block path is gone, but the data path keeps its old shape: per-command DMA buffer + memset, a copy into a std::vector, another full copy in cb(result.value_or(...)) (value_or on an lvalue copies the payload), then memcpy into UserGetDataBuffer. That's 3 copies and 2 N-byte heap buffers per READ, all on the Default queue that also services the OHCI interrupt. Most of this was already on main, not yours, but it's the path you're rewriting.

The SDK says UserGetDataBuffer "can have a significant impact on performance", and the kernel already maps the task buffer (fBufferIOVMAddr). With a 32-bit DMA spec that should give us zero-copy. SBP-2 allows it (unrestricted page table, §5.2.1, §9.2), and it's what Apple's SBP-2 did (IOFireWireSBP2ORB.cpp:960-1040). It needs a HW check that the address is mapped for the OHCI device. Worth deciding before the bounce buffer is final.

Good fixes in here

max_payload bounded by speed (§5.1.2; main violated it at S400; please log the silent 512-byte fallback in DriverContext.cpp), reading the posted-write fault address before the ack (same order as Linux ohci.c:2278-2282), and keeping buffers until recovery status (§10.4). These deserve to land soon.

The 0x06C generation fix: correct, but it belongs in its own PR

0x06C is reserved (OHCI 1.2 register map), and SelfIDCount.selfIDGeneration "increments each time a bus reset is detected" (OHCI 1.2 Table 11-2). The bus-reset packet description says the same: the generation is "for the new (not previous) generation" (Table 8-4 text). So at the bus-reset interrupt this reads the new generation, which is what the OnBusResetStarted(generation) consumers need to drop old-generation work (CyclePolicyCoordinator.cpp:248, IRMFallbackCoordinator.cpp:105). It matches Apple's model of moving to a new generation at the reset edge (IOFireWireController.cpp:1998-2000). main was reading a reserved register there, so this is a real fix for the whole controller.

Because it changes stale-work handling for the whole bus, it should land as its own small PR, not inside an SBP-2 perf change.

Proposed split

1. Standalone fixes: max_payload, posted-write address logging, the rcvPhyPkt constant. The bus-reset generation fix gets its own PR, since it affects the whole controller (see above).
2. Physical DMA enable on the Linux model (per node, per generation), data buffer in physical space, buffers kept until status. This alone should carry the speedup.
3. Recovery/teardown policy (items 1, 2, 8): logout before reset, no escalation on agent-reset failure.
4. Reconnect timer removal, with the timing measurement.
5. Zero-copy via fBufferIOVMAddr.

For 3 and 4 we need HW runs on the paths that are still untested: cancel, client exit and unplug during READ, timeout recovery, node-ID change after reset. Ideally with an audio device streaming on the same bus.

Spec references are to the T10 SBP-2 rev 4 draft (1998).

Thanks again. The speedup is real; I just want it in a shape we won't have to fight later.

[mhellevang]

Thanks for the feedback, you're right - I'll address your concerns 👌

[mhellevang]

Plan, so it's on record before I start:

I'll split this into the PRs you listed, with one ordering change. Reconnect-after-rediscovery (your item 4) has to land before the physical-DMA PR, not after: Reconnect() takes the node ID from targetInfo_, which is only refreshed by RefreshTargets, so the old 100 ms timer reconnects to the pre-reset node ID. With a per-node filter armed on that ID, a stale value arms it for the wrong node. Both references wait for rediscovery as well (Linux sbp2_update, Apple resumeNotify). I'll include the bus reset → Reconnect-write measurement in that PR.

Order:

1. Standalone fixes: max_payload (with the 512-byte fallback logged), posted-write address before ack, rcvPhyPkt constant.
2. Bus-reset generation from SelfIDCount (own PR).
3. Reconnect after rediscovery, with the timing measurement.
4. Physical DMA on the Linux model: new branch from main, mechanism carried over, lifecycle rewritten as an RAII lease owned by the session, buffer kept until status, no revoke/reset chain. The per-source-ID restriction in AddressSpaceManager goes away; it isn't in either reference and doesn't restrict anything real.
5. Recovery/teardown: logout before any reset, no escalation on agent-reset failure.
6. Zero-copy via fBufferIOVMAddr.

1–3 are independent and go against main; 4 will be opened as a draft on top of 2 and 3. HW: I'll test 1–3 in one build on the LS-9000 (scan, unplug/replug with Discovery/ROMScanner logging), and 4–6 in a second build (scan, cancel, client exit and unplug during READ). I don't have an audio device, so a run with one streaming on the same bus is open to anyone who does. I'll close this PR once 4 is up.

[mhellevang]

Superseded by #214, #215, #216 and #217 (draft), per the plan above.