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

[Koichiro Den]

On Fri, Jan 09, 2026 at 09:30:04AM +0100, Niklas Cassel wrote:
> 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.)
> 

Thanks for pointing me to the context, that really helps.

> 
> 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?

As you pointed out, support for dynamic_inbound_mapping is needed
independently of my series. Given that, it would make sense to handle it
either before this series, or to fold it into the next iteration (=v6) of
the series if that is preferred.

If you already have a patch for dynamic_inbound_mapping in mind, I'm happy
to wait for it and help test it.

> 
> 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.

That makes sense.

Thanks again,
Koichiro

> 
> 
> Kind regards,
> Niklas