Skip to content

soc: apple: mailbox: keep enable_irq() balanced when a send times out - #599

Open
brentkearney wants to merge 1 commit into
AsahiLinux:asahi-wipfrom
brentkearney:mailbox-tx-irq-balance
Open

soc: apple: mailbox: keep enable_irq() balanced when a send times out#599
brentkearney wants to merge 1 commit into
AsahiLinux:asahi-wipfrom
brentkearney:mailbox-tx-irq-balance

Conversation

@brentkearney

@brentkearney brentkearney commented Sep 4, 2026

Copy link
Copy Markdown

Balance the send-empty interrupt enable in apple_mbox_send(), so a send whose wait times out or takes a signal no longer leaves the interrupt enabled. Fixes #598.

Problem

  • apple_mbox_send() enables irq_send_empty before waiting for the A2I FIFO to drain; only apple_mbox_send_empty_irq() disables it again.
  • The timeout and signal returns skip that disable, so the interrupt's disable depth is off by one.
  • The next send that finds the FIFO full calls enable_irq() on an already-enabled interrupt, so __enable_irq() warns and underflows the depth counter. Every further stalled send warns again with a backtrace.
  • Both paths are reachable in normal operation, and every RTKit coprocessor on these machines uses this function: sep, dcp, dcpext, aop, isp, and nvme.
  • The same code is in mainline, so the patch applies there unchanged apart from the header include path.

Changes

  • Add tx_irq_enabled to struct apple_mbox, guarded by tx_lock.
  • Enable the interrupt only when it is not already enabled.
  • Clear the flag in the interrupt handler, where it already calls disable_irq_nosync().
  • Disable the interrupt on the timeout and signal paths when the handler never ran.

Notes for reviewers

  • The flag settles the race where the handler runs just after the wait expires: whichever side takes tx_lock first performs the single disable.
  • A completion left signaled by that handler is cleared by the existing reinit_completion() before the next wait, so no send returns early on a stale completion.
  • The failure path holds tx_lock, so it calls disable_irq_nosync(); disable_irq() can sleep.

Testing

Runtime-tested on a MacBook Pro 16-inch 2021 (apple,j316s / apple,t6000, M1 Pro). The stall must be provoked from a healthy SEP: the message that makes SEPOS stop draining only has that effect early in a boot. Later SEPOS reaches a state where it stops answering but keeps draining, so the FIFO never fills and the timeout path is never taken.

  • Before, on 7.1.6-1-4-ARCH: eight sends timed out and the last seven each printed Unbalanced enable for IRQ 57 with a backtrace through apple_mbox_send+0xb4/0x1b8. Only the first timed-out send is silent, matching the analysis.
  • After, on 7.1.6-1-5-ARCH: eight sends filled the A2I FIFO and the next 56 timed out, with zero Unbalanced enable WARNs.
  • After: no WARNING line anywhere in the boot, and taint stayed at 4 (arm64 S baseline, W clear).
  • Display, audio, NVMe, and the SEP boot handshake behave normally across the boot.

…fails

apple_mbox_send() enables irq_send_empty and waits for the coprocessor to
drain the A2I FIFO. Only the interrupt handler disables that interrupt
again, so when the wait does not complete -- the FIFO stays full for
APPLE_MBOX_TX_TIMEOUT, or the wait is interrupted by a signal -- the
interrupt is left enabled and its disable depth is off by one.

The next send that finds the FIFO full then calls enable_irq() on an
already-enabled interrupt, which warns and underflows the depth counter:

  Unbalanced enable for IRQ 57
  WARNING: kernel/irq/manage.c:774 at __enable_irq+0x4c/0x80, CPU#0
  Call trace:
   __enable_irq+0x4c/0x80 (P)
   enable_irq+0x74/0xe4
   apple_mbox_send+0xb4/0x1b8

Observed on an M1 Pro (apple,t6000) after SEPOS stopped draining its
mailbox: of eight consecutive sends that timed out, the last seven each
warned with a full backtrace.

Track whether the interrupt is enabled in struct apple_mbox under tx_lock
and disable it on the failure paths. The flag also settles the race with
the handler running just after the wait expired: whichever side takes
tx_lock first performs the single disable, and the stale completion is
cleared by the reinit_completion() before the next wait.

Signed-off-by: Brent <1550934+brentkearney@users.noreply.github.com>
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.

1 participant