Re: [PATCH net-next v5 08/15] idpf: refactor idpf to use libie_pci APIs

[Simon Horman <horms@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
https://sashiko.dev/#/patchset/20260715180042.1972010-2-anthony.l.nguyen@intel.com?part=8
---
--- Patch [8]: [PATCH net-next v5 08/15] idpf: refactor idpf to use libie_pci APIs ---
commit 131cbea6d4e2fb71f297aa7df605c1576a8704cf
Author: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
Subject: idpf: refactor idpf to use libie_pci APIs

This commit refactors the idpf driver to use the libie_pci initialization
and MMIO APIs. It updates register accesses across various components like
control queues and virtchnl messages to rely on the libie_ctlq_ctx
structure.

> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
> index c5e47e79a6411..92a120aadfcd2 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h

[ ... ]

> @@ -945,7 +884,8 @@ static inline bool idpf_is_reset_detected(struct idpf_adapter *adapter)
>  	if (!adapter->hw.arq)
>  		return true;
>  
> -	return !(readl(idpf_get_mbx_reg_addr(adapter, adapter->hw.arq->reg.len)) &
> +	return !(readl(libie_pci_get_mmio_addr(&adapter->ctlq_ctx.mmio_info,
> +					       adapter->hw.arq->reg.len)) &
>  		 adapter->hw.arq->reg.len_mask);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the transition to
libie_pci_get_mmio_addr() expose us to linked-list corruption here?

The old code performed an array access, but libie_pci_get_mmio_addr()
traverses the mmio_info->mmio_list. This traversal appears to be lockless
in asynchronous paths like idpf_is_reset_detected() in idpf.h and
idpf_ctlq_send() in idpf_controlq.c.

At the same time, driver teardown (idpf_remove) or hardware reset
(idpf_init_hard_reset) can call idpf_decfg_lan_memory_regions(), which
uses libie_pci_unmap_fltr_regs() to concurrently modify and free items
from this same list using list_del() and kvfree().

Since idpf_vc_xn_shutdown() doesn't wait for in-flight transactions
to drain, could a concurrent ethtool or interface operation preempt
during this list traversal and cause a use-after-free or list corruption?

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_controlq.h b/drivers/net/ethernet/intel/idpf/idpf_controlq.h
> index de4ece40c2ff3..acf595e9265f5 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_controlq.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_controlq.h
> @@ -109,8 +109,6 @@ struct idpf_mmio_reg {
>   * Align to ctlq_hw_info
>   */
>  struct idpf_hw {
> -	struct idpf_mmio_reg mbx;
> -	struct idpf_mmio_reg rstat;
>  	/* Array of remaining LAN BAR regions */
>  	int num_lan_regs;
>  	struct idpf_mmio_reg *lan_regs;

[Severity: Low]
Are these num_lan_regs and lan_regs fields still needed?

It looks like the new libie_pci API uses 
adapter->ctlq_ctx.mmio_info.mmio_list for LAN memory regions, and all
allocations and references to these array fields have been successfully
removed from the source files.