On Wed, Jan 14, 2026 at 11:39:03AM +0100, Niklas Cassel wrote:
> On Wed, Jan 14, 2026 at 12:54:37PM +0900, Koichiro Den wrote:
> > I realized that I missed one case in v7.
> >
> > I think dw_pcie_ep_clear_ib_maps() should also be called from
> > dw_pcie_ep_ib_atu_bar() to tear down any existing inbound mappings for the
> > same BAR before re-programming it in BAR Match Mode.
> >
> > This matters when updating inbound mappings for a BAR without resetting the
> > BAR in between. There are four possible transition patterns, and pattern #4
> > below was overlooked:
> >
> > 1. BAR Match Mode -> BAR Match Mode
> > As the current implementation does, the mapping is simply updated
> > (with the same atu index)
> >
> > 2. BAR Match Mode -> Address Match Mode
> > This patch series already ensures the old BAR Match mapping is
> > torn down before reprogramming.
> >
> > 3. Address Match Mode -> Address Match Mode
> > Likewise, existing Address Match mappings are cleared first.
> >
> > 4. Address Match Mode -> BAR Match Mode
> > This case was not handled. The change below adds the missing
> > teardown so that stale Address Match mappings do not remain active.
> >
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -148,9 +148,12 @@ static int dw_pcie_ep_ib_atu_bar(struct
> > dw_pcie_ep *ep, u8 func_no, int type,
> > u32 free_win;
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >
> > - if (!ep->bar_to_atu[bar])
> > + if (!ep->bar_to_atu[bar]) {
> > + /* Tear down existing mappings before (re)programming.
> > */
> > + dw_pcie_ep_clear_ib_maps(ep, bar);
> > +
> > free_win = find_first_zero_bit(ep->ib_window_map,
> > pci->num_ib_windows);
> > - else
> > + } else
> > free_win = ep->bar_to_atu[bar] - 1;
>
> If one of the branches has braces, both branches should have braces:
> https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces
>
>
> >
> > Unless there are objections, I'll include this fix in v8.
>
> Isn't it easier/cleaner if we call dw_pcie_ep_clear_ib_maps() in
> dw_pcie_ep_set_bar(), rather than calling it in both dw_pcie_ep_ib_atu_addr()
> and dw_pcie_ep_ib_atu_bar() ?
>
> dw_pcie_ep_set_bar() knows the condition if we are dynamically reprogramming
> a BAR or not, and all the four cases are when dynamically reprogramming a BAR.
>
> I.e. instead of adding additional code to dw_pcie_ep_ib_atu_bar(), we do
> something like:
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index b2ea2c2c986f..63ae5471fe13 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -318,9 +318,6 @@ static int dw_pcie_ep_ib_atu_addr(struct dw_pcie_ep *ep,
> u8 func_no, int type,
> return -EINVAL;
> }
>
> - /* Tear down any existing mappings before (re)programming. */
> - dw_pcie_ep_clear_ib_maps(ep, bar);
> -
> for (i = 0; i < epf_bar->num_submap; i++) {
> off = submap[i].offset;
> size = submap[i].size;
> @@ -571,6 +568,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8
> func_no, u8 vfunc_no,
> ep->epf_bar[bar]->flags != flags)
> return -EINVAL;
>
> + if (ep->epf_bar[bar]->num_submap || epf_bar->num_submap)
> + dw_pcie_ep_clear_ib_maps(ep, bar);
> +
> /*
> * When dynamically changing a BAR, skip writing the BAR reg,
> as
> * that would clear the BAR's PCI address assigned by the
> host.
>
For pattern #2 and #3 (ie. either mode -> Address Match Mode), the v7 code
withholds the dw_pcie_ep_clear_ib_maps() call unless the submap validation
passes. The above patch differs slightly in that sense, but I agree it
looks much simpler. I don't think the difference matters much, since
pci_epc_set_bar() with an invalid submap should already indicate that
something has gone wrong (most likely a bug in the API call site). So I
think I'll go with your suggestion.
Thanks!
Koichiro
>
>
