Re: [PATCH net-next v5 13/15] ixd: add reset checks and initialize the mailbox

[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=13
---
--- Patch [13]: [PATCH net-next v5 13/15] ixd: add reset checks and initialize the mailbox ---
commit 9019dd0f9c2d33f542f42c411b53c595e9c03614
Author: Larysa Zaremba <larysa.zaremba@intel.com>
Subject: ixd: add reset checks and initialize the mailbox

This commit introduces the reset and mailbox initialization sequence for the
Intel CPF device driver. It adds functions to trigger and verify hardware
resets, initializes default control queues, and sets up a delayed work task to
handle the post-reset initialization flow. It also adds cleanup logic to the
device removal path.

> diff --git a/drivers/net/ethernet/intel/ixd/ixd_dev.c b/drivers/net/ethernet/intel/ixd/ixd_dev.c
> new file mode 100644
> index 0000000000000..cdd5477cc1f4d
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ixd/ixd_dev.c
[ ... ]
> +void ixd_trigger_reset(struct ixd_adapter *adapter)
> +{
> +	void __iomem *addr;
> +	u32 reg_val;
> +
> +	addr = libie_pci_get_mmio_addr(&adapter->cp_ctx.mmio_info,
> +				       ixd_reset_reg.rtrigger);
> +	reg_val = readl(addr);
> +	writel(reg_val | ixd_reset_reg.rtrigger_m, addr);

[Severity: Medium]
If the device has been hot-removed or is experiencing a PCIe link error,
could readl() return 0xFFFFFFFF?

If reg_val is not checked for 0xFFFFFFFF, this will write all 1s back to
the PFGEN_CTRL register. Could this trigger unintended hardware actions or
test modes if the link recovers or the write completes?

> +}
[ ... ]
> diff --git a/drivers/net/ethernet/intel/ixd/ixd_main.c b/drivers/net/ethernet/intel/ixd/ixd_main.c
> index 75ee53152e614..35d0ff5c8d983 100644
> --- a/drivers/net/ethernet/intel/ixd/ixd_main.c
> +++ b/drivers/net/ethernet/intel/ixd/ixd_main.c
[ ... ]
> @@ -16,7 +17,15 @@ static void ixd_remove(struct pci_dev *pdev)
>  {
>  	struct ixd_adapter *adapter = pci_get_drvdata(pdev);
>  
> -	libie_pci_unmap_all_mmio_regions(&adapter->hw);
> +	/* Do not mix removal with (re)initialization */
> +	cancel_delayed_work_sync(&adapter->init_task.init_work);
> +	/* Leave the device clean on exit */
> +	if (adapter->xnm)
> +		libie_ctlq_xn_shutdown(adapter->xnm);
> +	ixd_trigger_reset(adapter);
> +	ixd_deinit_dflt_mbx(adapter);

[Severity: High]
Since ixd_trigger_reset() uses a posted write (writel) without a following
readl() to flush it, could the reset fail to reach the hardware before the
PCI subsystem disables Memory Space Enable?

Additionally, ixd_deinit_dflt_mbx() is called immediately after triggering
the reset. Does this free the control queue DMA memory before the hardware
is guaranteed to have halted? If the reset hasn't taken effect yet, could
the device continue to master DMA to the freed memory and cause IOMMU DMAR
faults or memory corruption?

> +
> +	libie_pci_unmap_all_mmio_regions(&adapter->cp_ctx.mmio_info);
>  }