Re: [PATCH v3 05/10] mailbox: apple: Add DockChannel FIFO controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mailbox: apple: lost TX interrupt due to clearing IRQ_FLAG after transmission starts
- [High] mailbox: apple: TOCTOU race in IRQ handler clears newly asserted interrupts
- [Medium] mailbox: apple: interrupt storm when IRQ_TX fires but FIFO is not empty
--

commit 2a04249ed27a338883aa783b0cfe092f13b2057a
Author: Michael Reeves <michael.reeves077@gmail.com>

mailbox: apple: Add DockChannel FIFO controller

DockChannel is a hardware FIFO used by Apple coprocessors for
low-latency byte-stream communication with the AP. This adds a
mailbox controller that preallocates RX storage, tracks IRQ enable
state in software, and reports TX completion from the TX-empty
interrupt.

> diff --git a/drivers/mailbox/apple-dockchannel.c b/drivers/mailbox/apple-dockchannel.c
> index 0000000000000..a99b8dad03987
> --- /dev/null
> +++ b/drivers/mailbox/apple-dockchannel.c
[ ... ]
> +static int apple_dockchannel_send_data(struct mbox_chan *chan, void *data)
> +{
[ ... ]
> +	dc->tx_buf = msg->data;
> +	dc->tx_len = msg->len;
> +	dc->tx_pos = 0;
> +	dc->tx_active = true;
> +
> +	apple_dockchannel_write_pending(dc);
> +	writel_relaxed(APPLE_DOCKCHANNEL_FIFO_SIZE,
> +		       dc->config_base + CONFIG_TX_THRESH);
> +	apple_dockchannel_irq_enable(dc, IRQ_TX);

[Severity: High]
Does this create a race condition where a valid completion interrupt
is cleared?

The transmission is initiated by writing to the FIFO in
apple_dockchannel_write_pending(). The hardware can transmit the bytes
and become empty extremely fast. If it does so before the call to
apple_dockchannel_irq_enable() executes, the hardware latches the IRQ_TX
completion flag, and apple_dockchannel_irq_enable() immediately clears
it, potentially permanently hanging the channel.

Could the stale flags be cleared prior to writing the FIFO?

[ ... ]
> +static irqreturn_t apple_dockchannel_irq_thread(int irq, void *data)
> +{
> +	struct apple_dockchannel *dc = data;
> +	unsigned long irqflags;
> +	u32 flags;
> +	u32 pending;
> +	bool tx_done = false;
> +
> +	flags = readl_relaxed(dc->irq_base + IRQ_FLAG);
> +
> +	spin_lock_irqsave(&dc->lock, irqflags);
> +
> +	pending = flags & dc->irq_mask & (IRQ_TX | IRQ_RX);
> +	if (!pending)
> +		goto out_unlock_none;
> +
> +	if (pending & IRQ_TX) {
> +		if (apple_dockchannel_tx_empty(dc)) {
> +			apple_dockchannel_irq_disable(dc, IRQ_TX);
> +			tx_done = dc->tx_active;
> +			dc->tx_active = false;
> +		} else {
> +			pending &= ~IRQ_TX;
> +		}
> +	}
> +
> +	writel_relaxed(pending, dc->irq_base + IRQ_FLAG);

[Severity: Medium]
Will this lead to an interrupt storm if the TX interrupt fires but the
FIFO is not empty?

Since IRQ_FLAG is a write-1-to-clear register, removing IRQ_TX from
pending intentionally skips clearing the active hardware interrupt in
this case. The handler returns while the unmasked interrupt is still
asserted, causing the IRQ controller to immediately re-fire the interrupt.

While this may require a spurious hardware interrupt or glitch to trigger,
it would result in an infinite interrupt loop that permanently hangs a
CPU core.

[ ... ]
> +out_unlock_none:
> +	spin_lock_irqrestore(&dc->lock, irqflags);
> +
> +	if (flags & (IRQ_TX | IRQ_RX))
> +		writel_relaxed(flags & (IRQ_TX | IRQ_RX),
> +			       dc->irq_base + IRQ_FLAG);

[Severity: High]
Can this write back a stale flags value, clearing newly asserted interrupts?

The flags value is read outside the lock at the beginning of the handler.
If the handler takes the out_unlock_none path (e.g., due to a concurrent
shutdown making pending == 0), it will write this stale flags value to
IRQ_FLAG locklessly. 

If another CPU executes apple_dockchannel_send_data() and completes a
transmission in the intervening window, this lockless write-1-to-clear
will wipe the newly generated IRQ_TX interrupt, hanging the mailbox channel.

> +
> +	return IRQ_NONE;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-apple-mtp-keyboard-final-v3-0-e356464e454b@gmail.com?part=5