Extend dw_pcie_ep_set_bar() to support inbound mappings for BAR
subranges using Address Match Mode IB iATU.
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
use_submap is set, 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 setting subrange_mapping in the EPC
features returned from dw_pcie_ep_get_features().
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
.../pci/controller/dwc/pcie-designware-ep.c | 242 +++++++++++++++++-
drivers/pci/controller/dwc/pcie-designware.h | 2 +
2 files changed, 232 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 1195d401df19..406e9218e4ea 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -139,9 +139,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;
@@ -174,6 +175,208 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
return 0;
}
+/* Inbound mapping bookkeeping for Address Match Mode */
+struct dw_pcie_ib_map {
+ struct list_head list;
+ enum pci_barno bar;
+ u64 pci_addr;
+ u64 parent_bus_addr;
+ u64 size;
+ u32 index;
+};
+
+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 dw_pcie_ib_map *m, *tmp;
+ struct device *dev = pci->dev;
+ u32 atu_index;
+
+ /* 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. */
+ guard(spinlock_irqsave)(&ep->ib_map_lock);
+ list_for_each_entry_safe(m, tmp, &ep->ib_map_list, list) {
+ if (m->bar != bar)
+ continue;
+ dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, m->index);
+ clear_bit(m->index, ep->ib_window_map);
+ list_del(&m->list);
+ devm_kfree(dev, m);
+ }
+}
+
+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 expected = 0;
+ size_t size, off;
+ unsigned int i;
+
+ if (!align || !IS_ALIGNED(bar_size, align))
+ return -EINVAL;
+
+ /*
+ * The array is expected to be sorted by offset before calling this
+ * helper. With sorted entries, we can enforce a strict, gapless
+ * decomposition of the BAR:
+ * - each entry has a non-zero size
+ * - offset/size/phys_addr are aligned to pci->region_align
+ * - each entry lies within the BAR range
+ * - entries are contiguous (no overlaps, no holes)
+ * - the entries exactly cover the whole BAR
+ *
+ * Note: dw_pcie_prog_inbound_atu() also checks alignment for
+ * offset/phys_addr, but validating up-front avoids partially
+ * programming iATU windows in vain.
+ */
+ for (i = 0; i < num_submap; i++) {
+ off = submap[i].offset;
+ 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;
+
+ /* Enforce contiguity (no overlaps, no holes). */
+ if (off != expected)
+ return -EINVAL;
+
+ expected += size;
+ }
+ if (expected != 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;
+ struct dw_pcie_ib_map *new;
+ u64 size, off, base;
+ unsigned long flags;
+ int free_win, ret;
+ unsigned int i;
+
+ 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;
+ }
+
+ /* 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;
+ parent_bus_addr = submap[i].phys_addr;
+
+ if (off > (~0ULL) - base) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ pci_addr = base + off;
+
+ new = devm_kzalloc(dev, sizeof(*new), GFP_KERNEL);
+ if (!new) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ spin_lock_irqsave(&ep->ib_map_lock, flags);
+
+ free_win = find_first_zero_bit(ep->ib_window_map,
+ pci->num_ib_windows);
+ if (free_win >= pci->num_ib_windows) {
+ spin_unlock_irqrestore(&ep->ib_map_lock, flags);
+ devm_kfree(dev, new);
+ ret = -ENOSPC;
+ goto err;
+ }
+ set_bit(free_win, ep->ib_window_map);
+
+ new->bar = bar;
+ new->index = free_win;
+ new->pci_addr = pci_addr;
+ new->parent_bus_addr = parent_bus_addr;
+ new->size = size;
+ list_add_tail(&new->list, &ep->ib_map_list);
+
+ spin_unlock_irqrestore(&ep->ib_map_lock, flags);
+
+ ret = dw_pcie_prog_inbound_atu(pci, free_win, type,
+ parent_bus_addr, pci_addr, size);
+ if (ret) {
+ spin_lock_irqsave(&ep->ib_map_lock, flags);
+ list_del(&new->list);
+ clear_bit(free_win, ep->ib_window_map);
+ spin_unlock_irqrestore(&ep->ib_map_lock, flags);
+ devm_kfree(dev, new);
+ goto err;
+ }
+ }
+ 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)
{
@@ -204,17 +407,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,
@@ -408,10 +609,17 @@ 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 (ret)
+ if (epf_bar->use_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) {
+ if (epf_bar->use_submap)
+ dw_pcie_ep_clear_bar(epc, func_no, vfunc_no, epf_bar);
return ret;
+ }
ep->epf_bar[bar] = epf_bar;
@@ -626,11 +834,19 @@ static const struct pci_epc_features*
dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+ struct pci_epc_features *features;
if (!ep->ops->get_features)
return NULL;
- return ep->ops->get_features(ep);
+ features = ep->ops->get_features(ep);
+ if (!features)
+ return NULL;
+
+ /* All DWC-based glue drivers support inbound subrange mapping */
+ features->subrange_mapping = true;
+
+ return features;
}
static const struct pci_epc_ops epc_ops = {
@@ -1120,6 +1336,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
struct device *dev = pci->dev;
INIT_LIST_HEAD(&ep->func_list);
+ INIT_LIST_HEAD(&ep->ib_map_list);
+ spin_lock_init(&ep->ib_map_lock);
ep->msi_iatu_mapped = false;
ep->msi_msg_addr = 0;
ep->msi_map_size = 0;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 4dda9a38d46b..969b1f32dddf 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -479,6 +479,8 @@ struct dw_pcie_ep {
phys_addr_t *outbound_addr;
unsigned long *ib_window_map;
unsigned long *ob_window_map;
+ struct list_head ib_map_list;
+ spinlock_t ib_map_lock;
void __iomem *msi_mem;
phys_addr_t msi_mem_phys;
struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
--
2.51.0
On Fri, Jan 09, 2026 at 02:24:03AM +0900, Koichiro Den wrote:
> Extend dw_pcie_ep_set_bar() to support inbound mappings for BAR
> subranges using Address Match Mode IB iATU.
>
> 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
> use_submap is set, 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 setting subrange_mapping in the EPC
> features returned from dw_pcie_ep_get_features().
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 242 +++++++++++++++++-
> drivers/pci/controller/dwc/pcie-designware.h | 2 +
> 2 files changed, 232 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 1195d401df19..406e9218e4ea 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -139,9 +139,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;
> @@ -174,6 +175,208 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> return 0;
> }
>
> +/* Inbound mapping bookkeeping for Address Match Mode */
> +struct dw_pcie_ib_map {
> + struct list_head list;
> + enum pci_barno bar;
> + u64 pci_addr;
> + u64 parent_bus_addr;
> + u64 size;
> + u32 index;
> +};
> +
> +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 dw_pcie_ib_map *m, *tmp;
> + struct device *dev = pci->dev;
> + u32 atu_index;
> +
> + /* 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. */
> + guard(spinlock_irqsave)(&ep->ib_map_lock);
> + list_for_each_entry_safe(m, tmp, &ep->ib_map_list, list) {
> + if (m->bar != bar)
> + continue;
> + dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, m->index);
> + clear_bit(m->index, ep->ib_window_map);
> + list_del(&m->list);
> + devm_kfree(dev, m);
> + }
> +}
> +
> +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 expected = 0;
> + size_t size, off;
> + unsigned int i;
> +
> + if (!align || !IS_ALIGNED(bar_size, align))
> + return -EINVAL;
> +
> + /*
> + * The array is expected to be sorted by offset before calling this
> + * helper. With sorted entries, we can enforce a strict, gapless
> + * decomposition of the BAR:
> + * - each entry has a non-zero size
> + * - offset/size/phys_addr are aligned to pci->region_align
> + * - each entry lies within the BAR range
> + * - entries are contiguous (no overlaps, no holes)
> + * - the entries exactly cover the whole BAR
> + *
> + * Note: dw_pcie_prog_inbound_atu() also checks alignment for
> + * offset/phys_addr, but validating up-front avoids partially
> + * programming iATU windows in vain.
> + */
> + for (i = 0; i < num_submap; i++) {
> + off = submap[i].offset;
> + 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;
> +
> + /* Enforce contiguity (no overlaps, no holes). */
> + if (off != expected)
> + return -EINVAL;
> +
> + expected += size;
> + }
> + if (expected != 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;
> + struct dw_pcie_ib_map *new;
> + u64 size, off, base;
> + unsigned long flags;
> + int free_win, ret;
> + unsigned int i;
> +
> + 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;
> + }
> +
> + /* 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;
> + parent_bus_addr = submap[i].phys_addr;
> +
> + if (off > (~0ULL) - base) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + pci_addr = base + off;
> +
> + new = devm_kzalloc(dev, sizeof(*new), GFP_KERNEL);
> + if (!new) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + spin_lock_irqsave(&ep->ib_map_lock, flags);
> +
> + free_win = find_first_zero_bit(ep->ib_window_map,
> + pci->num_ib_windows);
> + if (free_win >= pci->num_ib_windows) {
> + spin_unlock_irqrestore(&ep->ib_map_lock, flags);
> + devm_kfree(dev, new);
> + ret = -ENOSPC;
> + goto err;
> + }
> + set_bit(free_win, ep->ib_window_map);
> +
> + new->bar = bar;
> + new->index = free_win;
> + new->pci_addr = pci_addr;
> + new->parent_bus_addr = parent_bus_addr;
> + new->size = size;
> + list_add_tail(&new->list, &ep->ib_map_list);
> +
> + spin_unlock_irqrestore(&ep->ib_map_lock, flags);
> +
> + ret = dw_pcie_prog_inbound_atu(pci, free_win, type,
> + parent_bus_addr, pci_addr, size);
> + if (ret) {
> + spin_lock_irqsave(&ep->ib_map_lock, flags);
> + list_del(&new->list);
> + clear_bit(free_win, ep->ib_window_map);
> + spin_unlock_irqrestore(&ep->ib_map_lock, flags);
> + devm_kfree(dev, new);
> + goto err;
> + }
> + }
> + 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)
> {
> @@ -204,17 +407,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,
> @@ -408,10 +609,17 @@ 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 (ret)
> + if (epf_bar->use_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) {
> + if (epf_bar->use_submap)
> + dw_pcie_ep_clear_bar(epc, func_no, vfunc_no, epf_bar);
I just noticed that this dw_pcie_ep_clear_bar() should be dropped, since it
goes too far and resets the BAR. Also dw_pcie_ep_ib_atu_addr() already
rolls back partially programmed inbound mappings on failure, so this extra
clear_bar() is unnecessary. I'll drop this in v6.
Koichiro
> return ret;
> + }
>
> ep->epf_bar[bar] = epf_bar;
>
> @@ -626,11 +834,19 @@ static const struct pci_epc_features*
> dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> + struct pci_epc_features *features;
>
> if (!ep->ops->get_features)
> return NULL;
>
> - return ep->ops->get_features(ep);
> + features = ep->ops->get_features(ep);
> + if (!features)
> + return NULL;
> +
> + /* All DWC-based glue drivers support inbound subrange mapping */
> + features->subrange_mapping = true;
> +
> + return features;
> }
>
> static const struct pci_epc_ops epc_ops = {
> @@ -1120,6 +1336,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> struct device *dev = pci->dev;
>
> INIT_LIST_HEAD(&ep->func_list);
> + INIT_LIST_HEAD(&ep->ib_map_list);
> + spin_lock_init(&ep->ib_map_lock);
> ep->msi_iatu_mapped = false;
> ep->msi_msg_addr = 0;
> ep->msi_map_size = 0;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 4dda9a38d46b..969b1f32dddf 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -479,6 +479,8 @@ struct dw_pcie_ep {
> phys_addr_t *outbound_addr;
> unsigned long *ib_window_map;
> unsigned long *ob_window_map;
> + struct list_head ib_map_list;
> + spinlock_t ib_map_lock;
> void __iomem *msi_mem;
> phys_addr_t msi_mem_phys;
> struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
> --
> 2.51.0
>
Hello Koichiro,
On Fri, Jan 09, 2026 at 02:24:03AM +0900, Koichiro Den wrote:
(snip)
> +/* 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;
> + struct dw_pcie_ib_map *new;
> + u64 size, off, base;
> + unsigned long flags;
> + int free_win, ret;
> + unsigned int i;
> +
> + 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;
> + }
Sorry for giving additional review comments.
But there is one thing that I might not be so obvious for someone just
reading this source. How is this API supposed to be used in practice?
Most DWC-based controllers are not hotplug capable.
That means that we must boot the EP, create the EPF symlink in configfs,
and start link training by writing to configfs, before starting the host.
dw_pcie_ep_ib_atu_addr() reads the PCI address that the host has assigned
to the BAR, and returns an error if the host has not already assigned a
PCI addres to the BAR.
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.
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.
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.
So I can only assume that you will call set_bar() twice.
Once with BAR match mode, and then a second time with address map mode.
It might be obvious to you, but I think it makes sense to also have some
kind of documentation for this feature.
Kind regards,
Niklas
On Thu, Jan 08, 2026 at 09:55:27PM +0100, Niklas Cassel wrote:
> Hello Koichiro,
>
> On Fri, Jan 09, 2026 at 02:24:03AM +0900, Koichiro Den wrote:
>
> (snip)
>
> > +/* 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;
> > + struct dw_pcie_ib_map *new;
> > + u64 size, off, base;
> > + unsigned long flags;
> > + int free_win, ret;
> > + unsigned int i;
> > +
> > + 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;
> > + }
>
> Sorry for giving additional review comments.
Thanks again for the detailed feedback.
>
> But there is one thing that I might not be so obvious for someone just
> reading this source. How is this API supposed to be used in practice?
>
> Most DWC-based controllers are not hotplug capable.
>
> That means that we must boot the EP, create the EPF symlink in configfs,
> and start link training by writing to configfs, before starting the host.
>
> dw_pcie_ep_ib_atu_addr() reads the PCI address that the host has assigned
> to the BAR, and returns an error if the host has not already assigned a
> PCI addres to the BAR.
>
> 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.
>
>
> 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.
>
> So I can only assume that you will call set_bar() twice.
> Once with BAR match mode, and then a second time with address map mode.
>
> It might be obvious to you, but I think it makes sense to also have some
> kind of documentation for this feature.
Ok, I'll update Documentation/PCI/endpoint/pci-endpoint.rst accordingly.
Kind regards,
Koichiro
>
>
> Kind regards,
> Niklas
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
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
Hello Koichiro, On Sat, Jan 10, 2026 at 11:29:06PM +0900, Koichiro Den wrote: (snip) > > 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. Please fold it into the next iteration (=v6). It should be a one liner patch in the DWC driver, at least if you put it after your "PCI: dwc: Allow glue drivers to return mutable EPC features" patch. Thank you for all your efforts on improving the endpoint framework. Kind regards, Niklas
On Mon, Jan 12, 2026 at 12:43:50PM +0100, Niklas Cassel wrote: > Hello Koichiro, > > On Sat, Jan 10, 2026 at 11:29:06PM +0900, Koichiro Den wrote: > > (snip) > > > > 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. > > Please fold it into the next iteration (=v6). > > It should be a one liner patch in the DWC driver, at least if you put it > after your "PCI: dwc: Allow glue drivers to return mutable EPC features" > patch. > > Thank you for all your efforts on improving the endpoint framework. I just submitted v6: https://lore.kernel.org/all/20260113023715.3463724-1-den@valinux.co.jp/ Thank you very much for the review, Koichiro > > > Kind regards, > Niklas
© 2016 - 2026 Red Hat, Inc.