Repository navigation
media: apple: isp: contain IPC failures - #1560
DjDeveloperr merged 10 commits into
Conversation
Latch the first compact-protocol IPC failure and reject later sends before shared command memory is reused. Stop the coprocessor before reclaiming published surfaces and clear the latch only after fresh initialization. Retain Aurora firmware restoration, deferred detection and the recoverable P010 late-ACK resynchronization path. Adapt the host session fixture to the current command helper and transport fields. Signed-off-by: Satya Benson <satya@benson.earth> Assisted-by: LLM
Keep buffers owned by the host until submission succeeds and retain published metadata across uncertain sends. Reset metadata submissions for each new stream and cover rollback with the current Aurora host fixture. Signed-off-by: Satya Benson <satya@benson.earth> Assisted-by: LLM
Limit each receive turn to one ring so a refilling producer cannot starve other channels. Quiesce resident receive processing before reclamation and latch errors from channel handlers. Exercise refill and shutdown ordering with host control-flow tests. Signed-off-by: Satya Benson <satya@benson.earth> Assisted-by: LLM
Snapshot firmware return descriptors and validate identity, pool and duplicate returns against outstanding host ownership before completion. Cover rejected returns and retained buffers in host and V4L2 KUnit tests. Signed-off-by: Satya Benson <satya@benson.earth> Assisted-by: LLM
Notify streaming queues of a latched IPC error and return queued buffers without republishing them into the failed session. Cover queue wakeups and buffer ownership with host controls and V4L2 KUnit tests. Signed-off-by: Satya Benson <satya@benson.earth> Assisted-by: LLM
Register the five containment suites and eleven compiled negative controls without preview enablement. Pin rollback to the fresh public integration baseline and recognize its assertion failure as a killed control. Signed-off-by: Satya Benson <satya@benson.earth> Assisted-by: LLM
Preserve the merged exposure bounds and control-gate tests from PR aurora-silicon#978 alongside the suspended-return and error-wakeup containment cases. Retain both complete test functions and all registrations. Signed-off-by: Diljit Singh <hey@djdev.me>
DjDeveloperr
left a comment
DjDeveloperr
left a comment
There was a problem hiding this comment.
Codex (GPT-6.1 Sol), reviewing on @DjDeveloperr’s behalf.
Source review of 48c4159: no blocking source finding. I integrated current integration/aurora ea10c4d with a normal merge, preserving the #978 exposure bounds/helper and its entire control-gate test alongside both new containment tests. All 13 V4L2 KUnit case registrations remain unique. No preview defaults, admission tables, DTs, setfiles or format/interval selection changed.
The ownership change is conservative: uncertain submissions remain host-recorded until return or coprocessor shutdown; a returned list is snapshotted and wholly validated before any completion; shutdown stops the coprocessor before memory release. P010 late-ACK resynchronization and ACK-deferred lease completion remain intact. Error notification is serialized with the streaming transition, and receive rounds are bounded across channels.
I reran all five containment extraction suites and eleven compiled negative controls on hash-verified merged production sources: 16/16 passed under ASan/UBSan. The session suite used Clang; the remaining runs used GCC 16.1.1 to avoid this host’s severe Clang library I/O stalls. Controls failed at the intended runtime checks, not by failing to compile. Diff-check passes. The prior head’s hosted full runner was also inspected: 240/240 passed; that prior result is not current-head CI. Current head host run43070674486 is queued.
This approves the narrow generic containment source change. I did not run a new kernel/QEMU or hardware test. The author’s prior split-head KUnit46/46 and earlier combined-head J613 evidence keep their stated scope; exact integrated-head T6040/P010 hardware regression and forced IPC-failure hardware injection remain untested. No preview/default/image-quality qualification follows; #974 retains those gates. Merge still requires the new head’s CI disposition.
|
GPT-6.1 Sol on @satchlj's behalf Exact PR head tested: Unattended J613/T8122, firmware 26.6.2. Kernel This split head was tested together with #974's exact preview head f512cd7, not as a containment-only kernel. On apple-isp /dev/video1: all 65 advertised NV12 size/rate cells (13 sizes × 15/20/24/25/30 fps; 90 buffers each) and 20 additional stop/restart sessions passed readback, sequence, monotonic timestamp and ±5% cadence gates (6,450 buffers). The full suspend variant also passed: 6,780 validated buffers in 92 records, closed-camera and active-stream 20s RTC sleeps, post-resume/restart coverage. Manual active-stream review: 208 buffers, no sequence gap/nonmonotonic timestamp; 80 before suspend entry and 120 after resume exit. The naturally exercised firmware-stop failure/explicit-stop fallback remains visible: No clean firmware-suspend claim. No new matching kernel fault signature versus baseline. The runbook needed only a privileged read of root-only preview_26 metadata; camera operations stayed unprivileged. The initial EACCES receipt is retained. Not run: forced IPC/submission/return corruption, isolated containment-only hardware, T6040/P010 regression, KUnit in this hardware task, or human visual quality. Normal streaming/restart/suspend evidence does not qualify every containment failure branch. Raw statistics capture was deleted, no images exported. Left on default 30.12, no trial/one-shot, lease released. |
Preserve the current restartable firmware idle lease, heap restoration and idle work lifecycle alongside failure containment and P010 ownership. On a fresh boot, clear the receive-quiescing flag only after channel setup and before IRQ enable. On failed reuse of idling firmware, retain its pending stop lease rather than canceling it before returning the error. Extend the existing production-function session fixture with idle reuse, poisoned lease retirement and fresh-channel receive recovery. Preserve all existing checks and current main host registrations. Five containment suites and eleven runtime mutation controls pass, plus three additional compiled lifecycle mutations rejected at their intended assertions. Signed-off-by: Diljit Singh <hey@djdev.me>
DjDeveloperr
left a comment
DjDeveloperr
left a comment
There was a problem hiding this comment.
Codex (GPT-6.1 Sol), reviewing on @DjDeveloperr’s behalf.
Re-reviewed current integration9d26f349b25e5853e268aabfb6543433f6552428 (parents48c41597 and mainf9f14be0). The merge preserves current resident-firmware fallback, delayed idle work, init/flush callers and heap restoration alongside containment ownership, IRQ quiescing, bounded return handling and P010 late-ACK behavior. Source approval applies to this candidate after an independent lifecycle audit.
The functional merge exposed two restartable-idle composition bugs, both fixed after failing regressions were written:
- Fresh boot must clear stale ipc_quiescing only after fresh channel initialization and before enabling IRQs. Failed earlier boot stages retain the failure state; old firmware memory is not released while it can still run.
- A failed RUNNING reuse must return the existing IPC error before cancelling fw_idle, so the pending lease can still stop poisoned firmware. Only healthy reuse cancels that lease.
All five containment suites and eleven existing compiled runtime-negative controls pass under ASan/UBSan on the final source. The affected session and four old session controls explicitly hash-match final isp-fw.c blob2bea18d7/SHA256 eea5d2bf997b6a0bb41ccaae4aef5fb54deabb613f86a66a5e121723f6b3bc2a. Three added lifecycle mutations (missing fresh reset, premature reset, failed reuse cancelling the lease) compile and fail their intended runtime assertions. Current-main ISP init/flush/remove/suspend/shutdown hooks and heap-restoration functions are preserved; the full current host-runner registry and #1010 controls remain intact.
Fresh host run43089958792 is in progress; merge must wait for its actual result. No new kernel build, QEMU or hardware test was run for this candidate. The author’s exact48c J613 combined-tree streaming/restart/suspend report remains valuable earlier-head evidence, with firmware-stop fallback explicitly recorded. It is not a boot of this new merge, forced IPC-corruption testing, T6040/P010 hardware regression or human preview-quality qualification. #974’s preview/default gates remain separate. No loader/default, playback or image operation occurred.
Keep the merged J613 MTP sleep fixture and all current tests alongside the ISP containment entry. Register the unchanged ISP cases in the existing alphabetical section so later unrelated first-entry additions do not collide. All ISP sources and fixtures remain byte-identical to the reviewed, tested 9d26f34 source; only the runner union changes. The existing FAIL-line mutation recognition and all current-main jobs, ordering and controls are retained. Signed-off-by: Diljit Singh <hey@djdev.me>
DjDeveloperr
left a comment
DjDeveloperr
left a comment
There was a problem hiding this comment.
Codex (GPT-6.1 Sol), reviewing on @DjDeveloperr’s behalf.
The previous9d26f349 host check passed. The late hardware comment6804320576 is unchanged and already incorporated with its earlier48c combined-tree scope. New main changes produced one further conflict, limited to the host-test registry; no ISP source overlap exists.
Integrated main33a12085 as4415f9c7, preserving original ancestry. All15 ISP source/fixture blobs are byte-identical to the approved9d26f349, including both regression-backed idle fixes (FWblob2bea18d7/header1a62). The entire main registry’s100 entries/order/controls are preserved, with the same16 ISP cases registered once in a stable alphabetical position and the existing FAIL-line kill detection retained. No test assertion, source behavior or qualification scope changed, so the sealed16+3 results remain applicable without rerunning them solely for registry movement.
Existing source approval stands. Fresh current-head host workflow43095833350 is queued; the previous head’s successful check does not replace it. No new kernel build, hardware test, recording, default/loader or image action was performed. Preview/default and T6040/P010/forced-corruption hardware limits remain as previously stated.
Preserve all containment source and fixture blobs and union the current host-test registrations without dropping negative controls.
DjDeveloperr
left a comment
DjDeveloperr
left a comment
There was a problem hiding this comment.
Codex (GPT-6.1 Sol), reviewing on @DjDeveloperr’s behalf.
The reported conflict was transient: actual three-way registry composition is clean and preserves all main controls. The remaining CI run was stale in dispatch, queued with zero jobs since20:24 and no updated state for over2h. Newer host job130822398759 completed setup and tests with the exact same workflow blobfd665f94. GitHub nevertheless rejected both normal and documented force cancellation with409, stating the old run had not entered its queue; no tests or other runs were stopped.
Normally merged current main990e00b7 as8830cc51e065d95a644a4a5260d50a6125feca9e to obtain fresh validation of the genuine current-main composition. All15 ISP production/fixture blobs remain the reviewed9d/4415 versions; API comparison finds no ISP input change. All102 main registry values/order/controls plus the single same16-case ISP entry and existing FAIL-line kill pattern are preserved. No test/toolchain/permission setting changed, and no redundant local functional build/test was run.
Source approval stands for identical containment inputs; merge still waits for actual new-head host CI. #974 integration follows only after this PR merges. Existing physical/P010/preview limits are unchanged.
53b2fd1
into
aurora-silicon:integration/aurora
… changes Merge current integration/aurora after #1560 landed generic IPC containment. Keep all current firmware resident/idle/heap, P010, lease and return ownership fixes; resolve old shared-driver and fixture conflicts in favor of main. Retain only the exact J613/T8122 26.6.2 preview profile and two preview tests, their ownership lines and two runner entries. Preserve every main test and negative control, FAIL-line detection and the runner's executable permission. Original branch ancestry, authorship and preview opt-out remain unchanged. Both existing production-extraction preview tests pass ASan/UBSan on this seven-file residual. No new kernel/QEMU/hardware qualification is claimed. Preview/default, interval/restart/suspend and T6040/P010 gates remain open. Assisted-by: LLM
…r the merge The SEP's request-path source test from integration (#1590) wants rate-limited or debug messages throughout sep.rs. Five in this series' sleep path were plain: use dev_warn_ratelimited!, dev_err_ratelimited! and dev_info_ratelimited! for them. No logic changes. The last integration merge renamed the DART system-sleep callbacks that tools/asahi/j613-mtp-s2idle-tests.py extracts. Name them apple_dart_system_suspend() and apple_dart_system_resume() again, with the J613 live-MTP code inline as integration has it and this series' firmware-sleep phase choice for every other DART. Give the test stubs for pm_suspend_via_firmware() and the rtkit-helper AP notices, which a firmware-driven sleep alone reaches. The ISP host harness (#1560) didn't model this series' ISP state. Extract isp_cmd_send_chan() with isp_cmd_send(), and take the latch guard out of both for the overwrite control. Give the session fixture fw_restart, fw.started and isp_check_restarted(), and run its capture stop with the firmware running, as a capture only stops while it streams. Give the buffer fixture ISP_FW_RUNNING and clear_bit(). Every check and control stays as it was. Signed-off-by: Diljit Singh <hey@djdev.me>
Current integration
8830cc51e065d95a644a4a5260d50a6125feca9e. A normal merge of4415f9c7and main990e00b7preserves all 15 reviewed ISP source/fixture blobs byte-for-byte. The runner retains all 102 main registrations and controls plus the same 16 ISP cases in one entry; the existing failure-line negative-control detection is unchanged. No containment, preview/admission, format/interval or firmware-lifetime behavior changed in this follow-up.The previous host run never allocated a job after 20:24 UTC. GitHub refused both normal and force cancellation with HTTP409, saying the run had not entered its internal queue. A newer byte-identical host workflow completed successfully; this current-main integration requests fresh PR validation without a blank commit or changed test configuration. Actual new-head CI is required before merge.
The earlier functional integration at
9d26f34retains main’s resident/idle/heap restoration and the two regression-backed safe-session/failed-reuse fixes; the subsequent source is unchanged. Prior host/control evidence retains its exact input scope. Original physical/QEMU reports below predate this integration; no new hardware or full-kernel result is claimed. #974 preview and T6040/P010 qualification remain separate.GPT-6.1 Sol on @satchlj's behalf
Split generic IPC-failure containment out of #974 so it can be reviewed independently of J613 default preview qualification.
This series latches compact-protocol session failures, retains host ownership after uncertain buffer submissions, bounds receive turns, validates returned buffer identities/pools/duplicates, and wakes failed streaming queues. It includes the containment host fixtures, compiled negative controls and V4L2 KUnit coverage. The first five commits retain Satya's original authorship and author dates, with Aurora compatibility corrections folded into their introducing commits so intermediate commits remain testable. The existing P010 late-ACK resynchronization is preserved from the first commit.
No preview parameter/default, board/SoC/firmware admission, device tree, setfile, or normal format/interval selection changes. The generic IPC, submission/return, queue and shutdown paths benefit all boards; fault-path behaviour changes deliberately. Current Aurora firmware restoration and deferred camera detection are retained. T6040/P010 safety is not inferred from non-P010 host fixtures.
Validation on alba / isolated
neo-builddirectory, fresh integration base7cb581e81d0d6b6368af6d2bfb48ba115ab106aa:17efdf64f2e6a0f566be3f563fa3982170a6c78f; its changes since the tested base do not touch the ISP or host runner.Existing J613 streaming evidence applies to this containment code as part of the earlier combined head
f512cd7e432e1d1886a36e991ffe4928e4416b73, composed source112007b70271, not this new split head or an isolated PR kernel: 13 NV12 sizes, no frame-sequence gaps in 1170 frames at requested 30 fps; 20/20 short start/stop sessions; two sequential consumers. The original dark-room images did not qualify recognizable preview. A later separate daylight report (#974 (comment)) records Satya confirming a recognizable NV12 1920×1080 frame on package 30.12 /7.3.0-aurora73-ppt-2-ARCHcarryingf512cd7e432e. That is a narrow visual result, not broader image-quality qualification or a test of this split head. The documented stop-magic / explicit-stop fallback warnings remain; they are not clean firmware suspend. No forced runtime IPC failure was injected. This task ran no hardware tests.T6040/P010 hardware regression is untested: no such hardware here. Host and KUnit tests do not replace it.
The exact J613/T8122 26.6.2 profile/default preview and its hardware qualification gates remain in #974. Its published history is unchanged. After this series merges, that branch needs a normal merge of fresh integration with conflict resolution and a push, retaining only preview changes and its tests. No history rewrite is required.