Re: [PATCH v7 06/10] PCI: endpoint: pci-epf-vntb: Allow DMA and MW to share a BAR

[Koichiro Den]

On Wed, Oct 21, 2026 at 02:00:17PM -0500, Frank Li wrote:
> On Thu, Oct 22, 2026 at 01:35:18AM +0900, Koichiro Den wrote:
> > On Wed, Oct 21, 2026 at 10:52:32AM -0500, Frank Li wrote:
> > > On Wed, Oct 21, 2026 at 01:22:25PM +0900, Koichiro Den wrote:
> > > > Some endpoint configurations have no spare BAR for DMA resources. Allow
> > > > dma_bar to select an MW BAR and place the DMA ranges after that MW.
> > > >
> > > > DMA submaps have fixed BAR offsets, so a shared MW translation must
> > > > cover its configured size. Report that size as the translation alignment
> > > > and require it to be a power of two.
> > > >
> > > > Older hosts treat the whole BAR as an MW. Require the host to advertise
> > > > MW/DMA boundary support in LINK_UP before enabling a shared-BAR link.
> > > >
> > > > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> > > > ---
> > > > Changes since v6:
> > > >   - Split out the host-side changes. (Frank)
> > > >   - Require MW/DMA BAR boundary support in LINK_UP for shared BARs.
> > > >
> > > >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 154 ++++++++++++++----
> > > >  include/linux/pci-epf-ntb.h                   |   3 +
> > > >  2 files changed, 122 insertions(+), 35 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > index ac83af54122f..721cd841a1dd 100644
> > > > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > @@ -84,12 +84,13 @@ enum epf_irq_slot {
> > > >  #define MIN_DB_COUNT			(EPF_IRQ_DB_START + 1)
> > > >
> > > >  struct epf_ntb_dma {
> > > > +	struct mutex lock; /* Serialize DMA submap and MW updates */
> > > >  	struct epf_ntb_dma_ctrl ctrl;
> > > >  	struct dma_chan *dchan[EDMA_MAX_RD_CH];
> > > >  	void *bar_scratch;
> > > >  	dma_addr_t bar_scratch_phys;
> > > >  	size_t bar_scratch_size;
> > > > -	struct pci_epf_bar_submap submap[EDMA_MAX_RD_CH + 2];
> > > > +	struct pci_epf_bar_submap submap[EDMA_MAX_RD_CH + 3];
> > > >  	struct pci_epf_bar_submap *reg_submap;
> > > >  	unsigned int num_submap;
> > > >  	u16 rd_ch_cnt;
> > > > @@ -229,6 +230,22 @@ static bool epf_ntb_is_bar_used(struct epf_ntb *ntb,
> > > >  	return false;
> > > >  }
> > > >
> > > > +static u64 epf_ntb_dma_bar_offset(struct epf_ntb *ntb, enum pci_barno barno)
> > > > +{
> > > > +	unsigned int i;
> > > > +
> > > > +	for (i = 0; i < ntb->num_mws; i++)
> > > > +		if (ntb->epf_ntb_bar[BAR_MW1 + i] == barno)
> > > > +			return ntb->mws_size[i];
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static bool epf_ntb_dma_shares_bar(struct epf_ntb *ntb, enum pci_barno barno)
> > > > +{
> > > > +	return ntb->dma && ntb->epf_ntb_bar[BAR_DMA] == barno;
> > > > +}
> > > > +
> > > >  struct epf_ntb_dma_filter {
> > > >  	struct device *dev;
> > > >  	int chan_id;
> > > > @@ -353,6 +370,7 @@ static int epf_ntb_dw_edma_collect(struct epf_ntb *ntb,
> > > >  	unsigned int i;
> > > >  	size_t align;
> > > >  	u32 next = 0;
> > > > +	u64 offset;
> > > >  	int ret;
> > > >
> > > >  	if (ctrl->u.dma_ctrl.reg_layout_data != EDMA_MF_EDMA_UNROLL)
> > > > @@ -371,9 +389,30 @@ static int epf_ntb_dw_edma_collect(struct epf_ntb *ntb,
> > > >  	if (!is_power_of_2(align))
> > > >  		return -EINVAL;
> > > >
> > > > -	if (epf_ntb_is_bar_used(ntb, barno) ||
> > > > -	    pci_epc_get_next_free_bar(features, barno) != barno)
> > > > +	if (barno == ntb->epf_ntb_bar[BAR_CONFIG] ||
> > > > +	    barno == ntb->epf_ntb_bar[BAR_DB])
> > > > +		return -EINVAL;
> > > > +
> > > > +	offset = epf_ntb_dma_bar_offset(ntb, barno);
> > > > +	if (!offset &&
> > > > +	    (epf_ntb_is_bar_used(ntb, barno) ||
> > > > +	     pci_epc_get_next_free_bar(features, barno) != barno))
> > > > +		return -EINVAL;
> > > > +	if (offset > U32_MAX)
> > > > +		return -EOVERFLOW;
> > > > +	if (offset && !is_power_of_2(offset))
> > > >  		return -EINVAL;
> > > > +	dma->ctrl.submap.offset = offset;
> > > > +	next = offset;
> > > > +	if (next) {
> > > > +		/*
> > > > +		 * submap[0] covers the MW prefix. Keep its temporary
> > > > +		 * target at address 0 until the MW translation is
> > > > +		 * installed.
> > > > +		 */
> > > > +		dma->submap[0].size = next;
> > > > +		dma->num_submap = 1;
> > > > +	}
> > > >
> > > >  	dma->ctrl.magic = EPF_NTB_DMA_MAGIC;
> > > >  	dma->ctrl.revision = EPF_NTB_DMA_REVISION;
> > > > @@ -408,7 +447,7 @@ static int epf_ntb_dw_edma_collect(struct epf_ntb *ntb,
> > > >  		chan->desc_addr_lo = lower_32_bits(dma_addr);
> > > >  		chan->desc_addr_hi = upper_32_bits(dma_addr);
> > > >  	}
> > > > -	dma->ctrl.submap.size = next;
> > > > +	dma->ctrl.submap.size = next - dma->ctrl.submap.offset;
> > > >
> > > >  	dma_dev = ntb->epf->epc->dev.parent;
> > > >  	for (i = 0; i < dma->rd_ch_cnt; i++) {
> > > > @@ -481,6 +520,8 @@ static int epf_ntb_dma_collect(struct epf_ntb *ntb)
> > > >  	if (!dma)
> > > >  		return -ENOMEM;
> > > >
> > > > +	mutex_init(&dma->lock);
> > > > +
> > > >  	switch (ctrl->u.dma_ctrl.reg_layout) {
> > > >  	case PCI_EPC_AUX_DMA_REG_LAYOUT_DW_EDMA:
> > > >  		ret = epf_ntb_dw_edma_collect(ntb, dma, ctrl, resources, count);
> > > > @@ -551,7 +592,7 @@ static int epf_ntb_dma_bar_init(struct epf_ntb *ntb)
> > > >  		return -EOPNOTSUPP;
> > > >
> > > >  	barno = ntb->epf_ntb_bar[BAR_DMA];
> > > > -	mapped_size = dma->ctrl.submap.size;
> > > > +	mapped_size = dma->ctrl.submap.offset + dma->ctrl.submap.size;
> > > >  	/*
> > > >  	 * Submaps cannot be installed until the host assigns the BAR address.
> > > >  	 * Use address 0 for the temporary BAR Match Mode mapping, as is done
> > > > @@ -607,21 +648,25 @@ static void epf_ntb_dma_bar_clear(struct epf_ntb *ntb)
> > > >  	memset(bar, 0, sizeof(*bar));
> > > >  }
> > > >
> > > > -static int epf_ntb_dma_set_bar(struct epf_ntb *ntb)
> > > > +static int epf_ntb_dma_set_bar_locked(struct epf_ntb *ntb,
> > > > +				      const dma_addr_t *mw_addr)
> > > >  {
> > > >  	struct pci_epf_bar_submap *old_submap;
> > > >  	struct epf_ntb_dma *dma = ntb->dma;
> > > >  	unsigned int old_num_submap;
> > > >  	struct pci_epf_bar *bar;
> > > > +	dma_addr_t old_mw_addr;
> > > >  	int restore, ret;
> > > > -
> > > > -	if (!dma)
> > > > -		return 0;
> > > > +	lockdep_assert_held(&dma->lock);
> > > >
> > > >  	bar = &ntb->epf->bar[ntb->epf_ntb_bar[BAR_DMA]];
> > > >
> > > >  	old_submap = bar->submap;
> > > >  	old_num_submap = bar->num_submap;
> > > > +	if (mw_addr) {
> > > > +		old_mw_addr = dma->submap[0].phys_addr;
> > > > +		dma->submap[0].phys_addr = *mw_addr;
> > > > +	}
> > > >  	bar->submap = dma->submap;
> > > >  	bar->num_submap = dma->num_submap;
> > > >
> > > > @@ -631,6 +676,8 @@ static int epf_ntb_dma_set_bar(struct epf_ntb *ntb)
> > > >  		return 0;
> > > >
> > > >  	/* A failed dynamic update may have already removed the old mapping. */
> > > > +	if (mw_addr)
> > > > +		dma->submap[0].phys_addr = old_mw_addr;
> > > >  	bar->submap = old_submap;
> > > >  	bar->num_submap = old_num_submap;
> > > >  	restore = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
> > > > @@ -642,6 +689,18 @@ static int epf_ntb_dma_set_bar(struct epf_ntb *ntb)
> > > >  	return ret;
> > > >  }
> > > >
> > > > +static int epf_ntb_dma_set_bar(struct epf_ntb *ntb)
> > > > +{
> > > > +	struct epf_ntb_dma *dma = ntb->dma;
> > > > +
> > > > +	if (!dma)
> > > > +		return 0;
> > > > +
> > > > +	guard(mutex)(&dma->lock);
> > > > +
> > > > +	return epf_ntb_dma_set_bar_locked(ntb, NULL);
> > > > +}
> > > > +
> > > >  /**
> > > >   * epf_ntb_configure_mw() - Configure the Outbound Address Space for VHOST
> > > >   *   to access the memory window of HOST
> > > > @@ -780,6 +839,13 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> > > >  					     EPF_NTB_COMMAND_STATUS_OK;
> > > >  		break;
> > > >  	case EPF_NTB_COMMAND_LINK_UP:
> > > > +		if (ntb->dma && ntb->dma->ctrl.submap.offset &&
> > > > +		    !(argument & EPF_NTB_LINK_UP_DMA_MW_SUPPORTED)) {
> > > > +			epf_ntb_set_link_enabled(ntb, false);
> > > > +			dev_err(dev, "Host does not support MW/DMA BAR sharing\n");
> > > > +			ctrl->command_status = EPF_NTB_COMMAND_STATUS_ERROR;
> > >
> > > suppose it should be fallback to old method?
> > > EPF_NTB_LINK_UP_DMA_MW_SUPPORTED is new feature.
> >
> > It's possible (by using the whole BAR as an MW), but I personally think the
> > extra complexity would not really be worthwhile for this case.
> >
> > If a "New" endpoint is paired with an "Old" host whose kernel cannot be
> > upgraded, the DMA feature cannot be used anyway. For that setup, I think
> > disabling DMA export with dma_bar = -1 (the default) and rebinding the endpoint
> > would be sufficient, with host re-enumeration as needed.
> 
> At least it should work without using DMA as previous version. Assume build
> as PCIE card, which have already prebuild image, which may plug into any
> host machine. You can't require user to rebinding endpoint software.
> 
> Many PCI card have not provided any console after production.

Makes sense. I'll send v8 including the fallback implementation shortly.
"Compatibility matrix" in the cover letter will look like this:

  EP                              | Old host     | New host
  --------------------------------+--------------+----------
  Old                             | OK*          | OK*
  New (dma_bar = -1)              | OK*          | OK*
  New (dedicated DMA BAR)         | OK*          | OK
  New (shared MW/DMA BAR)         | OK*          | OK

  *  NTB works without DMA.

Thanks for the review!

Best regards,
Koichiro Den

> 
> Frank