fix(scsi): create target 0 at SBP-2 login, not at boot — HBA-side fix for mrmidi#54#1
Closed
mhellevang wants to merge 20 commits into
Closed
fix(scsi): create target 0 at SBP-2 login, not at boot — HBA-side fix for mrmidi#54#1mhellevang wants to merge 20 commits into
mhellevang wants to merge 20 commits into
Conversation
The OHCI controller loses its programmed state when the system sleeps (IntMask, LinkControl, PHY link). The dext never handled power events, so after wake it stayed alive but deaf: no bus-reset interrupts, so a device plugged after any sleep never mounted until the adapter was re-plugged or the machine rebooted (field failure 2026-07-05: Venice invisible after an overnight idle; full recovery only via TB re-enumeration). Override IOService::SetPowerState (matched services are always in the PM tree). Sleep quiesces and resets the runtime while the controller still answers MMIO; wake rebuilds it via the same bring-up as Start — full OHCI re-init ending in a forced bus reset, after which normal discovery re-publishes devices. This mirrors Linux firewire-ohci (pci_suspend = software_reset; pci_resume = the same ohci_enable as cold probe, ohci.c:3762) and Apple IOFireWireController::setPowerState (quiesce + gate on sleep, re-init + UpdateROM + resetBus on wake). Start/Stop bodies are factored into StartRuntime/QuiesceRuntime and shared with the power path; RegisterService() stays once-per-instance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… off SetPowerState(0) fired 2ms after the last audio nub terminated (device switched off): with no child expressing a power demand, idle power management powered the controller domain down, the sleep path tore down the runtime, and no SetPowerState(On) ever followed — nothing demanded power again, so the driver was dead until reboot (field trace 2026-07-05 13:41). A bus controller must stay fully powered with no devices attached: plug detection requires a programmed, interrupting controller. Declare the desire with ChangePowerState(kIOServicePowerCapabilityOn) at StartRuntime; capability 0 then only arrives for real system sleep, and wake restores the declared desire via SetPowerState(On). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ld-driven power-down HW test showed SetPowerState(0) still arrives 1ms after the last audio nub terminates, even with ChangePowerState(On) pinned: the audio driver matched on our nub is a PM-tree child, and our power state was still governed by its demand. Per IOService.iig, SetPowerOverride makes the state governed solely by our own desire — children can come and go, capability 0 then means real system sleep only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oin happens after Start returns HW test on b10cc02 showed SetPowerOverride(true) returning kIOReturnError (0xe00002bc) and the child-driven power-down still firing. Root cause from xnu IOUserServer.cpp: serviceStarted -> serviceJoinPMTree runs only AFTER the dext's Start() returns, so PM calls made during Start() hit an uninitialized PM object — powerOverrideOnPriv returns IOPMNotYetInitialized, and ChangePowerState_Impl silently discards the same failure (returns success). Move ChangePowerState(On) + SetPowerOverride(true) into the SetPowerState(On) callback, which the kernel delivers right after the PM join. Same log also confirmed the wake-rebuild path works end to end: the domain repowered at 18:54:59, rebuild forced a bus reset, and the device mounted unattended. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…retries Third HW test: device off/on now survives (SetPowerOverride holds — no power-down on nub termination), but after real system sleep the wake rebuild ran during dark wake (SetPowerState(On) at T+4s, full wake at T+7s) and completed cleanly over MMIO — PHY reads, version register, IBR write all fine — yet the forced bus reset never delivered an interrupt: upstream DMA/MSI state was evidently lost between dark wake and full wake, leaving a deaf controller. Every wake rebuild ends in a forced bus reset, and BusResetCoordinator's resetCount only advances through the full interrupt path, so a completed reset observed shortly after the rebuild proves the controller is alive end to end. VerifyWakeRuntime checks that 3s after wake (plus linkEnable/MMIO readback to classify the failure mode) and rebuilds the runtime again if the check fails, up to 5 attempts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…no queue-blocking sleep Review findings on the wake self-heal path: - Both delayed VerifyWakeRuntime dispatches captured `this` in a lambda; a Stop/free before the callback ran would dereference a freed service. - Scheduler::DispatchAsyncAfter implements delays as IOSleep *on* the bound queue, so each 3 s verify attempt froze the driver work queue (up to 15 s over 5 retries), starving the very interrupt path the check verifies. Replace both call sites with ScheduleWakeVerify(attempt): an IOTimerDispatchSource + OSAction targeting WakeVerifyTimerFired, the same pattern as the async watchdog and SBP-2 session timers. The OSAction retains the service, so a pending callback cannot outlive the driver; Stop disables and releases the timer (disable-then-release, no Cancel — see WatchdogCoordinator::Stop) which also breaks the action→service retain cycle. The timer binds to ctx.workQueue, which is the service default queue, so it stays valid across the sleep/wake runtime rebuilds it supervises and serializes with Start/Stop/ SetPowerState. Also document the blocking behavior on Scheduler::DispatchAsyncAfter for the remaining (short-delay) callers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ement Fix: Reinitialize Device after Sleep / Subsequent mounts
sr docs update
fix(build): gate SCSI HBA behind opt-in build flag — default build can boot-panic at cold boot
…oker FW-67: audio: extract clock request broker
…ofile FW-74: audio: extract duplex stream profile
…nsaction FW-70: Extract the duplex start transaction runner
… HBA fix) The HBA answered UserTargetPresentForID(0) = true unconditionally, so the kernel shim's bring-up presence scan auto-created target 0 on every boot — device present or not. With no SBP-2 device on the bus, the probe INQUIRY was then held with no deadline waiting for a login that never arrives, target-0 registration never completed, and watchdogd panicked the boot at 60 s (registry busy-timeout, IOService.cpp:5986). Move the target lifecycle onto the SBP-2 session (framework hotplug model): - UserTargetPresentForID answers true only while targetAttached — the bring-up scan creates nothing, so a device-less boot has no target whose probe could strand the registry. - The login observer (already delivered via SBP2BridgeHub; the registry emits terminal edges only — transient bus-reset suspension emits nothing, reconnect re-asserts login) now drives the lifecycle: UserCreateTargetForID(0) on login-up, held-INQUIRY flush + UserDestroyTargetForID(0) on terminal logout/login-failure. - Lifecycle calls run on a dedicated serial queue: NOT auxQueue (the create call is routed through AuxiliaryQueue by the framework — a call from that queue never dispatches; the v49 wedge) and NOT the Default queue (it services the framework's target-init upcalls). - Start catch-up: if the FireWire side is already logged in when the HBA starts (service restart), synthesize the up-edge. - Stop: a stopping flag gates queued lifecycle blocks off create/destroy, then a sync barrier drains the queue before the aux flush. Explicit destroy in Stop remains forbidden (re-enters the framework's own child termination — the documented teardown panic). - The deferred-INQUIRY machinery is kept for the one window where it still applies: a probe against an existing target while the session is suspended after a bus reset. Known limitation: a device that vanishes while suspended (never reconnects, never reaches a terminal state) leaves a zombie target that answers BUSY; harmless, cleaned up at teardown. A suspend timeout is future work. Builds with --scsi (personality verified in the artifact); host suite 1243 green. HW validation pending — test plan: 1. Cold boot, adapter attached, scanner OFF → expect clean boot, no target in ioreg, log shows "no target until SBP-2 login". 2. Power scanner on → login → "target 0 created" in log, scanner visible to VueScan, scan works. 3. Scanner off/unplug → logout → "target 0 destroyed", no panic. 4. Full scan, then restart with adapter in + scanner off → clean boot. 5. Unplug adapter mid-idle and re-plug → no panic. 6. Boot with scanner ON → login → create → scan (original happy path). Log filter: log show --last 10m --predicate 'eventMessage CONTAINS "[SCSIHBA]"' --info --debug
Fixes three confirmed findings against the previous commit: 1. Stop deadlock (blocker): Stop ran on the Default queue and DispatchSync'd onto lifecycleQueue while an in-flight UserCreateTargetForID cannot return until its target-init upcall is serviced on that same Default queue — a three-way cycle (Stop → lifecycleQueue → kernel create → Default) that wedges termination into the same 60 s registry busy-timeout panic this branch fixes. Stop no longer waits: queued lifecycle blocks are gated by the stopping flag, and queue objects are released in free(), which cannot run until every block (each holds a controller retain) has finished. This also fixes the failed-Start queue leak (free() now releases both queues; Stop is never called after a failed Start). 2. Scan/create duplicate (blocker): the bring-up presence scan is queued on the Default queue with no ordering against login edges on lifecycleQueue — a login-driven create landing before the scan would be duplicated by it (presence read true), the HW-observed v49 duplicate-target wedge. UserTargetPresentForID now answers false UNCONDITIONALLY; all creation is explicit on the login edge. 3. Suspended-window strand (blocker): the deferred-INQUIRY hold had no deadline, and a device that vanishes while Suspended emits no terminal edge (RefreshTargets skips sessions whose unit no longer resolves) — a held completion would never fire, pinning the registry: the mrmidi#54 mechanism relocated, not removed. The hold machinery is deleted. The SAM registration probe now runs right after the login-edge create, while the session is up, so it is forwarded to the device and returns its real identity; in the suspended window INQUIRY answers BUSY like everything else, so bounded initiator retries land after the reconnect or fail cleanly. TUR/REQUEST SENSE still answer GOOD. Also from the review: the up-edge re-checks IsReady() at execution time (a stale Start catch-up edge can no longer create a target for a session that has since logged out); the logout path logs destroy failures honestly; SBP2BridgeHub.hpp's observer comment now states the under-lock invariant Stop is load-bearing on. Known accepted race (documented in HandleLoginEdge): a lifecycle block that passed the stopping check can still be inside a create/destroy kernel call when the framework begins terminating the controller; the framework must tolerate hotplug calls racing termination. Builds with --scsi; host suite 1243 green. HW test plan unchanged (see previous commit), plus: bus reset mid-scan (plug second FW device) → scan resumes or fails cleanly, no wedge.
mhellevang
force-pushed
the
scsi/presence-login-target
branch
from
2417b3d to
585b418
Compare
Draft
9 tasks
Owner
Author
|
Superseded by mrmidi#61 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The HBA-side fix for mrmidi#54: SCSI target 0 is now created at SBP-2 login and destroyed at terminal logout, instead of being auto-created by the boot-time presence scan. This makes
--scsibuilds cold-boot-safe: a machine booting with no SBP-2 device on the bus has no target whose probe could strand the registry into watchdogd's 60 s busy-timeout panic (IOService.cpp:5986).Stacked on
fix/gate-scsi-entitlement(mrmidi#55). Draft until the hardware validation checklist below is complete; will be re-targeted upstream once mrmidi#55 merges.Model (pure framework hotplug)
UserTargetPresentForIDanswers false unconditionally — the bring-up scan never creates a target, and (because the scan's timing is unordered against login edges) a login-driven create can never be duplicated by the scan (the HW-observed v49 duplicate-target wedge).UserCreateTargetForID(0)on the login-up edge,UserDestroyTargetForID(0)on terminal logout/login-failure — delivered via the existingSBP2BridgeHubobserver; the registry emits terminal edges only, so a transient bus-reset suspension leaves the target alone (reconnect re-asserts login).AuxiliaryQueue(the create call is routed through it — calling from it never dispatches) and not the Default queue (it services the framework's target-init upcalls).Adversarial review
Three confirmed blockers in the first draft, fixed in the second commit:
free().trueby a late bring-up scan. Presence is now constant false.Build with
--scsiverified (personality present in the artifact); host suite 1243 green.Hardware validation checklist (blocking — needs FW adapter + scanner)
ioreg, log showsno target until SBP-2 logintarget 0 created, scanner visible to VueScan, scan completestarget 0 destroyed, no paniclogin edge stale (session not ready)on a normal scanner connect — if it appears, the observer edge fires before the registry state is queryable and the ordering needs adjustmentUserCreateTargetForID(0) failed/UserDestroyTargetForID(0) failed— the create/destroy-vs-termination race is accepted-and-logged; confirm it stays harmless on the unplug pathsLog capture:
log show --last 10m --predicate 'eventMessage CONTAINS "[SCSIHBA]"' --info --debugFor boot scenarios use
--boot -1after the reboot instead of--last 10m.Known accepted gaps (documented in code)
Follow-ups (not this PR)
v0.0.0-citest.2predates the SCSI gate).mainonce fix(build): gate SCSI HBA behind opt-in build flag — default build can boot-panic at cold boot mrmidi/ASFireWire#55 merges.