Re: [PATCH net-next v5 14/15] ixd: add the core initialization

[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=14
---
--- Patch [14]: [PATCH net-next v5 14/15] ixd: add the core initialization ---
commit 43763d63c75ee5bcdd9189c20cf8b272997bdfec
Author: Larysa Zaremba <larysa.zaremba@intel.com>

ixd: add the core initialization

This patch initializes the core when the mailbox is set up. The driver
confirms the virtchnl version, requests necessary capabilities, and sets up
the message handling framework.

> diff --git a/drivers/net/ethernet/intel/ixd/ixd_ctlq.c b/drivers/net/ethernet/intel/ixd/ixd_ctlq.c
> index 0000000000000..6d658be0695d4
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ixd/ixd_ctlq.c

[ ... ]

> @@ -0,0 +1,146 @@

[ ... ]

> +void ixd_ctlq_rx_task(struct work_struct *work)
> +{
> +	struct ixd_adapter *adapter;
> +
> +	adapter = container_of(work, struct ixd_adapter, mbx_task.work);
> +
> +	queue_delayed_work(system_unbound_wq, &adapter->mbx_task,
> +			   msecs_to_jiffies(300));
> +
> +	ixd_ctlq_recv_mb_msg(adapter);
> +}

[Severity: High]
Does queueing the delayed work before executing the main logic create a race
condition? If cancel_delayed_work_sync() is called (for example, during
teardown in ixd_deinit_dflt_mbx()), try_to_grab_pending() could successfully
delete the newly queued timer and return immediately, assuming the work is
fully cancelled.

This would bypass flush_work() and allow the teardown path to free
adapter->xnm and set adapter->arq to NULL while ixd_ctlq_recv_mb_msg() is
still running, potentially causing a use-after-free or NULL pointer
dereference. Should the re-arming be moved to the end of the task?

> diff --git a/drivers/net/ethernet/intel/ixd/ixd_lib.c b/drivers/net/ethernet/intel/ixd/ixd_lib.c
> index afc413d3650fb..ce870aba143d4 100644
> --- a/drivers/net/ethernet/intel/ixd/ixd_lib.c
> +++ b/drivers/net/ethernet/intel/ixd/ixd_lib.c

[ ... ]

> @@ -136,8 +145,27 @@ void ixd_init_task(struct work_struct *work)

[ ... ]

> +	libie_ctlq_xn_shutdown(adapter->xnm);
> +	ixd_trigger_reset(adapter);
> +	ixd_deinit_dflt_mbx(adapter);

[Severity: High]
Is it safe to trigger a hardware reset before cancelling the mailbox polling
task? ixd_deinit_dflt_mbx() is responsible for cancelling the mbx_task, but
calling ixd_trigger_reset() first means the polling task could run while the
hardware is resetting.

During reset, MMIO reads or DMA ring memory might temporarily contain invalid
states, and the still-running mbx_task might process corrupted descriptors.
Should the polling task be cancelled before asserting the hardware reset here,
as well as in ixd_remove() in ixd_main.c?