Re: [RFC PATCH v4 16/38] NTB: ntb_transport: Move TX memory window setup into setup_qp_mw()

Koichiro Den
2026-01-21T02:15:59Z

On Mon, Jan 19, 2026 at 03:36:47PM -0500, Frank Li wrote:
> On Sun, Jan 18, 2026 at 10:54:18PM +0900, Koichiro Den wrote:
> > Historically both TX and RX have assumed the same per-QP MW slice
> > (tx_max_entry == remote rx_max_entry), while those are calculated
> > separately in different places (pre and post the link-up negotiation
> > point). This has been safe because nt->link_is_up is never set to true
> > unless the pre-determined qp_count are the same among them, and qp_count
> > is typically limited to nt->mw_count, which should be carefully
> > configured by admin.
> >
> > However, setup_qp_mw can actually split mw and handle multi-qps in one
> > MW properly, so qp_count needs not to be limited by nt->mw_count. Once
> > we relax the limitation, pre-determined qp_count can differ among host
> > side and endpoint, and link-up negotiation can easily fail.
> >
> > Move the TX MW configuration (per-QP offset and size) into
> > ntb_transport_setup_qp_mw() so that both RX and TX layout decisions are
> > centralized in a single helper. ntb_transport_init_queue() now deals
> > only with per-QP software state, not with MW layout.
> >
> > This keeps the previous behavior, while preparing for relaxing the
> > qp_count limitation and improving readability.
> >
> > No functional change is intended.
> >
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> > drivers/ntb/ntb_transport.c | 76 ++++++++++++++++---------------------
> > 1 file changed, 32 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> > index d5a544bf8fd6..57a21f2daac6 100644
> > --- a/drivers/ntb/ntb_transport.c
> > +++ b/drivers/ntb/ntb_transport.c
> > @@ -569,7 +569,10 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
> > struct ntb_transport_mw *mw;
> > struct ntb_dev *ndev = nt->ndev;
> > struct ntb_queue_entry *entry;
> > - unsigned int rx_size, num_qps_mw;
> > + phys_addr_t mw_base;
> > + resource_size_t mw_size;
> > + unsigned int rx_size, tx_size, num_qps_mw;
> > + u64 qp_offset;
> > unsigned int mw_num, mw_count, qp_count;
> > unsigned int i;
> > int node;
> > @@ -588,13 +591,38 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
> > else
> > num_qps_mw = qp_count / mw_count;
> >
> > - rx_size = (unsigned int)mw->xlat_size / num_qps_mw;
> > - qp->rx_buff = mw->virt_addr + rx_size * (qp_num / mw_count);
> > - rx_size -= sizeof(struct ntb_rx_info);
> > + mw_base = nt->mw_vec[mw_num].phys_addr;
> > + mw_size = nt->mw_vec[mw_num].phys_size;
> > +
> > + if (mw_size > mw->xlat_size)
> > + mw_size = mw->xlat_size;
>
> old code have not check this.

Thanks for pointing it out, I'll drop it from this commit so the existing
behaviour remains unchanged, as stated in the commit message.

Thanks,
Koichiro

>
> Frank
> > + if (max_mw_size && mw_size > max_mw_size)
> > + mw_size = max_mw_size;
> > +
> > + tx_size = (unsigned int)mw_size / num_qps_mw;
> > + qp_offset = tx_size * (qp_num / mw_count);
> > +
> > + qp->rx_buff = mw->virt_addr + qp_offset;
> > +
> > + qp->tx_mw_size = tx_size;
> > + qp->tx_mw = nt->mw_vec[mw_num].vbase + qp_offset;
> > + if (!qp->tx_mw)
> > + return -EINVAL;
> > +
> > + qp->tx_mw_phys = mw_base + qp_offset;
> > + if (!qp->tx_mw_phys)
> > + return -EINVAL;
> >
> > + rx_size = tx_size;
> > + rx_size -= sizeof(struct ntb_rx_info);
> > qp->remote_rx_info = qp->rx_buff + rx_size;
> >
> > + tx_size -= sizeof(struct ntb_rx_info);
> > + qp->rx_info = qp->tx_mw + tx_size;
> > +
> > /* Due to housekeeping, there must be atleast 2 buffs */
> > + qp->tx_max_frame = min(transport_mtu, tx_size / 2);
> > + qp->tx_max_entry = tx_size / qp->tx_max_frame;
> > qp->rx_max_frame = min(transport_mtu, rx_size / 2);
> > qp->rx_max_entry = rx_size / qp->rx_max_frame;
> > qp->rx_index = 0;
> > @@ -1132,16 +1160,6 @@ static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
> > unsigned int qp_num)
> > {
> > struct ntb_transport_qp *qp;
> > - phys_addr_t mw_base;
> > - resource_size_t mw_size;
> > - unsigned int num_qps_mw, tx_size;
> > - unsigned int mw_num, mw_count, qp_count;
> > - u64 qp_offset;
> > -
> > - mw_count = nt->mw_count;
> > - qp_count = nt->qp_count;
> > -
> > - mw_num = QP_TO_MW(nt, qp_num);
> >
> > qp = &nt->qp_vec[qp_num];
> > qp->qp_num = qp_num;
> > @@ -1151,36 +1169,6 @@ static int ntb_transport_init_queue(struct ntb_transport_ctx *nt,
> > qp->event_handler = NULL;
> > ntb_qp_link_context_reset(qp);
> >
> > - if (mw_num < qp_count % mw_count)
> > - num_qps_mw = qp_count / mw_count + 1;
> > - else
> > - num_qps_mw = qp_count / mw_count;
> > -
> > - mw_base = nt->mw_vec[mw_num].phys_addr;
> > - mw_size = nt->mw_vec[mw_num].phys_size;
> > -
> > - if (max_mw_size && mw_size > max_mw_size)
> > - mw_size = max_mw_size;
> > -
> > - tx_size = (unsigned int)mw_size / num_qps_mw;
> > - qp_offset = tx_size * (qp_num / mw_count);
> > -
> > - qp->tx_mw_size = tx_size;
> > - qp->tx_mw = nt->mw_vec[mw_num].vbase + qp_offset;
> > - if (!qp->tx_mw)
> > - return -EINVAL;
> > -
> > - qp->tx_mw_phys = mw_base + qp_offset;
> > - if (!qp->tx_mw_phys)
> > - return -EINVAL;
> > -
> > - tx_size -= sizeof(struct ntb_rx_info);
> > - qp->rx_info = qp->tx_mw + tx_size;
> > -
> > - /* Due to housekeeping, there must be atleast 2 buffs */
> > - qp->tx_max_frame = min(transport_mtu, tx_size / 2);
> > - qp->tx_max_entry = tx_size / qp->tx_max_frame;
> > -
> > if (nt->debugfs_node_dir) {
> > char debugfs_name[8];
> >
> > --
> > 2.51.0
> >