Re: [PATCH net-next v5 09/15] idpf: refactor idpf to use libie control queues
From: Simon Horman <horms@kernel.org>
Date: 2026-07-22 16:16:23
Also in:
linux-doc
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=9 ---
--- Patch [9]: [PATCH net-next v5 09/15] idpf: refactor idpf to use libie control queues ---commit 06a0b55dccd2a8bd004309d725e04b4253d24231 Author: Pavan Kumar Linga [off-list ref] Subject: idpf: refactor idpf to use libie control queues This commit refactors the idpf driver to use the libie control queue APIs for initializing, configuring, and managing control queue transactions. It replaces the local control queue implementation with libie equivalents, which introduces DMA mapping of send buffers rather than performing memory copies.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
[ ... ]
-static int idpf_mb_clean(struct idpf_adapter *adapter,
- struct idpf_ctlq_info *asq)
+static void idpf_mb_clean(struct idpf_adapter *adapter,
+ struct libie_ctlq_info *asq, bool deinit)
{
- u16 i, num_q_msg = IDPF_DFLT_MBX_Q_LEN;
- struct idpf_ctlq_msg **q_msg;
- struct idpf_dma_mem *dma_mem;
- int err;
-
- q_msg = kzalloc_objs(struct idpf_ctlq_msg *, num_q_msg, GFP_ATOMIC);
- if (!q_msg)
- return -ENOMEM;
-
- err = idpf_ctlq_clean_sq(asq, &num_q_msg, q_msg);
- if (err)
- goto err_kfree;
-
- for (i = 0; i < num_q_msg; i++) {
- if (!q_msg[i])
- continue;
- dma_mem = q_msg[i]->ctx.indirect.payload;
- if (dma_mem)
- dma_free_coherent(&adapter->pdev->dev, dma_mem->size,
- dma_mem->va, dma_mem->pa);
- kfree(q_msg[i]);
- kfree(dma_mem);
- }
-
-err_kfree:
- kfree(q_msg);
+ struct libie_ctlq_xn_clean_params clean_params = {
+ .ctlq = asq,
+ .rel_tx_buf = kfree,
+ .num_msgs = IDPF_DFLT_MBX_Q_LEN,
+ .force = deinit,
+ };
- return err;
+ libie_ctlq_xn_send_clean(&clean_params);
}[Severity: Medium] Does setting force = deinit here cause a memory leak when encountering zero-length messages? If a zero-length payload message (like a VF reset message) is enqueued followed by a non-zero length message, libie_ctlq_xn_send_clean evaluates the loop break condition on the zero-length payload. If the condition !(params->force && msg->data_len) evaluates to true due to data_len being 0, the cleanup loop terminates early. Could this cause any subsequent pending messages in the queue to be skipped, leaking their DMA mappings during a teardown path? [ ... ]
+void idpf_send_vf_reset_msg(struct idpf_adapter *adapter)
+{
+ struct libie_ctlq_info *ctlq = adapter->asq;
+
+ /* Forcefully claim send queue slot */
+ idpf_mb_clean(adapter, ctlq, true);[Severity: High] Could this forceful cleanup unmap DMA buffers that the hardware is still actively reading? During driver unload or VF reset, the hardware is still active since the reset message hasn't been sent yet. Calling idpf_mb_clean with deinit=true bypasses the hardware Done check for non-empty messages and immediately unmaps their DMA buffers. Can this lead to a DMA use-after-free and trigger IOMMU faults if the hardware attempts to read the unmapped buffers?
+
+ scoped_guard(spinlock, &ctlq->lock) {
+ *ctlq->tx_msg[ctlq->next_to_use] = (struct libie_ctlq_msg) {
+ .opcode = LIBIE_CTLQ_SEND_MSG_TO_CP,
+ .chnl_opcode = VIRTCHNL2_OP_RESET_VF,
+ };
+
+ libie_ctlq_send(adapter->asq, 1);
+ }
+}