Extend dw_pcie_ep_set_bar() to support inbound mappings for BAR
subranges using Address Match Mode IB iATU when pci_epf_bar.num_submap
is non-zero.
Rename the existing BAR-match helper into dw_pcie_ep_ib_atu_bar() and
introduce dw_pcie_ep_ib_atu_addr() for Address Match Mode. When
num_submap is non-zero, read the assigned BAR base address and program
one inbound iATU window per subrange. Validate the submap array before
programming:
- each subrange is aligned to pci->region_align
- subranges cover the whole BAR (no gaps and no overlaps)
- subranges are sorted in ascending order by offset
Track Address Match Mode mappings and tear them down on clear_bar() and
on set_bar() error paths to avoid leaving half-programmed state or
untranslated BAR holes.
Advertise this capability by extending the common feature bit
initializer macro (DWC_EPC_COMMON_FEATURES).
This enables multiple inbound windows within a single BAR, which is
useful on platforms where usable BARs are scarce but EPFs need multiple
inbound regions.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
.../pci/controller/dwc/pcie-designware-ep.c | 203 +++++++++++++++++-
drivers/pci/controller/dwc/pcie-designware.h | 7 +-
2 files changed, 199 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index cfd59899c7b8..0567552b784c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -100,9 +100,10 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
return 0;
}
-static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
- dma_addr_t parent_bus_addr, enum pci_barno bar,
- size_t size)
+/* BAR Match Mode inbound iATU mapping */
+static int dw_pcie_ep_ib_atu_bar(struct dw_pcie_ep *ep, u8 func_no, int type,
+ dma_addr_t parent_bus_addr, enum pci_barno bar,
+ size_t size)
{
int ret;
u32 free_win;
@@ -135,6 +136,179 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
return 0;
}
+static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, enum pci_barno bar)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct device *dev = pci->dev;
+ unsigned int i, num;
+ u32 atu_index;
+ u32 *indexes;
+
+ /* Tear down the BAR Match Mode mapping, if any. */
+ if (ep->bar_to_atu[bar]) {
+ atu_index = ep->bar_to_atu[bar] - 1;
+ dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, atu_index);
+ clear_bit(atu_index, ep->ib_window_map);
+ ep->bar_to_atu[bar] = 0;
+ }
+
+ /* Tear down all Address Match Mode mappings, if any. */
+ indexes = ep->ib_atu_indexes[bar];
+ num = ep->num_ib_atu_indexes[bar];
+ ep->ib_atu_indexes[bar] = NULL;
+ ep->num_ib_atu_indexes[bar] = 0;
+ if (!indexes)
+ return;
+ for (i = 0; i < num; i++) {
+ dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, indexes[i]);
+ clear_bit(indexes[i], ep->ib_window_map);
+ }
+ devm_kfree(dev, indexes);
+}
+
+static u64 dw_pcie_ep_read_bar_assigned(struct dw_pcie_ep *ep, u8 func_no,
+ enum pci_barno bar, int flags)
+{
+ u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
+ u32 lo, hi;
+ u64 addr;
+
+ lo = dw_pcie_ep_readl_dbi(ep, func_no, reg);
+
+ if (flags & PCI_BASE_ADDRESS_SPACE)
+ return lo & PCI_BASE_ADDRESS_IO_MASK;
+
+ addr = lo & PCI_BASE_ADDRESS_MEM_MASK;
+ if (!(flags & PCI_BASE_ADDRESS_MEM_TYPE_64))
+ return addr;
+
+ hi = dw_pcie_ep_readl_dbi(ep, func_no, reg + 4);
+ return addr | ((u64)hi << 32);
+}
+
+static int dw_pcie_ep_validate_submap(struct dw_pcie_ep *ep,
+ const struct pci_epf_bar_submap *submap,
+ unsigned int num_submap, size_t bar_size)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ u32 align = pci->region_align;
+ size_t off = 0;
+ unsigned int i;
+ size_t size;
+
+ if (!align || !IS_ALIGNED(bar_size, align))
+ return -EINVAL;
+
+ /*
+ * The submap array order defines the BAR layout (submap[0] starts
+ * at offset 0 and each entry immediately follows the previous
+ * one). Here, validate that it forms a strict, gapless
+ * decomposition of the BAR:
+ * - each entry has a non-zero size
+ * - sizes, implicit offsets and phys_addr are aligned to
+ * pci->region_align
+ * - each entry lies within the BAR range
+ * - the entries exactly cover the whole BAR
+ *
+ * Note: dw_pcie_prog_inbound_atu() also checks alignment for the
+ * PCI address and the target phys_addr, but validating up-front
+ * avoids partially programming iATU windows in vain.
+ */
+ for (i = 0; i < num_submap; i++) {
+ size = submap[i].size;
+
+ if (!size)
+ return -EINVAL;
+
+ if (!IS_ALIGNED(size, align) || !IS_ALIGNED(off, align))
+ return -EINVAL;
+
+ if (!IS_ALIGNED(submap[i].phys_addr, align))
+ return -EINVAL;
+
+ if (off > bar_size || size > bar_size - off)
+ return -EINVAL;
+
+ off += size;
+ }
+ if (off != bar_size)
+ return -EINVAL;
+
+ return 0;
+}
+
+/* Address Match Mode inbound iATU mapping */
+static int dw_pcie_ep_ib_atu_addr(struct dw_pcie_ep *ep, u8 func_no, int type,
+ const struct pci_epf_bar *epf_bar)
+{
+ const struct pci_epf_bar_submap *submap = epf_bar->submap;
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ enum pci_barno bar = epf_bar->barno;
+ struct device *dev = pci->dev;
+ u64 pci_addr, parent_bus_addr;
+ u64 size, base, off = 0;
+ int free_win, ret;
+ unsigned int i;
+ u32 *indexes;
+
+ if (!epf_bar->num_submap || !submap || !epf_bar->size)
+ return -EINVAL;
+
+ ret = dw_pcie_ep_validate_submap(ep, submap, epf_bar->num_submap,
+ epf_bar->size);
+ if (ret)
+ return ret;
+
+ base = dw_pcie_ep_read_bar_assigned(ep, func_no, bar, epf_bar->flags);
+ if (!base) {
+ dev_err(dev,
+ "BAR%u not assigned, cannot set up sub-range mappings\n",
+ bar);
+ return -EINVAL;
+ }
+
+ indexes = devm_kcalloc(dev, epf_bar->num_submap, sizeof(*indexes),
+ GFP_KERNEL);
+ if (!indexes)
+ return -ENOMEM;
+
+ ep->ib_atu_indexes[bar] = indexes;
+ ep->num_ib_atu_indexes[bar] = 0;
+
+ for (i = 0; i < epf_bar->num_submap; i++) {
+ size = submap[i].size;
+ parent_bus_addr = submap[i].phys_addr;
+
+ if (off > (~0ULL) - base) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ pci_addr = base + off;
+ off += size;
+
+ free_win = find_first_zero_bit(ep->ib_window_map,
+ pci->num_ib_windows);
+ if (free_win >= pci->num_ib_windows) {
+ ret = -ENOSPC;
+ goto err;
+ }
+
+ ret = dw_pcie_prog_inbound_atu(pci, free_win, type,
+ parent_bus_addr, pci_addr, size);
+ if (ret)
+ goto err;
+
+ set_bit(free_win, ep->ib_window_map);
+ indexes[i] = free_win;
+ ep->num_ib_atu_indexes[bar] = i + 1;
+ }
+ return 0;
+err:
+ dw_pcie_ep_clear_ib_maps(ep, bar);
+ return ret;
+}
+
static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep,
struct dw_pcie_ob_atu_cfg *atu)
{
@@ -165,17 +339,15 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
enum pci_barno bar = epf_bar->barno;
- u32 atu_index = ep->bar_to_atu[bar] - 1;
- if (!ep->bar_to_atu[bar])
+ if (!ep->epf_bar[bar])
return;
__dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
- dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, atu_index);
- clear_bit(atu_index, ep->ib_window_map);
+ dw_pcie_ep_clear_ib_maps(ep, bar);
+
ep->epf_bar[bar] = NULL;
- ep->bar_to_atu[bar] = 0;
}
static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie *pci,
@@ -331,6 +503,13 @@ 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;
+ /*
+ * When dynamically changing a BAR, tear down any existing
+ * mappings before re-programming.
+ */
+ 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.
@@ -369,8 +548,12 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
else
type = PCIE_ATU_TYPE_IO;
- ret = dw_pcie_ep_inbound_atu(ep, func_no, type, epf_bar->phys_addr, bar,
- size);
+ if (epf_bar->num_submap)
+ ret = dw_pcie_ep_ib_atu_addr(ep, func_no, type, epf_bar);
+ else
+ ret = dw_pcie_ep_ib_atu_bar(ep, func_no, type,
+ epf_bar->phys_addr, bar, size);
+
if (ret)
return ret;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index be47f34b49ca..938dcb541ce3 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -306,7 +306,8 @@
#define DMA_LLP_MEM_SIZE PAGE_SIZE
/* Common struct pci_epc_feature bits among DWC EP glue drivers */
-#define DWC_EPC_COMMON_FEATURES .dynamic_inbound_mapping = true
+#define DWC_EPC_COMMON_FEATURES .dynamic_inbound_mapping = true, \
+ .subrange_mapping = true
struct dw_pcie;
struct dw_pcie_rp;
@@ -487,6 +488,10 @@ struct dw_pcie_ep {
phys_addr_t msi_mem_phys;
struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
+ /* Only for Address Match Mode inbound iATU */
+ u32 *ib_atu_indexes[PCI_STD_NUM_BARS];
+ unsigned int num_ib_atu_indexes[PCI_STD_NUM_BARS];
+
/* MSI outbound iATU state */
bool msi_iatu_mapped;
u64 msi_msg_addr;
--
2.51.0On Thu, Jan 22, 2026 at 05:49:08PM +0900, Koichiro Den wrote:
> Extend dw_pcie_ep_set_bar() to support inbound mappings for BAR
> subranges using Address Match Mode IB iATU when pci_epf_bar.num_submap
> is non-zero.
>
> Rename the existing BAR-match helper into dw_pcie_ep_ib_atu_bar() and
> introduce dw_pcie_ep_ib_atu_addr() for Address Match Mode. When
> num_submap is non-zero, read the assigned BAR base address and program
> one inbound iATU window per subrange. Validate the submap array before
> programming:
> - each subrange is aligned to pci->region_align
> - subranges cover the whole BAR (no gaps and no overlaps)
> - subranges are sorted in ascending order by offset
>
> Track Address Match Mode mappings and tear them down on clear_bar() and
> on set_bar() error paths to avoid leaving half-programmed state or
> untranslated BAR holes.
>
> Advertise this capability by extending the common feature bit
> initializer macro (DWC_EPC_COMMON_FEATURES).
>
> This enables multiple inbound windows within a single BAR, which is
> useful on platforms where usable BARs are scarce but EPFs need multiple
> inbound regions.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> @@ -331,6 +503,13 @@ 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;
>
> + /*
> + * When dynamically changing a BAR, tear down any existing
> + * mappings before re-programming.
> + */
> + 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.
> @@ -369,8 +548,12 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> else
> type = PCIE_ATU_TYPE_IO;
>
> - ret = dw_pcie_ep_inbound_atu(ep, func_no, type, epf_bar->phys_addr, bar,
> - size);
> + if (epf_bar->num_submap)
> + ret = dw_pcie_ep_ib_atu_addr(ep, func_no, type, epf_bar);
> + else
> + ret = dw_pcie_ep_ib_atu_bar(ep, func_no, type,
> + epf_bar->phys_addr, bar, size);
If someone calls set_bar() with a submap, without having called set_bar() first
without a submap, we will still call dw_pcie_ep_ib_atu_addr() here.
To make sure that dw_pcie_ep_ib_atu_addr() cannot be called without already
having a BAR configured, to we perhaps want something like:
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 0567552b784c..fe26b7f7b212 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -487,6 +487,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
if ((flags & PCI_BASE_ADDRESS_MEM_TYPE_64) && (bar & 1))
return -EINVAL;
+ if (!ep->epf_bar[bar] && epf_bar->num_submap)
+ return -EINVAL;
+
/*
* Certain EPF drivers dynamically change the physical address of a BAR
* (i.e. they call set_bar() twice, without ever calling clear_bar(), as
or
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 0567552b784c..8aeaa6fe53f9 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -475,6 +475,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
enum pci_barno bar = epf_bar->barno;
+ bool use_addr_match_mode = false;
size_t size = epf_bar->size;
enum pci_epc_bar_type bar_type;
int flags = epf_bar->flags;
@@ -510,6 +511,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
if (ep->epf_bar[bar]->num_submap || epf_bar->num_submap)
dw_pcie_ep_clear_ib_maps(ep, bar);
+ if (epf_bar->num_submap)
+ use_addr_match_mode = true;
+
/*
* When dynamically changing a BAR, skip writing the BAR reg, as
* that would clear the BAR's PCI address assigned by the host.
@@ -548,7 +552,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
else
type = PCIE_ATU_TYPE_IO;
- if (epf_bar->num_submap)
+ if (use_addr_match_mode)
ret = dw_pcie_ep_ib_atu_addr(ep, func_no, type, epf_bar);
else
ret = dw_pcie_ep_ib_atu_bar(ep, func_no, type,
On Thu, Jan 22, 2026 at 10:23:03AM +0100, Niklas Cassel wrote:
> On Thu, Jan 22, 2026 at 05:49:08PM +0900, Koichiro Den wrote:
> > Extend dw_pcie_ep_set_bar() to support inbound mappings for BAR
> > subranges using Address Match Mode IB iATU when pci_epf_bar.num_submap
> > is non-zero.
> >
> > Rename the existing BAR-match helper into dw_pcie_ep_ib_atu_bar() and
> > introduce dw_pcie_ep_ib_atu_addr() for Address Match Mode. When
> > num_submap is non-zero, read the assigned BAR base address and program
> > one inbound iATU window per subrange. Validate the submap array before
> > programming:
> > - each subrange is aligned to pci->region_align
> > - subranges cover the whole BAR (no gaps and no overlaps)
> > - subranges are sorted in ascending order by offset
> >
> > Track Address Match Mode mappings and tear them down on clear_bar() and
> > on set_bar() error paths to avoid leaving half-programmed state or
> > untranslated BAR holes.
> >
> > Advertise this capability by extending the common feature bit
> > initializer macro (DWC_EPC_COMMON_FEATURES).
> >
> > This enables multiple inbound windows within a single BAR, which is
> > useful on platforms where usable BARs are scarce but EPFs need multiple
> > inbound regions.
> >
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
>
>
> > @@ -331,6 +503,13 @@ 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;
> >
> > + /*
> > + * When dynamically changing a BAR, tear down any existing
> > + * mappings before re-programming.
> > + */
> > + 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.
> > @@ -369,8 +548,12 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > else
> > type = PCIE_ATU_TYPE_IO;
> >
> > - ret = dw_pcie_ep_inbound_atu(ep, func_no, type, epf_bar->phys_addr, bar,
> > - size);
> > + if (epf_bar->num_submap)
> > + ret = dw_pcie_ep_ib_atu_addr(ep, func_no, type, epf_bar);
> > + else
> > + ret = dw_pcie_ep_ib_atu_bar(ep, func_no, type,
> > + epf_bar->phys_addr, bar, size);
>
> If someone calls set_bar() with a submap, without having called set_bar() first
> without a submap, we will still call dw_pcie_ep_ib_atu_addr() here.
>
> To make sure that dw_pcie_ep_ib_atu_addr() cannot be called without already
> having a BAR configured, to we perhaps want something like:
Thanks for the review.
Isn't the existing guard in dw_pcie_ep_ib_atu_addr sufficient?
[...]
base = dw_pcie_ep_read_bar_assigned(ep, func_no, bar, epf_bar->flags);
if (!base) {
dev_err(dev,
"BAR%u not assigned, cannot set up sub-range mappings\n",
bar);
return -EINVAL;
}
Koichiro
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 0567552b784c..fe26b7f7b212 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -487,6 +487,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> if ((flags & PCI_BASE_ADDRESS_MEM_TYPE_64) && (bar & 1))
> return -EINVAL;
>
> + if (!ep->epf_bar[bar] && epf_bar->num_submap)
> + return -EINVAL;
> +
> /*
> * Certain EPF drivers dynamically change the physical address of a BAR
> * (i.e. they call set_bar() twice, without ever calling clear_bar(), as
>
>
> or
>
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 0567552b784c..8aeaa6fe53f9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -475,6 +475,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> enum pci_barno bar = epf_bar->barno;
> + bool use_addr_match_mode = false;
> size_t size = epf_bar->size;
> enum pci_epc_bar_type bar_type;
> int flags = epf_bar->flags;
> @@ -510,6 +511,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> if (ep->epf_bar[bar]->num_submap || epf_bar->num_submap)
> dw_pcie_ep_clear_ib_maps(ep, bar);
>
> + if (epf_bar->num_submap)
> + use_addr_match_mode = true;
> +
> /*
> * When dynamically changing a BAR, skip writing the BAR reg, as
> * that would clear the BAR's PCI address assigned by the host.
> @@ -548,7 +552,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> else
> type = PCIE_ATU_TYPE_IO;
>
> - if (epf_bar->num_submap)
> + if (use_addr_match_mode)
> ret = dw_pcie_ep_ib_atu_addr(ep, func_no, type, epf_bar);
> else
> ret = dw_pcie_ep_ib_atu_bar(ep, func_no, type,
On 22 January 2026 15:29:02 CET, Koichiro Den <den@valinux.co.jp> wrote:
>
>> To make sure that dw_pcie_ep_ib_atu_addr() cannot be called without already
>> having a BAR configured, to we perhaps want something like:
>
>Thanks for the review.
>Isn't the existing guard in dw_pcie_ep_ib_atu_addr sufficient?
>
> [...]
> base = dw_pcie_ep_read_bar_assigned(ep, func_no, bar, epf_bar->flags);
> if (!base) {
> dev_err(dev,
> "BAR%u not assigned, cannot set up sub-range mappings\n",
> bar);
> return -EINVAL;
> }
>
Well, for a driver that does not call dw_pcie_ep_reset_bar() in their .init() to disable all BARs that are enabled in the controller by default, the host side will assign an PCI address even if no EPF has called set_bar() on that BAR.
See e.g.
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/drivers/pci/controller/dwc/pcie-tegra194.c?h=controller/dwc&id=42f9c66a6d0cc45758dab77233c5460e1cf003df
There might be other EPC drivers that don't disable all BARs in their .init(), so I would say that simply checking if the BAR has an address is not sufficient to guarantee that an EPF driver has called set_bar().
I think the safest option is my second suggestion because then we know that we will only call
dw_pcie_ep_ib_atu_addr()
When:
1) If ep->epf_bar[bar] is set:
https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L363
2) All the other requirements to dynamically update a BAR is also met:
https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L368-L370
Kind regards,
Niklas
On Thu, Jan 22, 2026 at 05:59:18PM +0100, Niklas Cassel wrote:
> On 22 January 2026 15:29:02 CET, Koichiro Den <den@valinux.co.jp> wrote:
> >
> >> To make sure that dw_pcie_ep_ib_atu_addr() cannot be called without already
> >> having a BAR configured, to we perhaps want something like:
> >
> >Thanks for the review.
> >Isn't the existing guard in dw_pcie_ep_ib_atu_addr sufficient?
> >
> > [...]
> > base = dw_pcie_ep_read_bar_assigned(ep, func_no, bar, epf_bar->flags);
> > if (!base) {
> > dev_err(dev,
> > "BAR%u not assigned, cannot set up sub-range mappings\n",
> > bar);
> > return -EINVAL;
> > }
> >
>
> Well, for a driver that does not call dw_pcie_ep_reset_bar() in their .init() to disable all BARs that are enabled in the controller by default, the host side will assign an PCI address even if no EPF has called set_bar() on that BAR.
Thanks for the explanation.
>
> See e.g.
> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/drivers/pci/controller/dwc/pcie-tegra194.c?h=controller/dwc&id=42f9c66a6d0cc45758dab77233c5460e1cf003df
>
> There might be other EPC drivers that don't disable all BARs in their .init(), so I would say that simply checking if the BAR has an address is not sufficient to guarantee that an EPF driver has called set_bar().
>
Even if an EPC driver does not reset the BAR in their .init() and some
default translation is left exposed, wouldn't it be safe as long as
dw_pcie_ep_ib_atu_addr() succeeds in programming inbound mappings for the
entire BAR?
That said, such usage apparently contradicts the documented usage (1st
set_bar with no submap, then with submap) described in the docs and commit
messages in this series, and allowing it would make things unnecessarily
complicated. So I agree that adding such a safeguard is the right approach.
>
> I think the safest option is my second suggestion because then we know that we will only call
> dw_pcie_ep_ib_atu_addr()
>
> When:
>
> 1) If ep->epf_bar[bar] is set:
> https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L363
>
>
> 2) All the other requirements to dynamically update a BAR is also met:
>
> https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L368-L370
>
That makes sense, and it ensures that the behavior always accords with the
docs and commit messages in this series.
Thanks a lot for the careful review,
Koichiro
>
>
> Kind regards,
> Niklas
>
On Fri, Jan 23, 2026 at 10:16:21AM +0900, Koichiro Den wrote: > > > > There might be other EPC drivers that don't disable all BARs in their .init(), so I would say that simply checking if the BAR has an address is not sufficient to guarantee that an EPF driver has called set_bar(). > > > > Even if an EPC driver does not reset the BAR in their .init() and some > default translation is left exposed, wouldn't it be safe as long as > dw_pcie_ep_ib_atu_addr() succeeds in programming inbound mappings for the > entire BAR? For e.g. on RK3588, the default HW configuration of the DWC controller has all 5 BARs as enabled, with a size of 1 GB. There is no inbound address translation for these BARs by default. So for it to be safe, the size of the set_bar() call would have to match the current size of the BAR, but how should the EPF driver know that when it has not called set_bar() yet? dw_pcie_ep_read_bar_assigned() does not return the current size of the BAR. So you can't verify that the set_bar() call has the same size as the BARs "default size". > > That said, such usage apparently contradicts the documented usage (1st > set_bar with no submap, then with submap) described in the docs and commit > messages in this series, and allowing it would make things unnecessarily > complicated. So I agree that adding such a safeguard is the right approach. > > > > > I think the safest option is my second suggestion because then we know that we will only call > > dw_pcie_ep_ib_atu_addr() > > > > When: > > > > 1) If ep->epf_bar[bar] is set: > > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L363 > > > > > > 2) All the other requirements to dynamically update a BAR is also met: > > > > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L368-L370 > > > > That makes sense, and it ensures that the behavior always accords with the > docs and commit messages in this series. I think it makes most sense to put the "use_addr_translation = true" after the check: /* * We can only dynamically change a BAR if the new BAR size and * BAR flags do not differ from the existing configuration. */ if (ep->epf_bar[bar]->barno != bar || ep->epf_bar[bar]->size != size || ep->epf_bar[bar]->flags != flags) return -EINVAL; So we know that dw_pcie_ep_ib_atu_addr() is only called when the size is the same. Kind regards, Niklas
On Fri, Jan 23, 2026 at 09:51:19AM +0100, Niklas Cassel wrote: > On Fri, Jan 23, 2026 at 10:16:21AM +0900, Koichiro Den wrote: > > > > > > There might be other EPC drivers that don't disable all BARs in their .init(), so I would say that simply checking if the BAR has an address is not sufficient to guarantee that an EPF driver has called set_bar(). > > > > > > > Even if an EPC driver does not reset the BAR in their .init() and some > > default translation is left exposed, wouldn't it be safe as long as > > dw_pcie_ep_ib_atu_addr() succeeds in programming inbound mappings for the > > entire BAR? > > For e.g. on RK3588, the default HW configuration of the DWC controller has > all 5 BARs as enabled, with a size of 1 GB. > > There is no inbound address translation for these BARs by default. > > So for it to be safe, the size of the set_bar() call would have to > match the current size of the BAR, but how should the EPF driver know > that when it has not called set_bar() yet? > > dw_pcie_ep_read_bar_assigned() does not return the current size of the > BAR. So you can't verify that the set_bar() call has the same size as > the BARs "default size". I wasn't considering either of the following cases as unsafe: - succeeding by chance in programming via a one-shot set_bar() with submaps - such a set_bar() failing (due to incorrect size recognition) while as I mentioned in my previous reply, the first case effectively becomes a loophole that contradicts the docs and git commit messages. However, since v8, the second case clears any existing mappings, which could indeed lead to an unsafe situtation. > > > > > > That said, such usage apparently contradicts the documented usage (1st > > set_bar with no submap, then with submap) described in the docs and commit > > messages in this series, and allowing it would make things unnecessarily > > complicated. So I agree that adding such a safeguard is the right approach. > > > > > > > > I think the safest option is my second suggestion because then we know that we will only call > > > dw_pcie_ep_ib_atu_addr() > > > > > > When: > > > > > > 1) If ep->epf_bar[bar] is set: > > > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L363 > > > > > > > > > 2) All the other requirements to dynamically update a BAR is also met: > > > > > > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L368-L370 > > > > > > > That makes sense, and it ensures that the behavior always accords with the > > docs and commit messages in this series. > > I think it makes most sense to put the "use_addr_translation = true" > > after the check: > > /* > * We can only dynamically change a BAR if the new BAR size and > * BAR flags do not differ from the existing configuration. > */ > if (ep->epf_bar[bar]->barno != bar || > ep->epf_bar[bar]->size != size || > ep->epf_bar[bar]->flags != flags) > return -EINVAL; > > > So we know that dw_pcie_ep_ib_atu_addr() is only called when the size is the > same. I'll send v10 with the fix, possibly adding a BAR_SUBRANGE_TEST to pci endpoint test as well. Kind regards, Koichiro > > > Kind regards, > Niklas
On Fri, Jan 23, 2026 at 11:28:30PM +0900, Koichiro Den wrote:
> On Fri, Jan 23, 2026 at 09:51:19AM +0100, Niklas Cassel wrote:
> > On Fri, Jan 23, 2026 at 10:16:21AM +0900, Koichiro Den wrote:
> > > >
> > > > There might be other EPC drivers that don't disable all BARs in their .init(), so I would say that simply checking if the BAR has an address is not sufficient to guarantee that an EPF driver has called set_bar().
> > > >
> > >
> > > Even if an EPC driver does not reset the BAR in their .init() and some
> > > default translation is left exposed, wouldn't it be safe as long as
> > > dw_pcie_ep_ib_atu_addr() succeeds in programming inbound mappings for the
> > > entire BAR?
> >
> > For e.g. on RK3588, the default HW configuration of the DWC controller has
> > all 5 BARs as enabled, with a size of 1 GB.
> >
> > There is no inbound address translation for these BARs by default.
> >
> > So for it to be safe, the size of the set_bar() call would have to
> > match the current size of the BAR, but how should the EPF driver know
> > that when it has not called set_bar() yet?
> >
> > dw_pcie_ep_read_bar_assigned() does not return the current size of the
> > BAR. So you can't verify that the set_bar() call has the same size as
> > the BARs "default size".
>
> I wasn't considering either of the following cases as unsafe:
> - succeeding by chance in programming via a one-shot set_bar() with submaps
> - such a set_bar() failing (due to incorrect size recognition)
>
> while as I mentioned in my previous reply, the first case effectively
> becomes a loophole that contradicts the docs and git commit messages.
>
> However, since v8, the second case clears any existing mappings, which
> could indeed lead to an unsafe situtation.
>
> >
> >
> > >
> > > That said, such usage apparently contradicts the documented usage (1st
> > > set_bar with no submap, then with submap) described in the docs and commit
> > > messages in this series, and allowing it would make things unnecessarily
> > > complicated. So I agree that adding such a safeguard is the right approach.
> > >
> > > >
> > > > I think the safest option is my second suggestion because then we know that we will only call
> > > > dw_pcie_ep_ib_atu_addr()
> > > >
> > > > When:
> > > >
> > > > 1) If ep->epf_bar[bar] is set:
> > > > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L363
> > > >
> > > >
> > > > 2) All the other requirements to dynamically update a BAR is also met:
> > > >
> > > > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L368-L370
> > > >
> > >
> > > That makes sense, and it ensures that the behavior always accords with the
> > > docs and commit messages in this series.
> >
> > I think it makes most sense to put the "use_addr_translation = true"
> >
> > after the check:
> >
> > /*
> > * We can only dynamically change a BAR if the new BAR size and
> > * BAR flags do not differ from the existing configuration.
> > */
> > if (ep->epf_bar[bar]->barno != bar ||
> > ep->epf_bar[bar]->size != size ||
> > ep->epf_bar[bar]->flags != flags)
> > return -EINVAL;
> >
> >
> > So we know that dw_pcie_ep_ib_atu_addr() is only called when the size is the
> > same.
>
> I'll send v10 with the fix, possibly adding a BAR_SUBRANGE_TEST to pci
> endpoint test as well.
After thinking again, I believe just the following is the most robust and
safest approach, as it makes subrange mapping strictly update-only and
avoids any silent success on invalid first-time calls.
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -508,20 +508,29 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
* mappings before re-programming.
*/
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.
*/
goto config_atu;
+ } else {
+ /*
+ * Subrange mapping is an update-only operation.
+ * The BAR must have been configured once without submaps so that
+ * subsequent set_bar() calls can update inbound mappings without
+ * touching the BAR register (and clobbering the host-assigned address).
+ */
+ if (epf_bar->num_submap)
+ return -EINVAL;
}
bar_type = dw_pcie_ep_get_bar_type(ep, bar);
switch (bar_type) {
case BAR_FIXED:
/*
* There is no need to write a BAR mask for a fixed BAR (except
* to write 1 to the LSB of the BAR mask register, to enable the
* BAR). Write the BAR mask regardless. (The fixed bits in the
* BAR mask register will be read-only anyway.)
This is close to your first suggestion at:
https://lore.kernel.org/linux-pci/aXHsd7-WWAGyhy_w@ryzen/
but it avoids even performing BAR sizing when set_bar() is called in an invalid manner.
With this, we still guarantee dw_pcie_ep_ib_atu_addr() is only reached when:
1) ep->epf_bar[bar] is set
2) All the other requirements to dynamically update a BAR is also met
The resulting behavior matrix becomes:
| num_submap > 0 | num_submap == 0 |
-------------------------+--------------------------+--------------------------+
ep->epf_bar[bar] == NULL | returns -EINVAL | always try BAR Match |
ep->epf_bar[bar] != NULL | always try Address Match | always try BAR Match |
By contrast, with the latest idea that relies on the local
"use_addr_translation" variable, the case marked as [1] below possibly
leads to an unexpected success in BAR Match Mode, .submap/.num_submap are
silently ignored, and the caller has no way to notice the mistake.
| num_submap > 0 | num_submap == 0 |
-------------------------+--------------------------+--------------------------+
ep->epf_bar[bar] == NULL | always try BAR Match [1] | always try BAR Match |
ep->epf_bar[bar] != NULL | always try Address Match | always try BAR Match |
Kind regards,
Koichiro
>
> Kind regards,
> Koichiro
>
> >
> >
> > Kind regards,
> > Niklas
© 2016 - 2026 Red Hat, Inc.