On Tue, 15 Sep 2026 17:04:17 +0530
Prashant Gupta <[email protected]> wrote:
> This series is the first of four that upstream the missing NXP dpaa2
> driver changes. It collects the foundational bus/fslmc, dma/dpaa2 and
> mempool/dpaa2 fixes that the later series build on:
>
> - defer fslmc bus initialization to probe and reduce probe-time logging
> and MC traffic,
> - fix an array-bounds warning and validate IOVA in the dpaa2 QDMA
> pre-populate helpers,
> - support fetching the mempool ops index from the primary process in a
> secondary process.
>
> Every commit builds cleanly (including the aarch64 DPAA cross build with
> -Werror) and the series is bisectable.
>
> Gagandeep Singh (1):
> dma/dpaa2: validate IOVA in pre-populate helpers
>
> Hemant Agrawal (1):
> bus/fslmc: reduce probe-time logging and MC traffic
>
> Jun Yang (2):
> dma/dpaa2: fix array-bounds warning in dequeue path
> mempool/dpaa2: support ops index from primary in secondary
>
> Prashant Gupta (1):
> bus/fslmc: defer bus initialization to probe
>
> drivers/bus/fslmc/fslmc_bus.c | 92 +++++++++++++----------
> drivers/bus/fslmc/fslmc_vfio.c | 3 +-
> drivers/dma/dpaa2/dpaa2_qdma.c | 96 ++++++++++++++++--------
> drivers/mempool/dpaa2/dpaa2_hw_mempool.c | 95 ++++++++++++++++++++++-
> 4 files changed, 209 insertions(+), 77 deletions(-)
>
Detailed AI review finds errors
Review: [PATCH v3-S1 0/5] NXP fslmc/dpaa2 fixes
Base: main f43632a (26.11.0-rc0)
Series applies cleanly. Per-commit build of bus/fslmc, mempool/dpaa2,
dma/dpaa2 with -Dwerror=true passes at every commit.
Series summary
--------------
Patch 5 does not work: the IPC reply is parsed from the wrong offset,
so every secondary lookup fails, and the lookup sits in the dpaa2_sec
per-op enqueue path. Patch 3's commit message describes a change that
is not in the diff. Patch 1 reverts only half of cdefd2e980bd; the
DPAA bus has the same problem.
Patch 1/5: bus/fslmc: defer bus initialization to probe
-------------------------------------------------------
Warning:
cdefd2e980bd moved init into scan for both NXP buses. Only fslmc is
restored here. rte_dpaa_bus_scan() still calls
rte_mbuf_set_platform_mempool_ops(), which does
rte_memzone_reserve(), and dpaax_iova_table_populate(), which does
rte_zmalloc(). EAL runs rte_bus_scan() (eal.c:680) before
rte_eal_memzone_init() (784) and rte_eal_malloc_heap_init() (802), so
bus/dpaa is broken the same way. Fix both in this series, or say in
the commit message why dpaa is unaffected.
Info:
rte_fslmc_probe() returns 0 on every init failure and skips
rte_bus_generic_probe(), so a VFIO or DMA map failure results in no
devices and a zero return from rte_bus_probe(). This is the
pre-cdefd2e980bd behaviour, but now that the function is being
rewritten, returning ret would let EAL init fail visibly.
Patch 2/5: bus/fslmc: reduce probe-time logging and MC traffic
--------------------------------------------------------------
Warning:
The subject says "and MC traffic" but the patch only changes one log
level. Either drop that part of the subject or include the MC change.
Also fslmc_map_dma() is called from the memory hotplug callback, not
just at probe time.
Patch 3/5: dma/dpaa2: fix array-bounds warning in dequeue path
--------------------------------------------------------------
Error:
Commit message does not match the patch. It says an
idxs[DPAA2_QDMA_MAX_DESC] scratch buffer is added to struct
qdma_virt_queue and idxs[0] is used instead of &idx. Neither happens:
dpaa2_qdma_dq_fd() still passes &idx, and dpaa2_qdma.h is untouched.
The real change is rewriting qdma_cntx_idx_ring_eq() from a per-element
loop to a two-segment copy. Describe that, and state which compiler,
version and target emit the warning, since a Cc: stable fix for a
warning needs to be reproducible.
Warning:
New code uses rte_memcpy() for small variable-length copies of
uint16_t. Use memcpy(); rte_memcpy is being removed from non-datapath
and small-copy users tree-wide.
Info:
Merging the LONG/SG fle_sdd handling is an unrelated cleanup inside a
stable backport. Split it out, or drop it from the fix.
Patch 4/5: dma/dpaa2: validate IOVA in pre-populate helpers
-----------------------------------------------------------
Warning:
The check runs in the enqueue path. On failure fle[SRC].length stays
zero, so every subsequent copy on that object re-runs
rte_fslmc_cold_mem_vaddr_to_iova() plus rte_mem_virt2iova() and emits
DPAA2_QDMA_ERR, flooding the log at packet rate. fle_pool is created
by the driver in dpaa2_qdma_vchan_setup(); validate the pool mapping
once there (e.g. rte_mempool_mem_iter over mp chunks) and fail
vchan_setup, instead of checking per object on first use.
Patch 5/5: mempool/dpaa2: support ops index from primary in secondary
---------------------------------------------------------------------
Error:
Reply parsed at the wrong offset:
rsp_msg = (void *)mp_reply.msgs;
mp_reply.msgs is struct rte_mp_msg *, whose first member is name[].
msg_type is read from the bytes "dp" of "dpaa2_pool_mp_sync", never
matches DPAA2_POOL_OPS_IDX_RSP, and every request fails with "received
invalid response". Must be:
rsp_msg = (void *)mp_reply.msgs[0].param;
Also check mp_reply.nb_received == 1 rather than msgs != NULL, and
check len_param before copying msg_data. This path has clearly not
been exercised.
Error:
rte_dpaa2_mpool_get_ops_idx() is called per op in
dpaa2_sec_enqueue_burst() and the ordered variant
(dpaa2_sec_dpseci.c:1544, 1903). In a secondary, whenever the lookup
fails, s_dpaa2_pool_ops_idx stays RTE_MEMPOOL_MAX_OPS_IDX and the next
op issues another rte_mp_request_sync() with a 5 s timeout. That
happens always with the bug above, and also whenever the primary has
not created a dpaa2 pool (no action registered, primary replies
MP_IGN, nb_received is 0). A blocking IPC must not be reachable from
a datapath function; resolve once at init and cache the result,
including failure.
Warning:
The IPC is unnecessary. The mempool ops_index stored in the shared
struct rte_mempool is only valid because primary and secondary
register ops in the same order; the secondary already dispatches
through its own rte_mempool_ops_table with that index. The secondary
can find its own index by scanning rte_mempool_ops_table for
DPAA2_MEMPOOL_OPS_NAME, which removes the handler, the message
protocol and the timeout entirely.
Warning:
Missing Fixes: tag. The commit message describes secondaries failing
due to the unset index.
Info:
dpaa2_mbuf_pool_mp_primary() returns -ENOTSUP on an unknown type
without replying, so the requester waits the full timeout. The action
is never unregistered.
Commit message: rte_mp_action_register() returns ENOTSUP when IPC is
disabled at runtime (--no-shconf / --in-memory), not due to a build
time option.
