On Wed, 4 Nov 2026 16:49:48 +0530
Raghavendra Ningoji <[email protected]> wrote:
> This series addresses the post-merge review of the AMD NTB support
> by Stephen. Some of the issues also exist in the Intel code and in
> released versions; those patches are tagged for stable.
>
> - Patches 1-4 fix the type confusion in rte_pmd_ntb_get_mem_align(),
> the unbounded peer memory window count, the off-by-one in the
> address remapping and the memory window leaks.
>
> - Patch 5 fixes the doorbell interrupts with vfio-pci: only MSI-X
> vector 0 (DB0) was serviced, so the DB1/DB2 exchange of dev_stop
> never completed and the stop waited for its 1s timeout.
>
> - Patch 7 removes the AMD hardware event handling, which could not run
> as wired, and documents that hardware events are not handled.
>
> - Patches 8-9 fix the Rx path: a partially consumed packet was leaked
> and desynchronized the ring, and an mbuf owned by the application
> could be freed again on device stop.
>
> - The other patches are documentation and cleanup.
>
> The peer memory window size check suggested for the AMD handshake is
> not added: all the AMD scratchpads are in use, and the window limit
> register programmed with the local memzone length already bounds what
> the peer can reach.
>
> Tested with vfio-pci on two AMD EPYC Embedded hosts: handshake,
> file transfer, and stop/quit with DB1/DB2 exchange.
>
> Raghavendra Ningoji (13):
> raw/ntb: fix device check in memory alignment query
> raw/ntb: fix peer memory window count check
> raw/ntb: fix memory window bound in address remapping
> raw/ntb: fix memory leaks
> raw/ntb: fix doorbell interrupts with VFIO MSI-X
> raw/ntb: fix AMD processor names
> raw/ntb: remove AMD hardware event handling
> raw/ntb: fix Rx of incomplete packets
> raw/ntb: fix double free of Rx mbufs on stop
> examples/ntb: fix raw/ntb library linking
> doc: add NTB device info change to ABI section
> doc: add IOVA requirement for NTB
> raw/ntb: remove unused AMD register definitions
>
> doc/guides/rawdevs/ntb.rst | 12 ++-
> doc/guides/rel_notes/release_26_11.rst | 6 +-
> drivers/raw/ntb/ntb.c | 136 ++++++++++++++++++++-----
> drivers/raw/ntb/ntb.h | 7 +-
> drivers/raw/ntb/ntb_hw_amd.c | 54 ++--------
> drivers/raw/ntb/ntb_hw_amd.h | 25 +----
> drivers/raw/ntb/ntb_hw_intel.c | 9 +-
> examples/ntb/Makefile | 3 +-
> 8 files changed, 149 insertions(+), 103 deletions(-)
>
Lots of AI reported issues here:
Subject: Re: [PATCH 00/13] raw/ntb: post-merge fixes
Series applies to main (26.11-rc2) and builds at every commit with
-Dwerror=true (raw/ntb + examples/ntb).
Patch 04/13 raw/ntb: fix memory leaks
Warning: mw_size is freed in dev_close, but rte_rawdev_close() does not
detach the device; it stays attached and its ops remain reachable.
+ rte_free(hw->mw_size);
+ hw->mw_size = NULL;
After close, rte_pmd_ntb_get_mem_align() passes the new checks from
patch 1 (attached, dev_ops == &ntb_ops, mw_id < mw_cnt) and on Intel
calls intel_ntb_get_mem_align(), which does
return hw->mw_size[mw_id];
i.e. a NULL dereference. A configure after close does the same in
intel_ntb_dev_handshake() (hw->mw_size[i]), and ntb_dev_info_get()
now hands the application mw_cnt = 2 with mw_size = NULL; ntb_fwd
indexes ntb_info.mw_size[0] unconditionally.
Both vendors have a fixed count of 2 windows (XEON_MW_COUNT,
AMD_MW_COUNT), so drop the allocation and embed the array in
struct ntb_hw. It then lives exactly as long as dev_private, the
info->mw_size pointer stays valid, and the leak goes away with no
free at all:
#define NTB_MAX_MW 2
...
uint64_t mw_size[NTB_MAX_MW];
with RTE_BUILD_BUG_ON(XEON_MW_COUNT > NTB_MAX_MW) (and AMD) in the
vendor init, and the rte_zmalloc()/NULL check removed there.
Patch 05/13 raw/ntb: fix doorbell interrupts with VFIO MSI-X
Warning: db_intr_handles is an array of process-private pointers
(handles come from rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE))
used only by the control path. No reason to put it in hugepage memory.
+ hw->db_intr_handles = rte_zmalloc("ntb_db_intr",
+ nb_efd * sizeof(*hw->db_intr_handles), 0);
Use calloc(nb_efd, sizeof(*hw->db_intr_handles)) and free() in
ntb_db_intr_unregister().
Info (pre-existing, not introduced by this patch): peer_dev_up is a
plain uint8_t written by the interrupt thread (DB1/DB2 handlers,
"hw->peer_dev_up = 0;") and polled by ntb_dev_stop():
while (hw->peer_dev_up && time_out) {
Now that DB2 is actually delivered, this loop is the normal stop path
rather than the timeout. It only works because rte_delay_us() is an
opaque call. Should be rte_atomic_store_explicit()/load with
release/acquire, as a separate patch.
Patch 08/13 raw/ntb: fix Rx of incomplete packets
Info (pre-existing, not introduced by this patch): the new on-stack
+ struct rte_mbuf *new_mbufs[NTB_MAX_DESC_SIZE];
like the existing rx_desc[], is only bounded because nb_rx_desc is
assumed <= NTB_MAX_DESC_SIZE. ntb_rxq_setup() takes
"rxq->nb_rx_desc = rxq_conf->nb_desc;" without checking that, or that
it is a power of two (the ring indexes use & (nb_rx_desc - 1)); only
the ntb_fwd example range-checks it. The same function also leaks rxq
when rx_mp is NULL. Worth a separate fix:
if (rxq_conf->rx_mp == NULL ||
rxq_conf->nb_desc < NTB_MIN_DESC_SIZE ||
rxq_conf->nb_desc > NTB_MAX_DESC_SIZE ||
!rte_is_power_of_2(rxq_conf->nb_desc))
return -EINVAL;
placed before the rte_zmalloc_socket().
Review-Result: WARNING
