On Fri, Sep 25, 2026 at 10:22:17AM +0000, Bruce Richardson wrote:
> Reviewing changes to common code only, please see inline below. I think
> there is quite a bit of complexity introduced by the alignment constraints
> which could do with being simplified.
Thanks Bruce, agreed on all counts. v2 reworks the alignment handling
exactly along the lines you suggested. Point-by-point below.
> > - 'ntb_hw_intel.c')
> > + 'ntb_hw_intel.c',
> > + 'ntb_hw_amd.c')
>
> Very minor nit, but consider putting the ")" on the next line and putting a
> comma after the 'ntb_hw_amd.c' entry (since meson allows a trailing comma).
Done in v2:
sources = files(
'ntb.c',
'ntb_hw_intel.c',
'ntb_hw_amd.c',
)
> > + /**
> > + * AMD NTB uses an outbound translation window: writes to a BARxx
> > + ...
> > + */
>
> Consider shortening the coment here. Also see other feedback below
> regarding this field.
The whole block is gone in v2 - ntb_dev_info_get() no longer sets any
per-vendor alignment field (see below).
> > + uint64_t mw_addr_align;
>
> Again, shorten the comment here to just a line or two.
> Also, in terms of how it is used, remove the special case for 0 == cache
> aligned, and instead change code assignment above to be cache aligned by
> default. [...]
>
> This is really complex to have in the app [...] I would suggest that,
> rather than trying to expose this via a single addr_align value - which it
> turns out isn't actually the alignment needed - you add a separate API
> called "get_mem_align" or something similar, and then hide the complexity
> of this calculation in the driver. Then you can drop the mw_addr_align in
> the info struct.
Done. v2 drops mw_addr_align entirely and adds an experimental API:
uint64_t rte_pmd_ntb_get_mem_align(uint16_t dev_id, uint32_t mw_id,
uint64_t mw_len);
backed by a new mem_align op. The driver returns the actual base-address
alignment required for a window of the given length, so the app has no
hardware-specific logic. The AMD driver returns
max(4K, rte_align64pow2(mw_len)); the Intel driver returns the memory
window size (matching the previous mw_size_align behaviour). The app
just reserves the memzone with the returned value, no branching:
align = rte_pmd_ntb_get_mem_align(dev_id, mz_id, mz_len);
mz = rte_memzone_reserve_aligned(mz_name, mz_len, socket_id,
RTE_MEMZONE_IOVA_CONTIG, align);
The alignment rule itself now lives in amd_ntb_get_mem_align() with a
short comment; the misaligned-base check stays in amd_ntb_mw_set_trans().
Thanks,
Raghavendra
