Hello,
On Mon, 19 Oct 2026 at 08:39, Raghavendra Ningoji
<[email protected]> wrote:
>
> This series adds support for the Non-Transparent Bridge (NTB) endpoints
> integrated in AMD EPYC Embedded "Turin", "Genoa" and "Siena" processors
> to the raw/ntb driver.
>
> Following David Marchand's review of v2, the common NTB code has been
> reworked so that it no longer relies on runtime checks of the vendor ops
> presence, and the vendor abstraction has been redesigned around a small
> set of mandatory ops instead of optional hooks with built-in fallbacks:
>
> - Patch 1 is a preparation commit that enforces the vendor ops once, at
> probe time, and removes the per-call NULL checks that are scattered
> throughout the common code.
>
> - Patch 2 generalizes the framework for multiple vendors. The link
> bring-up/tear-down is now expressed through mandatory ops
> (dev_handshake, peer_mw_validate, read_peer_config, link_cleanup); the
> former built-in scratchpad logic moved into the Intel driver. The
> mw_set_trans op is gone (it is now a vendor-internal concern of the
> handshake), and there is a single common interrupt handler with just
> one optional op (handle_hw_event) for vendor-specific hardware events.
>
> - Patch 3 adds the AMD hardware driver, its documentation, release note
> and MAINTAINERS entry.
>
> A few AMD-specific hardware details worth noting for reviewers:
>
> - Primary/secondary topology. One endpoint enumerates as the primary
> (device ID 0x14c0) and the other as the secondary (0x14c3), rather
> than the Intel back-to-back model.
>
> - Packed scratchpad handshake. The hardware exposes a single shared
> 16-register scratchpad bank split into two disjoint 8-register sets
> (one per side), so the driver uses a packed handshake layout that
> fits in 8 registers, plugged in through the dev_handshake,
> peer_mw_validate and read_peer_config ops.
>
> - Outbound translation window. AMD NTB uses an outbound translation
> window: a write to a BARxx memory-window offset is forwarded to
> (xlat_base | offset) in the peer's memory rather than
> (xlat_base + offset). The translation base must therefore be aligned
> to a power of two >= the window length so that no offset bit collides
> with a set bit in the base. This is reported to applications through
> the rte_pmd_ntb_get_mem_align() API and honoured by the ntb example
> when reserving the memory window memzone.
>
> - Secondary-side link status. On the secondary side the device's own
> PCIe link status does not reflect the true inter-host link, so the
> speed and width are read from the upstream switch port via sysfs.
>
> Tested on a two-node setup (primary + secondary EPYC Embedded hosts)
> with the dpdk-ntb example: the link comes up at the expected speed and
> width on both sides, and file transfers are byte-correct end to end for
> both small and large files.
>
> Changes since v2:
>
> - The series is freshly rebased on main (26.11-rc0); the release note
> and dpdk-devbind.py now apply cleanly and CI should pass.
>
> - New preparation patch (patch 1): the vendor ops are validated once at
> probe time and all the runtime ntb_ops NULL checks in the common code
> are removed.
>
> - Reworked the vendor abstraction (patch 2): dev_handshake,
> peer_mw_validate, read_peer_config and link_cleanup are now mandatory
> ops provided by both the Intel and AMD drivers. The built-in
> scratchpad handshake/validate/cleanup/peer-config logic that used to
> live in the common code moved into the Intel driver.
>
> - Dropped the mw_set_trans op: it is now a vendor-internal detail of the
> handshake.
>
> - Dropped the vendor interrupt_handler op: there is now a single common
> interrupt handler, with handle_hw_event as the only optional op, used
> by AMD for its SMU/power-management events.
Thank you, that's better.
Still a few things to improve.
- On patch 1, prefer a simple check like:
#define NTB_OPS_REQ(member) \
if (hw->ntb_ops->member == NULL) { \
NTB_LOG(ERR, "Vendor ops lacks mandatory ." # member); \
return -EINVAL; \
}
- The change on alignment is not listed in the API changes section of
the release notes.
An update in the RN should highlight that an application must now
rely on rte_pmd_ntb_get_mem_align() and not the mw_size_align field.
Btw, why keep this field? It would be easier for a developer to
notice a broken compilation. No strong opinion on this point.
- Even though the AMD implementation is added now, I see no reason to
always list it after the Intel code.
By this, I mean that you must alphabetically sort:
- the files in meson.build,
- when including the ntb_hw_amd.h header,
- when adding the AMD PCI id in the driver mapping table,
- when adding the PCI id in usertools/dpdk-devbind.py (I just
cleaned this up, I don't want to see the convention broken at the next
commit),
- etc...
- The AMD topology definitions do not belong to patch 2, but patch 3.
- The pmd_private field is unneeded, please remove:
- amd_hw->self_mmio is just a copy of ntb->hw_addr,
- all uses of amd_hw->peer_mmio involve an addition with an offset.
AMD implementation can simply rely on ntb->hw_addr +
AMD_PEER_OFFSET, or introduce a macro like:
#define AMD_PEER_MMIO(ntb) RTE_PTR_ADD((ntb)->hw_addr, AMD_PEER_OFFSET)
- all uses of self_spad / peer_spad involve a check on the peer, and
can be replaced with something like:
#define AMD_SPAD_BASE(ntb, peer) (((ntb)->topo == NTB_TOPO_PRI) ==
(peer) ? AMD_SPAD_SET_OFFSET : 0)
- int_mask is an *unused* constant, making it a double reason to remove,
- peer_status is unused,
- ctl_status is unused,
- As I was reading the documentation, I noticed that you missed to
update doc/guides/rawdevs/ntb.rst with something like:
@@ -151,8 +177,9 @@ like the following:
Limitation
----------
-This PMD is only supported on Intel Xeon Platforms:
+This PMD is supported on:
+- AMD EPYC Embedded "Turin", "Genoa" and "Siena" processors.
- 4th Generation Intel® Xeon® Scalable Processors.
- 3rd Generation Intel® Xeon® Scalable Processors.
- 2nd Generation Intel® Xeon® Scalable Processors.
- I also noticed strange indentation, so please double check the
coding style guidelines.
And, DPDK allows lines up to 100 characters.
--
David Marchand
