Re: [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line

[Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>]

On 8/24/2026 1:16 PM, Marc Kleine-Budde wrote:
> On 24.08.2026 18:12:41, Bough Chen wrote:
>> On Mon, Jul 13, 2026 at 09:08:03AM +0000, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>>
>>> Pre-existing issues:
>>> - [High] Concurrent execution of multiple flexcan IRQ handlers causes lockless list corruption in the can_rx_offload queue.
>>> --
>>>
>>> --- Patch [3]: [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line ---
>>> commit 9f143e05fb5957d463ce1d05d4126d168798b0d8
>>> Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>>
>>> can: flexcan: split rx/tx masks per mailbox IRQ line
>>>
>>> This commit updates flexcan to process split rx/tx masks based on the
>>> mailbox IRQ line for platforms like S32G2. It introduces MB0 and MB1 masks
>>> and passes them to flexcan_do_mb() to only process the relevant mailbox
>>> range for each IRQ.
>>>
>>>> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
>>>> --- a/drivers/net/can/flexcan/flexcan-core.c
>>>> +++ b/drivers/net/can/flexcan/flexcan-core.c
>>>
>>> [ ... ]
>>>
>>>> @@ -1244,8 +1255,14 @@ static irqreturn_t flexcan_irq_mb(int irq, void *dev_id)
>>>>   	struct net_device *dev = dev_id;
>>>>   	struct flexcan_priv *priv = netdev_priv(dev);
>>>>   	irqreturn_t handled;
>>>> +	u64 mb_mask = ~0ULL;
>>>> +
>>>> +	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
>>>> +		mb_mask = (irq == priv->irq_secondary_mb) ?
>>>> +			  FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK :
>>>> +			  FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK;
>>>>
>>>> -	handled = flexcan_do_mb(dev);
>>>> +	handled = flexcan_do_mb(dev, mb_mask);
>>>>
>>>>   	if (handled)
>>>>   		can_rx_offload_irq_finish(&priv->offload);
>>>
>>> [Severity: High]
>>> This is a pre-existing issue, but does this concurrent execution risk
>>> corrupting the lockless rx-offload queues?
>>>
>>> Since the primary and secondary mailbox IRQs are separate lines (e.g., on
>>> S32G2), they can be dispatched concurrently on different CPUs when frames
>>> arrive simultaneously for both mailbox groups.
>>>
>>> If both IRQs execute at the same time:
>>>
>>> CPU1
>>> flexcan_irq_mb()
>>>     flexcan_do_mb()
>>>        can_rx_offload_irq_offload_timestamp()
>>>           __skb_queue_add_sort(&offload->skb_irq_queue, ...)
>>>
>>> CPU2
>>> flexcan_irq_mb()
>>>     flexcan_do_mb()
>>>        can_rx_offload_irq_offload_timestamp()
>>>           __skb_queue_add_sort(&offload->skb_irq_queue, ...)
>>>
>>> Or if CPU2 reaches the finish step while CPU1 is still adding:
>>>
>>> CPU2
>>> flexcan_irq_mb()
>>>     can_rx_offload_irq_finish()
>>>        skb_queue_splice_tail_init(&offload->skb_irq_queue, ...)
>>>
>>> Since __skb_queue_add_sort() and skb_queue_splice_tail_init() operate
>>> locklessly on offload->skb_irq_queue, could this lead to corrupted linked
>>> list next/prev pointers or infinite loops during list traversal?
>>
>> Hi Ciprian Costea,
>>
>> Please double check whether S32 can generate these two mb interrupt at
>> the same time, if not, no need change, just comment this feature, if
>> yes, then need to add lock.
> 
> If the IRQ is delayed for some reason, mailboxes 7 (associated with
> mb-0) and 8 (associated with mb-1) might be pending at the same time.
> 
> I assume that both IRQs will fire. One workaround would be to only use
> the mb-1 IRQ until the rx-offloading has been fixed.
> 
> regards,
> Marc
> 

Hello Haibo, Marc,

You're both right. On S32G2 the mb-0 and mb-1 lines are independent and
can fire concurrently if one handler is delayed (Marc's MB7/MB8
example).

For V7 I'll implement Marc's suggestion and use only the mb-1 line on
S32G2: move the RX-offload window into the MB 8–63 group and register
enable only the secondary MB interrupt.
This keeps everything on a single hardirq context and avoids the race.

The underlying limitation is the lockless skb_irq_queue in the rx
offload core. I'll address that in a separate patchset. Once it's fixed, 
this workaround can be dropped and the second MB line re-enabled.

Best regards,
Ciprian