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

[sashiko-bot@kernel.org]

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.

> +
> +	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.

> +		if (!irq_grp->napi_ndev) {
> +			ret = -ENOMEM;
> +			goto fail_allocate;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-mpp_bringup-v1-0-2c5797b5e4ac@oss.qualcomm.com?part=12