Subject: Re: [PATCH v5 3/3] PCI: dwc: ep: Support BAR subrange inbound mapping via Address Match Mode iATU

From: Niklas Cassel <[email-masked]>
Date: Fri, 9 Jan 2026 09:30:04 +0100

On Fri, Jan 09, 2026 at 04:29:14PM +0900, Koichiro Den wrote:
> > Does that mean that the usage of this API will be something like:
> > 
> > 1) set_bar() ## using BAR match mode, since BAR match mode can write
> >    the BAR mask to define a BAR size, so that the host can assign a
> >    PCI address to the BAR.
> 
> BAR sizing is done by dw_pcie_ep_set_bar_{programmable,resizable}() before
> iATU programming regardless of match mode. I keep BAR match mode here just
> because Address match mode requires a valid base address. That's why I
> added the `if (!base)` guard.
> 
> > 
> > 2) start() ## start link
> > 
> > 3) link up
> > 
> > 4) wait for some special command, perhaps NTB_EPF_COMMAND
> > CMD_CONFIGURE_DOORBELL or NTB_EPF_COMMAND CMD_CONFIGURE_MW
> > 
> > 5) set_bar() ## using Address match mode. Because address match mode
> >    requires that the host has assigned a PCI address to the BAR, we
> >    can only change the mapping for a BAR after the host has assigned
> >    PCI addresses for all bars.
> > 
> 
> The overall usage flow matches what I'm assuming.

Ok, perhaps document something that the submap feature requires that the
BAR already has been assigned an address (and that it thus has to call
set_bar() twice, without calling clear_bar() in-between.

This is a new feature, and since you provide a mapping for the whole BAR,
I think it is logical for people to incorrectly assume that you could use
this feature by just calling set_bar() once.

> > Perhaps you should add some text to:
> > Documentation/PCI/endpoint/pci-endpoint.rst
> > 
> > Because right now the documentation for pci_epc_set_bar() says:
> > 
> >    The PCI endpoint function driver should use pci_epc_set_bar() to configure
> >    the Base Address Register in order for the host to assign PCI addr space.
> >    Register space of the function driver is usually configured
> >    using this API.
> > 
> > So it is obviously meant to be called *before* the host assigns a PCI
> > address for the BAR. Now with submap ranges, it appears that it has to
> > be called *after* the host assigned a PCI address for the BAR.
> 
> I agree. As I understand it, the current text seems not to reflect mainline
> since commit 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar()
> update inbound map address"), but anyway I should add explicit
> documentation for this subrange mapping use case.

Sure, 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar() update
inbound map address") modified so that set_bar() can be called twice,
without calling clear_bar().

However, that patch was a mess because:
1) It got merged via the NTB tree, even though the PCI maintainer wanted a
   different design:
   https://lore.kernel.org/linux-pci/20220818060230.GA12008@thinkpad/T/#m411b3e9f6625da9dde747f624ed6870bef3e8823
2) It was buggy:
   https://github.com/torvalds/linux/commit/c2a57ee0f2f1ad8c970ff58b78a43e85abbdeb7f
3) It lacked the proper conditional checks for the feature to work (and it
   lacked any comments in the source explaining why it was skipping steps):
   https://github.com/torvalds/linux/commit/3708acbd5f169ebafe1faa519cb28adc56295546
4) It failed to update the documentation.
5) It failed to add a new flag for this feature in epc_features.
   I seriously doubt that any non-DWC based EP controller supports changing
   the inbound address translations without first disabling the BAR.
   (It probably should have added a epc_features->dynamic_inbound_mapping bit.)

So all in all 4284c88fff0e in not the best example :)

Your new feature (epc_features->subrange_mapping) in epc_features appears
to depend on epc_features->dynamic_inbound_mapping, so it is a shame that
we don't have a epc_features->dynamic_inbound_mapping bit, so that this new
feature could have depended on that bit.

	if (epf_bar->use_submap &&
	    !(epc_features->dynamic_inbound_mapping &&
	      epc_features->subrange_mapping))
		return -EINVAL;

I think adding some documentation is a good step.

Perhaps we should also introduce a epc_features->dynamic_inbound_mapping bit?
Since you are making DWC glue drivers return a mutable EPC features, we could
set this bit in the DWC driver after that commit. What do you think?

I realize that we would not be able to add any actual verification for the
epc_features->dynamic_inbound_mapping feature itself (in set_bar()), since
there is no way for set_bar() to know if a BAR has already been configured
(since struct pci_epc does not have an array of the currently configured BARs).

But at least it would allow an EPF driver (e.g. vNTB) to know if it can call
set_bar() twice or not, and can error out if the EPF driver is being used on
a EPC that doesn't support epc_features->dynamic_inbound_mapping.

Kind regards,
Niklas