Re: [PATCH ath-next 12/17] wifi: ath12k: Enable MSI interrupt support for DP interrupt

[Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>]

On 10/8/2026 4:15 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] ath12k: leaked IRQ disable depth when NAPI poll is aborted
> - [Medium] ath12k: NAPI netdev and MSI IRQs leak on AHB probe failure
> --
> 
> commit 05cbf53b4f5fb7b0ba611762ab10f0a7205ad37a
> Author: Sowmiya Sree Elavalagan <quic_ssreeela@quicinc.com>
> 
> wifi: ath12k: Enable MSI interrupt support for DP interrupt
>     
> Configure and request external MSI interrupts for DP ring groups on the
> QCN6432 platform. Create a NAPI context for each active external interrupt
> group and disable the corresponding IRQs while NAPI polling is in progress.
> 
>> diff --git a/drivers/net/wireless/ath/ath12k/pcic.c b/drivers/net/wireless/ath/ath12k/pcic.c
>> index fea284c3707ba..06178b4f20d17 100644
>> --- a/drivers/net/wireless/ath/ath12k/pcic.c
>> +++ b/drivers/net/wireless/ath/ath12k/pcic.c
>> @@ -181,6 +181,112 @@ static irqreturn_t ath12k_pcic_ce_interrupt_handler(int irq, void *arg)
>>  	return IRQ_HANDLED;
>>  }
>>  
>> +static void ath12k_pcic_ext_grp_enable(struct ath12k_ext_irq_grp *irq_grp)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < irq_grp->num_irq; i++)
>> +		enable_irq(irq_grp->ab->irq_num[irq_grp->irqs[i]]);
>> +}
>> +
>> +static int ath12k_pcic_ext_grp_napi_poll(struct napi_struct *napi, int budget)
>> +{
>> +	struct ath12k_ext_irq_grp *irq_grp = container_of(napi,
>> +						struct ath12k_ext_irq_grp,
>> +						napi);
>> +	struct ath12k_base *ab = irq_grp->ab;
>> +	int work_done;
>> +
>> +	work_done = ath12k_dp_service_srng(ab->dp, irq_grp, budget);
>> +	if (work_done < budget) {
>> +		if (likely(napi_complete_done(napi, work_done)))
>> +			ath12k_pcic_ext_grp_enable(irq_grp);
>> +	}
> 
> [Severity: Medium]
> Can this leak the OS-level IRQ disable depth?
> 
> If napi_disable() interrupts a scheduled NAPI instance before it runs, or if
> the poll uses its full budget, the networking core (__napi_poll in
> net/core/dev.c) observes napi_disable_pending() and calls napi_complete()
> directly without scheduling the driver's custom poll loop again.
> 
> Because the hardware IRQ handler (ath12k_pci_ext_interrupt_handler) uses
> disable_irq_nosync() instead of masking the interrupt via device registers,
> this skips the paired enable_irq() call here, leaving the OS-level IRQ disable
> depth unbalanced.
> 
> The driver's teardown path (ath12k_pci_ext_irq_disable) further increments
> the depth by blindly calling disable_irq_nosync(). Upon resume or
> re-enablement, enable_irq() will only decrement the leaked depth by 1,
> leaving the IRQ physically disabled permanently.
> 

This is handled by the ATH12K_FLAG_EXT_IRQ_ENABLED flag. The IRQ handler checks this flag
before disabling the IRQ and scheduling NAPI. During teardown, this flag is cleared using
test_and_clear_bit(), so subsequent IRQ handler calls will return without disabling the
IRQ or scheduling NAPI.

>> +
>> +	if (work_done > budget)
>> +		work_done = budget;
>> +
>> +	return work_done;
>> +}
>> +
>> +static int ath12k_pcic_ext_irq_config(struct ath12k_base *ab,
>> +				      struct msi_desc **msi_desc, int num_msi)
>> +{
>> +	struct ath12k_ahb *ab_ahb = ath12k_ab_to_ahb(ab);
>> +	struct ath12k_ext_irq_grp *irq_grp;
>> +	int j, i, k, vec_idx = 0, ret = 0;
>> +	u8 userpd_id = ab_ahb->userpd_id;
>> +	u32 num_irq, base_idx;
>> +	struct msi_desc *desc;
>> +
>> +	base_idx = ATH12K_MSI_IRQ_CE0_OFFSET + CE_COUNT_MAX;
>> +
>> +	for (i = 0; i < ATH12K_EXT_IRQ_GRP_NUM_MAX; i++) {
>> +		num_irq = 0;
>> +		desc = msi_desc[vec_idx];
>> +		irq_grp = &ab->ext_irq_grp[i];
>> +		irq_grp->ab = ab;
>> +		irq_grp->grp_id = i;
>> +
>> +		if (ab->hw_params->ring_mask->tx[i] ||
>> +		    ab->hw_params->ring_mask->rx[i] ||
>> +		    ab->hw_params->ring_mask->rx_err[i] ||
>> +		    ab->hw_params->ring_mask->rx_wbm_rel[i] ||
>> +		    ab->hw_params->ring_mask->reo_status[i] ||
>> +		    ab->hw_params->ring_mask->host2rxdma[i] ||
>> +		    ab->hw_params->ring_mask->rx_mon_dest[i]) {
>> +			num_irq = 1;
>> +		}
>> +
>> +		if (!num_irq)
>> +			continue;
>> +
>> +		irq_grp->napi_ndev = alloc_netdev_dummy(0);
> 
> [Severity: Medium]
> Does this dummy netdev and the associated MSI IRQs leak if probe fails?
> 
> For hybrid bus devices (e.g., QCN6432), ath12k_ahb_probe() delegates IRQ
> configuration to ath12k_pcic_config_irq(), which explicitly allocates memory
> for dummy NAPI netdevs (alloc_netdev_dummy()) and manually requests MSIs
> without using devm_ variants.
> 
> If ath12k_ahb_probe() fails in subsequent initialization steps (such as
> arch_init or ath12k_core_init), the error handling paths jump to labels
> (err_deinit_arch, err_rproc_deconfigure) that appear to skip releasing these
> resources by not calling ath12k_pcic_free_irq() or ath12k_ahb_disable_irq(),
> leading to a permanent memory and IRQ leak on probe failure.
> 

Sure, I will address this in the next version.
>> +		if (!irq_grp->napi_ndev) {
>> +			ret = -ENOMEM;
>> +			goto fail_allocate;
>> +		}
>