* [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support
@ 2026-01-21 5:42 Aksh Garg
2026-01-21 5:42 ` [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations Aksh Garg
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Aksh Garg @ 2026-01-21 5:42 UTC (permalink / raw)
To: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh,
bhelgaas, cassel
Cc: linux-kernel, s-vadapalli, danishanwar, Aksh Garg
This series addresses multi-function endpoint configuration issues in
the DWC PCIe controller driver. The changes enable proper operations
for physical functions and enhance the multi-function endpoint support.
Aksh Garg (2):
PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations
PCI: dwc: ep: Add per-PF BAR and iATU mapping support
.../pci/controller/dwc/pcie-designware-ep.c | 97 +++++++++++++------
drivers/pci/controller/dwc/pcie-designware.h | 4 +-
2 files changed, 69 insertions(+), 32 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations 2026-01-21 5:42 [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support Aksh Garg @ 2026-01-21 5:42 ` Aksh Garg 2026-01-21 9:58 ` Niklas Cassel 2026-01-21 5:42 ` [PATCH 2/2] PCI: dwc: ep: Add per-PF BAR and iATU mapping support Aksh Garg 2026-01-21 8:50 ` [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support Niklas Cassel 2 siblings, 1 reply; 14+ messages in thread From: Aksh Garg @ 2026-01-21 5:42 UTC (permalink / raw) To: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, cassel Cc: linux-kernel, s-vadapalli, danishanwar, Aksh Garg The resizable BAR support added by the commit 3a3d4cabe681 ("PCI: dwc: ep: Allow EPF drivers to configure the size of Resizable BARs") incorrectly configures the resizable BARs only for the first Physical Function (PF0) in EP mode. The resizable BAR configuration functions use generic dw_pcie_*_dbi operations instead of physical function specific dw_pcie_ep_*_dbi operations. This causes resizable BAR configuration to always target PF0 regardless of the requested function number. Additionally, dw_pcie_ep_init_non_sticky_registers() only initializes resizable BAR registers for PF0, leaving other PFs unconfigured during the execution of this function. Fix this by using physical function specific configuration space access operations throughout the resizable BAR code path and initializing registers for all the physical functions that support resizable BARs. Fixes: 3a3d4cabe681 ("PCI: dwc: ep: Allow EPF drivers to configure the size of Resizable BARs") Signed-off-by: Aksh Garg <a-garg7@ti.com> --- .../pci/controller/dwc/pcie-designware-ep.c | 49 +++++++++++++------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 19571ac2b961..f222677a7a87 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -75,6 +75,13 @@ static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap) cap, ep, func_no); } +static u16 dw_pcie_ep_find_ext_capability(struct dw_pcie_ep *ep, + u8 func_no, u8 cap) +{ + return PCI_FIND_NEXT_EXT_CAP(dw_pcie_ep_read_cfg, 0, + cap, ep, func_no); +} + /** * dw_pcie_ep_hide_ext_capability - Hide a capability from the linked list * @pci: DWC PCI device @@ -217,22 +224,22 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, ep->bar_to_atu[bar] = 0; } -static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie *pci, +static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no, enum pci_barno bar) { u32 reg, bar_index; unsigned int offset, nbars; int i; - offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); + offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR); if (!offset) return offset; - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); + reg = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, reg); for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) { - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); + reg = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); bar_index = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, reg); if (bar_index == bar) return offset; @@ -253,7 +260,7 @@ static int dw_pcie_ep_set_bar_resizable(struct dw_pcie_ep *ep, u8 func_no, u32 rebar_cap, rebar_ctrl; int ret; - rebar_offset = dw_pcie_ep_get_rebar_offset(pci, bar); + rebar_offset = dw_pcie_ep_get_rebar_offset(ep, func_no, bar); if (!rebar_offset) return -EINVAL; @@ -283,16 +290,16 @@ static int dw_pcie_ep_set_bar_resizable(struct dw_pcie_ep *ep, u8 func_no, * 1 MB to 128 TB. Bits 31:16 in PCI_REBAR_CTRL define "supported sizes" * bits for sizes 256 TB to 8 EB. Disallow sizes 256 TB to 8 EB. */ - rebar_ctrl = dw_pcie_readl_dbi(pci, rebar_offset + PCI_REBAR_CTRL); + rebar_ctrl = dw_pcie_ep_readl_dbi(ep, func_no, rebar_offset + PCI_REBAR_CTRL); rebar_ctrl &= ~GENMASK(31, 16); - dw_pcie_writel_dbi(pci, rebar_offset + PCI_REBAR_CTRL, rebar_ctrl); + dw_pcie_ep_writel_dbi(ep, func_no, rebar_offset + PCI_REBAR_CTRL, rebar_ctrl); /* * The "selected size" (bits 13:8) in PCI_REBAR_CTRL are automatically * updated when writing PCI_REBAR_CAP, see "Figure 3-26 Resizable BAR * Example for 32-bit Memory BAR0" in DWC EP databook 5.96a. */ - dw_pcie_writel_dbi(pci, rebar_offset + PCI_REBAR_CAP, rebar_cap); + dw_pcie_ep_writel_dbi(ep, func_no, rebar_offset + PCI_REBAR_CAP, rebar_cap); dw_pcie_dbi_ro_wr_dis(pci); @@ -836,20 +843,17 @@ void dw_pcie_ep_deinit(struct dw_pcie_ep *ep) } EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit); -static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) +static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 func_no) { - struct dw_pcie_ep *ep = &pci->ep; unsigned int offset; unsigned int nbars; enum pci_barno bar; u32 reg, i, val; - offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); - - dw_pcie_dbi_ro_wr_en(pci); + offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR); if (offset) { - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); + reg = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, reg); /* @@ -870,16 +874,29 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) * the controller when RESBAR_CAP_REG is written, which * is why RESBAR_CAP_REG is written here. */ - val = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); + val = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); bar = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, val); if (ep->epf_bar[bar]) pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val); else val = BIT(4); - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, val); + dw_pcie_ep_writel_dbi(ep, func_no, offset + PCI_REBAR_CAP, val); } } +} + +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) +{ + struct dw_pcie_ep *ep = &pci->ep; + u8 func_no, funcs; + + funcs = ep->epc->max_functions; + + dw_pcie_dbi_ro_wr_en(pci); + + for (func_no = 0; func_no < funcs; func_no++) + __dw_pcie_ep_init_non_sticky_registers(ep, func_no); dw_pcie_setup(pci); dw_pcie_dbi_ro_wr_dis(pci); -- 2.34.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations 2026-01-21 5:42 ` [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations Aksh Garg @ 2026-01-21 9:58 ` Niklas Cassel 2026-01-21 11:40 ` Aksh Garg 2026-01-22 5:05 ` Aksh Garg 0 siblings, 2 replies; 14+ messages in thread From: Niklas Cassel @ 2026-01-21 9:58 UTC (permalink / raw) To: Aksh Garg Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar On Wed, Jan 21, 2026 at 11:12:13AM +0530, Aksh Garg wrote: > The resizable BAR support added by the commit 3a3d4cabe681 > ("PCI: dwc: ep: Allow EPF drivers to configure the size of Resizable > BARs") incorrectly configures the resizable BARs only for the first > Physical Function (PF0) in EP mode. > > The resizable BAR configuration functions use generic dw_pcie_*_dbi > operations instead of physical function specific dw_pcie_ep_*_dbi > operations. This causes resizable BAR configuration to always target > PF0 regardless of the requested function number. > > Additionally, dw_pcie_ep_init_non_sticky_registers() only initializes > resizable BAR registers for PF0, leaving other PFs unconfigured during > the execution of this function. > > Fix this by using physical function specific configuration space access > operations throughout the resizable BAR code path and initializing > registers for all the physical functions that support resizable BARs. > > Fixes: 3a3d4cabe681 ("PCI: dwc: ep: Allow EPF drivers to configure the size of Resizable BARs") > Signed-off-by: Aksh Garg <a-garg7@ti.com> > --- > .../pci/controller/dwc/pcie-designware-ep.c | 49 +++++++++++++------ > 1 file changed, 33 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 19571ac2b961..f222677a7a87 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -75,6 +75,13 @@ static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap) > cap, ep, func_no); > } > > +static u16 dw_pcie_ep_find_ext_capability(struct dw_pcie_ep *ep, > + u8 func_no, u8 cap) > +{ > + return PCI_FIND_NEXT_EXT_CAP(dw_pcie_ep_read_cfg, 0, > + cap, ep, func_no); > +} > + > /** > * dw_pcie_ep_hide_ext_capability - Hide a capability from the linked list > * @pci: DWC PCI device > @@ -217,22 +224,22 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > ep->bar_to_atu[bar] = 0; > } > > -static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie *pci, > +static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no, > enum pci_barno bar) > { > u32 reg, bar_index; > unsigned int offset, nbars; > int i; > > - offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > + offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR); > if (!offset) > return offset; > > - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); > + reg = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); > nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, reg); > > for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL) { > - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); > + reg = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); > bar_index = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, reg); > if (bar_index == bar) > return offset; > @@ -253,7 +260,7 @@ static int dw_pcie_ep_set_bar_resizable(struct dw_pcie_ep *ep, u8 func_no, > u32 rebar_cap, rebar_ctrl; > int ret; > > - rebar_offset = dw_pcie_ep_get_rebar_offset(pci, bar); > + rebar_offset = dw_pcie_ep_get_rebar_offset(ep, func_no, bar); > if (!rebar_offset) > return -EINVAL; > > @@ -283,16 +290,16 @@ static int dw_pcie_ep_set_bar_resizable(struct dw_pcie_ep *ep, u8 func_no, > * 1 MB to 128 TB. Bits 31:16 in PCI_REBAR_CTRL define "supported sizes" > * bits for sizes 256 TB to 8 EB. Disallow sizes 256 TB to 8 EB. > */ > - rebar_ctrl = dw_pcie_readl_dbi(pci, rebar_offset + PCI_REBAR_CTRL); > + rebar_ctrl = dw_pcie_ep_readl_dbi(ep, func_no, rebar_offset + PCI_REBAR_CTRL); > rebar_ctrl &= ~GENMASK(31, 16); > - dw_pcie_writel_dbi(pci, rebar_offset + PCI_REBAR_CTRL, rebar_ctrl); > + dw_pcie_ep_writel_dbi(ep, func_no, rebar_offset + PCI_REBAR_CTRL, rebar_ctrl); > > /* > * The "selected size" (bits 13:8) in PCI_REBAR_CTRL are automatically > * updated when writing PCI_REBAR_CAP, see "Figure 3-26 Resizable BAR > * Example for 32-bit Memory BAR0" in DWC EP databook 5.96a. > */ > - dw_pcie_writel_dbi(pci, rebar_offset + PCI_REBAR_CAP, rebar_cap); > + dw_pcie_ep_writel_dbi(ep, func_no, rebar_offset + PCI_REBAR_CAP, rebar_cap); > > dw_pcie_dbi_ro_wr_dis(pci); > > @@ -836,20 +843,17 @@ void dw_pcie_ep_deinit(struct dw_pcie_ep *ep) > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit); > > -static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) > +static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 func_no) > { > - struct dw_pcie_ep *ep = &pci->ep; > unsigned int offset; > unsigned int nbars; > enum pci_barno bar; > u32 reg, i, val; > > - offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > - > - dw_pcie_dbi_ro_wr_en(pci); > + offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR); > > if (offset) { > - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); > + reg = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); > nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, reg); > > /* > @@ -870,16 +874,29 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) > * the controller when RESBAR_CAP_REG is written, which > * is why RESBAR_CAP_REG is written here. > */ > - val = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL); > + val = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); > bar = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, val); > if (ep->epf_bar[bar]) > pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val); > else > val = BIT(4); > > - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, val); > + dw_pcie_ep_writel_dbi(ep, func_no, offset + PCI_REBAR_CAP, val); > } > } > +} > + > +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) > +{ > + struct dw_pcie_ep *ep = &pci->ep; > + u8 func_no, funcs; > + > + funcs = ep->epc->max_functions; Initialize this on the same line as the variable is declared. u8 func_no; will be on a separate line. > + > + dw_pcie_dbi_ro_wr_en(pci); > + > + for (func_no = 0; func_no < funcs; func_no++) > + __dw_pcie_ep_init_non_sticky_registers(ep, func_no); > > dw_pcie_setup(pci); > dw_pcie_dbi_ro_wr_dis(pci); > -- > 2.34.1 > Thank you for fixing this! Reviewed-by: Niklas Cassel <cassel@kernel.org> You do need another patch in this series though, that fixes: https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L972-L986 As currently, ptm_cap_base is fetched using dw_pcie_find_ext_capability() instead of your new dw_pcie_ep_find_ext_capability() which takes a func_no. Kind regards, Niklas ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations 2026-01-21 9:58 ` Niklas Cassel @ 2026-01-21 11:40 ` Aksh Garg 2026-01-22 5:05 ` Aksh Garg 1 sibling, 0 replies; 14+ messages in thread From: Aksh Garg @ 2026-01-21 11:40 UTC (permalink / raw) To: Niklas Cassel Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar > On Wed, Jan 21, 2026 at 11:12:13AM +0530, Aksh Garg wrote: >> +static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) >> +{ >> + struct dw_pcie_ep *ep = &pci->ep; >> + u8 func_no, funcs; >> + >> + funcs = ep->epc->max_functions; > > Initialize this on the same line as the variable is declared. > u8 func_no; will be on a separate line. > I will fix this nit. > >> + >> + dw_pcie_dbi_ro_wr_en(pci); >> + >> + for (func_no = 0; func_no < funcs; func_no++) >> + __dw_pcie_ep_init_non_sticky_registers(ep, func_no); >> >> dw_pcie_setup(pci); >> dw_pcie_dbi_ro_wr_dis(pci); >> -- >> 2.34.1 >> > > > Thank you for fixing this! > > Reviewed-by: Niklas Cassel <cassel@kernel.org> > > > You do need another patch in this series though, that fixes: > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L972-L986 > > As currently, ptm_cap_base is fetched using dw_pcie_find_ext_capability() > instead of your new dw_pcie_ep_find_ext_capability() which takes a func_no. > Thank you for pointing this out. I will add a patch for this fix as well. > > Kind regards, > Niklas > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations 2026-01-21 9:58 ` Niklas Cassel 2026-01-21 11:40 ` Aksh Garg @ 2026-01-22 5:05 ` Aksh Garg 2026-01-22 5:56 ` Niklas Cassel 1 sibling, 1 reply; 14+ messages in thread From: Aksh Garg @ 2026-01-22 5:05 UTC (permalink / raw) To: Niklas Cassel Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar >> -- >> 2.34.1 >> > > > Thank you for fixing this! > > Reviewed-by: Niklas Cassel <cassel@kernel.org> > > > You do need another patch in this series though, that fixes: > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L972-L986 > > As currently, ptm_cap_base is fetched using dw_pcie_find_ext_capability() > instead of your new dw_pcie_ep_find_ext_capability() which takes a func_no. > I examined the register spaces across different PFs to check whether all the PFs have the PTM capability registers, and confirmed that PTM capability registers exist only in PF0. PCIe r6.0 section 7.9.15 'Precision Time Management Extended Capability (PTM Capability)' states that " For Endpoints and Switch Upstream Ports that support PTM, this Capability is required in exactly one Function of the Upstream Port and that Capability controls the PTM behavior of all PTM capable Functions associated with that Upstream Port". This indicates that PTM capabilities are controller-level registers rather than per-function registers. Hence, in my opinion, ptm_cap_base does not require modification, since dw_pcie_find_ext_capability() and dw_pcie_*_dbi() already correctly access PF0's register space, which is the expected behavior for controller-level PTM management. > > Kind regards, > Niklas > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations 2026-01-22 5:05 ` Aksh Garg @ 2026-01-22 5:56 ` Niklas Cassel 2026-01-22 6:20 ` Aksh Garg 0 siblings, 1 reply; 14+ messages in thread From: Niklas Cassel @ 2026-01-22 5:56 UTC (permalink / raw) To: Aksh Garg Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar On 22 January 2026 06:05:47 CET, Aksh Garg <a-garg7@ti.com> wrote: >>> -- >>> 2.34.1 >>> >> >> >> Thank you for fixing this! >> >> Reviewed-by: Niklas Cassel <cassel@kernel.org> >> >> >> You do need another patch in this series though, that fixes: >> https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L972-L986 >> >> As currently, ptm_cap_base is fetched using dw_pcie_find_ext_capability() >> instead of your new dw_pcie_ep_find_ext_capability() which takes a func_no. >> > >I examined the register spaces across different PFs to check whether all the PFs have the PTM capability registers, and confirmed that PTM capability registers exist only in PF0. PCIe r6.0 section 7.9.15 'Precision Time Management Extended Capability (PTM Capability)' states that " For Endpoints and Switch Upstream Ports that support PTM, this Capability is required in exactly one Function of the Upstream Port and that Capability controls the PTM behavior of all PTM capable Functions associated with that Upstream Port". This indicates that PTM capabilities are controller-level registers rather than per-function registers. Hence, in my opinion, ptm_cap_base does not require modification, since dw_pcie_find_ext_capability() and dw_pcie_*_dbi() already correctly access PF0's register space, which is the expected behavior for controller-level PTM management. Hello Aksh, Thanks a lot for digging in to this. Since commit: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dwc&id=86291f774fe8524178446cb2c792939640b4970c Together with your patch, there will only be a single call site in pcie-designware-ep.c that uses dw_pcie_*_dbi() instead of dw_pcie_ep_*_dbi() remaining. Thus, I think we should at least add a comment explain why this is the only place in the whole file that can ignore func_no. Kind regards, Niklas ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations 2026-01-22 5:56 ` Niklas Cassel @ 2026-01-22 6:20 ` Aksh Garg 0 siblings, 0 replies; 14+ messages in thread From: Aksh Garg @ 2026-01-22 6:20 UTC (permalink / raw) To: Niklas Cassel Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar >>>> -- >>>> 2.34.1 >>>> >>> >>> >>> Thank you for fixing this! >>> >>> Reviewed-by: Niklas Cassel <cassel@kernel.org> >>> >>> >>> You do need another patch in this series though, that fixes: >>> https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L972-L986 >>> >>> As currently, ptm_cap_base is fetched using dw_pcie_find_ext_capability() >>> instead of your new dw_pcie_ep_find_ext_capability() which takes a func_no. >>> >> >>I examined the register spaces across different PFs to check whether all the PFs have the PTM capability registers, and confirmed that PTM capability registers exist only in PF0. PCIe r6.0 section 7.9.15 'Precision Time Management Extended Capability (PTM Capability)' states that " For Endpoints and Switch Upstream Ports that support PTM, this Capability is required in exactly one Function of the Upstream Port and that Capability controls the PTM behavior of all PTM capable Functions associated with that Upstream Port". This indicates that PTM capabilities are controller-level registers rather than per-function registers. Hence, in my opinion, ptm_cap_base does not require modification, since dw_pcie_find_ext_capability() and dw_pcie_*_dbi() already correctly access PF0's register space, which is the expected behavior for controller-level PTM management. > > > Hello Aksh, > > Thanks a lot for digging in to this. > > Since commit: > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dwc&id=86291f774fe8524178446cb2c792939640b4970c > > Together with your patch, > there will only be a single call site in pcie-designware-ep.c that uses dw_pcie_*_dbi() instead of dw_pcie_ep_*_dbi() remaining. > > Thus, I think we should at least add a comment explain why this is the only place in the whole file that can ignore func_no. Yes, I agree that a comment should be added with the above explanation. I will add a patch for this in the series. Regards, Aksh Garg > > > Kind regards, > Niklas > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] PCI: dwc: ep: Add per-PF BAR and iATU mapping support 2026-01-21 5:42 [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support Aksh Garg 2026-01-21 5:42 ` [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations Aksh Garg @ 2026-01-21 5:42 ` Aksh Garg 2026-01-21 9:20 ` Niklas Cassel 2026-01-21 8:50 ` [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support Niklas Cassel 2 siblings, 1 reply; 14+ messages in thread From: Aksh Garg @ 2026-01-21 5:42 UTC (permalink / raw) To: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, cassel Cc: linux-kernel, s-vadapalli, danishanwar, Aksh Garg The commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP way of finding") adds support for each physical function to have its own MSI and MSI-X capability structures by introducing struct dw_pcie_ep_func. However, BAR configuration and iATU mappings are still being managed globally in struct dw_pcie_ep, meaning all PFs shared the same BAR-to-iATU mapping table. This creates conflicts when multiple physical functions attempts to configure BARs independently, as they would overwrite each other's iATU mappings and BAR configurations. Move bar_to_atu and epf_bar from struct dw_pcie_ep to struct dw_pcie_ep_func to allow proper multi-function endpoint operation, where each function can configure its BARs without interfering with other functions. Signed-off-by: Aksh Garg <a-garg7@ti.com> --- .../pci/controller/dwc/pcie-designware-ep.c | 50 +++++++++++++------ drivers/pci/controller/dwc/pcie-designware.h | 4 +- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index f222677a7a87..769602b58bd7 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -153,11 +153,16 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type, int ret; u32 free_win; struct dw_pcie *pci = to_dw_pcie_from_ep(ep); + struct dw_pcie_ep_func *ep_func; - if (!ep->bar_to_atu[bar]) + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); + if (!ep_func) + return -EINVAL; + + if (!ep_func->bar_to_atu[bar]) free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows); else - free_win = ep->bar_to_atu[bar] - 1; + free_win = ep_func->bar_to_atu[bar] - 1; if (free_win >= pci->num_ib_windows) { dev_err(pci->dev, "No free inbound window\n"); @@ -175,7 +180,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type, * Always increment free_win before assignment, since value 0 is used to identify * unallocated mapping. */ - ep->bar_to_atu[bar] = free_win + 1; + ep_func->bar_to_atu[bar] = free_win + 1; set_bit(free_win, ep->ib_window_map); return 0; @@ -211,17 +216,22 @@ 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; + struct dw_pcie_ep_func *ep_func; + u32 atu_index; + + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); - if (!ep->bar_to_atu[bar]) + if (!ep_func || !ep_func->bar_to_atu[bar]) return; + atu_index = ep_func->bar_to_atu[bar] - 1; + __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); - ep->epf_bar[bar] = NULL; - ep->bar_to_atu[bar] = 0; + ep_func->epf_bar[bar] = NULL; + ep_func->bar_to_atu[bar] = 0; } static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no, @@ -349,11 +359,16 @@ 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; + struct dw_pcie_ep_func *ep_func; size_t size = epf_bar->size; enum pci_epc_bar_type bar_type; int flags = epf_bar->flags; int ret, type; + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); + if (!ep_func) + return -EINVAL; + /* * DWC does not allow BAR pairs to overlap, e.g. you cannot combine BARs * 1 and 2 to form a 64-bit BAR. @@ -367,14 +382,14 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, * calling clear_bar() would clear the BAR's PCI address assigned by the * host). */ - if (ep->epf_bar[bar]) { + if (ep_func->epf_bar[bar]) { /* * 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) + if (ep_func->epf_bar[bar]->barno != bar || + ep_func->epf_bar[bar]->size != size || + ep_func->epf_bar[bar]->flags != flags) return -EINVAL; /* @@ -420,7 +435,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, if (ret) return ret; - ep->epf_bar[bar] = epf_bar; + ep_func->epf_bar[bar] = epf_bar; return 0; } @@ -782,7 +797,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset); tbl_offset &= PCI_MSIX_TABLE_OFFSET; - msix_tbl = ep->epf_bar[bir]->addr + tbl_offset; + msix_tbl = ep_func->epf_bar[bir]->addr + tbl_offset; msg_addr = msix_tbl[(interrupt_num - 1)].msg_addr; msg_data = msix_tbl[(interrupt_num - 1)].msg_data; vec_ctrl = msix_tbl[(interrupt_num - 1)].vector_ctrl; @@ -845,11 +860,16 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit); static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 func_no) { + struct dw_pcie_ep_func *ep_func; unsigned int offset; unsigned int nbars; enum pci_barno bar; u32 reg, i, val; + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); + if (!ep_func) + return; + offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR); if (offset) { @@ -876,8 +896,8 @@ static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 fun */ val = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); bar = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, val); - if (ep->epf_bar[bar]) - pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val); + if (ep_func->epf_bar[bar]) + pci_epc_bar_size_to_rebar_cap(ep_func->epf_bar[bar]->size, &val); else val = BIT(4); diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 31685951a080..a4d1733f5c6a 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -463,6 +463,8 @@ struct dw_pcie_ep_func { u8 func_no; u8 msi_cap; /* MSI capability offset */ u8 msix_cap; /* MSI-X capability offset */ + u8 bar_to_atu[PCI_STD_NUM_BARS]; + struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS]; }; struct dw_pcie_ep { @@ -472,13 +474,11 @@ struct dw_pcie_ep { phys_addr_t phys_base; size_t addr_size; size_t page_size; - u8 bar_to_atu[PCI_STD_NUM_BARS]; phys_addr_t *outbound_addr; unsigned long *ib_window_map; unsigned long *ob_window_map; void __iomem *msi_mem; phys_addr_t msi_mem_phys; - struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS]; }; struct dw_pcie_ops { -- 2.34.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] PCI: dwc: ep: Add per-PF BAR and iATU mapping support 2026-01-21 5:42 ` [PATCH 2/2] PCI: dwc: ep: Add per-PF BAR and iATU mapping support Aksh Garg @ 2026-01-21 9:20 ` Niklas Cassel 2026-01-21 11:35 ` [EXTERNAL] " Aksh Garg 0 siblings, 1 reply; 14+ messages in thread From: Niklas Cassel @ 2026-01-21 9:20 UTC (permalink / raw) To: Aksh Garg Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar On Wed, Jan 21, 2026 at 11:12:14AM +0530, Aksh Garg wrote: > The commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP > way of finding") adds support for each physical function to have its > own MSI and MSI-X capability structures by introducing struct > dw_pcie_ep_func. However, BAR configuration and iATU mappings are still > being managed globally in struct dw_pcie_ep, meaning all PFs shared the > same BAR-to-iATU mapping table. You are mentioning commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP way of finding"), but you should probably also mention commit 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC") which added support for PFs in the DWC driver. That is the commit that is incorrect IMO. You cannot add support for PFs and then have a different EPFs over overwriting address translation for other EPFs. The design was simply broken from the start. > > This creates conflicts when multiple physical functions attempts to > configure BARs independently, as they would overwrite each other's > iATU mappings and BAR configurations. > > Move bar_to_atu and epf_bar from struct dw_pcie_ep to struct > dw_pcie_ep_func to allow proper multi-function endpoint operation, > where each function can configure its BARs without interfering with > other functions. > > Signed-off-by: Aksh Garg <a-garg7@ti.com> Fixes: 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC") > --- > .../pci/controller/dwc/pcie-designware-ep.c | 50 +++++++++++++------ > drivers/pci/controller/dwc/pcie-designware.h | 4 +- > 2 files changed, 37 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index f222677a7a87..769602b58bd7 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -153,11 +153,16 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type, > int ret; > u32 free_win; > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + struct dw_pcie_ep_func *ep_func; > > - if (!ep->bar_to_atu[bar]) > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); There is no reason why this can't be initialized on the same line as your are declaring the ep_func variable, just like we do for other variables, e.g. struct dw_pcie *pci = to_dw_pcie_from_ep(ep); so please more the initialization to same line as the declaration. > + if (!ep_func) > + return -EINVAL; > + > + if (!ep_func->bar_to_atu[bar]) > free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows); > else > - free_win = ep->bar_to_atu[bar] - 1; > + free_win = ep_func->bar_to_atu[bar] - 1; > > if (free_win >= pci->num_ib_windows) { > dev_err(pci->dev, "No free inbound window\n"); > @@ -175,7 +180,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type, > * Always increment free_win before assignment, since value 0 is used to identify > * unallocated mapping. > */ > - ep->bar_to_atu[bar] = free_win + 1; > + ep_func->bar_to_atu[bar] = free_win + 1; > set_bit(free_win, ep->ib_window_map); > > return 0; > @@ -211,17 +216,22 @@ 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; > + struct dw_pcie_ep_func *ep_func; > + u32 atu_index; > + > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); Same comment as above. > > - if (!ep->bar_to_atu[bar]) > + if (!ep_func || !ep_func->bar_to_atu[bar]) > return; > > + atu_index = ep_func->bar_to_atu[bar] - 1; > + > __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); > - ep->epf_bar[bar] = NULL; > - ep->bar_to_atu[bar] = 0; > + ep_func->epf_bar[bar] = NULL; > + ep_func->bar_to_atu[bar] = 0; > } > > static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no, > @@ -349,11 +359,16 @@ 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; > + struct dw_pcie_ep_func *ep_func; > size_t size = epf_bar->size; > enum pci_epc_bar_type bar_type; > int flags = epf_bar->flags; > int ret, type; > > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); Same comment as above. > + if (!ep_func) > + return -EINVAL; > + > /* > * DWC does not allow BAR pairs to overlap, e.g. you cannot combine BARs > * 1 and 2 to form a 64-bit BAR. > @@ -367,14 +382,14 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > * calling clear_bar() would clear the BAR's PCI address assigned by the > * host). > */ > - if (ep->epf_bar[bar]) { > + if (ep_func->epf_bar[bar]) { > /* > * 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) > + if (ep_func->epf_bar[bar]->barno != bar || > + ep_func->epf_bar[bar]->size != size || > + ep_func->epf_bar[bar]->flags != flags) > return -EINVAL; > > /* > @@ -420,7 +435,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, > if (ret) > return ret; > > - ep->epf_bar[bar] = epf_bar; > + ep_func->epf_bar[bar] = epf_bar; > > return 0; > } > @@ -782,7 +797,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset); > tbl_offset &= PCI_MSIX_TABLE_OFFSET; > > - msix_tbl = ep->epf_bar[bir]->addr + tbl_offset; > + msix_tbl = ep_func->epf_bar[bir]->addr + tbl_offset; > msg_addr = msix_tbl[(interrupt_num - 1)].msg_addr; > msg_data = msix_tbl[(interrupt_num - 1)].msg_data; > vec_ctrl = msix_tbl[(interrupt_num - 1)].vector_ctrl; > @@ -845,11 +860,16 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit); > > static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 func_no) > { > + struct dw_pcie_ep_func *ep_func; > unsigned int offset; > unsigned int nbars; > enum pci_barno bar; > u32 reg, i, val; > > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); Same comment as above. > + if (!ep_func) > + return; > + > offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR); > > if (offset) { > @@ -876,8 +896,8 @@ static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 fun > */ > val = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); > bar = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, val); > - if (ep->epf_bar[bar]) > - pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val); > + if (ep_func->epf_bar[bar]) > + pci_epc_bar_size_to_rebar_cap(ep_func->epf_bar[bar]->size, &val); > else > val = BIT(4); > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 31685951a080..a4d1733f5c6a 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -463,6 +463,8 @@ struct dw_pcie_ep_func { > u8 func_no; > u8 msi_cap; /* MSI capability offset */ > u8 msix_cap; /* MSI-X capability offset */ > + u8 bar_to_atu[PCI_STD_NUM_BARS]; > + struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS]; > }; > > struct dw_pcie_ep { > @@ -472,13 +474,11 @@ struct dw_pcie_ep { > phys_addr_t phys_base; > size_t addr_size; > size_t page_size; > - u8 bar_to_atu[PCI_STD_NUM_BARS]; > phys_addr_t *outbound_addr; > unsigned long *ib_window_map; > unsigned long *ob_window_map; > void __iomem *msi_mem; > phys_addr_t msi_mem_phys; > - struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS]; > }; > > struct dw_pcie_ops { > -- > 2.34.1 > With minor nits fixed: Reviewed-by: Niklas Cassel <cassel@kernel.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXTERNAL] Re: [PATCH 2/2] PCI: dwc: ep: Add per-PF BAR and iATU mapping support 2026-01-21 9:20 ` Niklas Cassel @ 2026-01-21 11:35 ` Aksh Garg 2026-01-21 12:56 ` Niklas Cassel 0 siblings, 1 reply; 14+ messages in thread From: Aksh Garg @ 2026-01-21 11:35 UTC (permalink / raw) To: Niklas Cassel Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar > On Wed, Jan 21, 2026 at 11:12:14AM +0530, Aksh Garg wrote: >> The commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP >> way of finding") adds support for each physical function to have its >> own MSI and MSI-X capability structures by introducing struct >> dw_pcie_ep_func. However, BAR configuration and iATU mappings are still >> being managed globally in struct dw_pcie_ep, meaning all PFs shared the >> same BAR-to-iATU mapping table. > > You are mentioning commit 47a062609a30 ("PCI: designware-ep: Modify MSI and > MSIX CAP way of finding"), but you should probably also mention commit > 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC") > which added support for PFs in the DWC driver. > > That is the commit that is incorrect IMO. You cannot add support for PFs > and then have a different EPFs over overwriting address translation for > other EPFs. The design was simply broken from the start. > Yes, the commit 24ede430fa49 is indeed the culprit for this issue. Maybe the way I wrote this commit message confused you. I was just referring to the commit 47a062609a30 saying that as this commit added the feature for each PF to have its own MSI and MSI-X capability, I wanted to move bar_to_atu and epf_bar per-PF as well, such that each PF should have its own epf_bar[] and bar_to_atu[]. The index passed to these fields should be unique across PF+BAR combinations while the current implementation is only keeping it unique across BARs but not PFs. This results in overwriting address translation regions across the PFs. I will rephrase the commit message and add fixes tag for 24ede430fa49. Please provide your input whether the above explaination is sufficient. Should I refer to commit 47a062609a30 as an example in the commit message, or that would not be required given the above explaination is added? > >> >> This creates conflicts when multiple physical functions attempts to >> configure BARs independently, as they would overwrite each other's >> iATU mappings and BAR configurations. >> >> Move bar_to_atu and epf_bar from struct dw_pcie_ep to struct >> dw_pcie_ep_func to allow proper multi-function endpoint operation, >> where each function can configure its BARs without interfering with >> other functions. >> >> Signed-off-by: Aksh Garg <a-garg7@ti.com> > > Fixes: 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC") > > >> --- >> .../pci/controller/dwc/pcie-designware-ep.c | 50 +++++++++++++------ >> drivers/pci/controller/dwc/pcie-designware.h | 4 +- >> 2 files changed, 37 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c >> index f222677a7a87..769602b58bd7 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c >> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c >> @@ -153,11 +153,16 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type, >> int ret; >> u32 free_win; >> struct dw_pcie *pci = to_dw_pcie_from_ep(ep); >> + struct dw_pcie_ep_func *ep_func; >> >> - if (!ep->bar_to_atu[bar]) >> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > > There is no reason why this can't be initialized on the same line as your are > declaring the ep_func variable, just like we do for other variables, e.g. > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > so please more the initialization to same line as the declaration. > > >> + if (!ep_func) >> + return -EINVAL; >> + >> + if (!ep_func->bar_to_atu[bar]) >> free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows); >> else >> - free_win = ep->bar_to_atu[bar] - 1; >> + free_win = ep_func->bar_to_atu[bar] - 1; >> >> if (free_win >= pci->num_ib_windows) { >> dev_err(pci->dev, "No free inbound window\n"); >> @@ -175,7 +180,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type, >> * Always increment free_win before assignment, since value 0 is used to identify >> * unallocated mapping. >> */ >> - ep->bar_to_atu[bar] = free_win + 1; >> + ep_func->bar_to_atu[bar] = free_win + 1; >> set_bit(free_win, ep->ib_window_map); >> >> return 0; >> @@ -211,17 +216,22 @@ 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; >> + struct dw_pcie_ep_func *ep_func; >> + u32 atu_index; >> + >> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > > Same comment as above. > >> >> - if (!ep->bar_to_atu[bar]) >> + if (!ep_func || !ep_func->bar_to_atu[bar]) >> return; >> >> + atu_index = ep_func->bar_to_atu[bar] - 1; >> + >> __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); >> - ep->epf_bar[bar] = NULL; >> - ep->bar_to_atu[bar] = 0; >> + ep_func->epf_bar[bar] = NULL; >> + ep_func->bar_to_atu[bar] = 0; >> } >> >> static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no, >> @@ -349,11 +359,16 @@ 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; >> + struct dw_pcie_ep_func *ep_func; >> size_t size = epf_bar->size; >> enum pci_epc_bar_type bar_type; >> int flags = epf_bar->flags; >> int ret, type; >> >> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > > Same comment as above. > > >> + if (!ep_func) >> + return -EINVAL; >> + >> /* >> * DWC does not allow BAR pairs to overlap, e.g. you cannot combine BARs >> * 1 and 2 to form a 64-bit BAR. >> @@ -367,14 +382,14 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, >> * calling clear_bar() would clear the BAR's PCI address assigned by the >> * host). >> */ >> - if (ep->epf_bar[bar]) { >> + if (ep_func->epf_bar[bar]) { >> /* >> * 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) >> + if (ep_func->epf_bar[bar]->barno != bar || >> + ep_func->epf_bar[bar]->size != size || >> + ep_func->epf_bar[bar]->flags != flags) >> return -EINVAL; >> >> /* >> @@ -420,7 +435,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, >> if (ret) >> return ret; >> >> - ep->epf_bar[bar] = epf_bar; >> + ep_func->epf_bar[bar] = epf_bar; >> >> return 0; >> } >> @@ -782,7 +797,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, >> bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset); >> tbl_offset &= PCI_MSIX_TABLE_OFFSET; >> >> - msix_tbl = ep->epf_bar[bir]->addr + tbl_offset; >> + msix_tbl = ep_func->epf_bar[bir]->addr + tbl_offset; >> msg_addr = msix_tbl[(interrupt_num - 1)].msg_addr; >> msg_data = msix_tbl[(interrupt_num - 1)].msg_data; >> vec_ctrl = msix_tbl[(interrupt_num - 1)].vector_ctrl; >> @@ -845,11 +860,16 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit); >> >> static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 func_no) >> { >> + struct dw_pcie_ep_func *ep_func; >> unsigned int offset; >> unsigned int nbars; >> enum pci_barno bar; >> u32 reg, i, val; >> >> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > > Same comment as above. > > >> + if (!ep_func) >> + return; >> + >> offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR); >> >> if (offset) { >> @@ -876,8 +896,8 @@ static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 fun >> */ >> val = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL); >> bar = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, val); >> - if (ep->epf_bar[bar]) >> - pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val); >> + if (ep_func->epf_bar[bar]) >> + pci_epc_bar_size_to_rebar_cap(ep_func->epf_bar[bar]->size, &val); >> else >> val = BIT(4); >> >> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h >> index 31685951a080..a4d1733f5c6a 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware.h >> +++ b/drivers/pci/controller/dwc/pcie-designware.h >> @@ -463,6 +463,8 @@ struct dw_pcie_ep_func { >> u8 func_no; >> u8 msi_cap; /* MSI capability offset */ >> u8 msix_cap; /* MSI-X capability offset */ >> + u8 bar_to_atu[PCI_STD_NUM_BARS]; >> + struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS]; >> }; >> >> struct dw_pcie_ep { >> @@ -472,13 +474,11 @@ struct dw_pcie_ep { >> phys_addr_t phys_base; >> size_t addr_size; >> size_t page_size; >> - u8 bar_to_atu[PCI_STD_NUM_BARS]; >> phys_addr_t *outbound_addr; >> unsigned long *ib_window_map; >> unsigned long *ob_window_map; >> void __iomem *msi_mem; >> phys_addr_t msi_mem_phys; >> - struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS]; >> }; >> >> struct dw_pcie_ops { >> -- >> 2.34.1 >> > > With minor nits fixed: > Reviewed-by: Niklas Cassel <cassel@kernel.org> > Thank you for reviewing the patch. I will fix these minor nits. Regards, Aksh Garg ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXTERNAL] Re: [PATCH 2/2] PCI: dwc: ep: Add per-PF BAR and iATU mapping support 2026-01-21 11:35 ` [EXTERNAL] " Aksh Garg @ 2026-01-21 12:56 ` Niklas Cassel 0 siblings, 0 replies; 14+ messages in thread From: Niklas Cassel @ 2026-01-21 12:56 UTC (permalink / raw) To: Aksh Garg Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar On Wed, Jan 21, 2026 at 05:05:50PM +0530, Aksh Garg wrote: > > > > On Wed, Jan 21, 2026 at 11:12:14AM +0530, Aksh Garg wrote: > > > The commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP > > > way of finding") adds support for each physical function to have its > > > own MSI and MSI-X capability structures by introducing struct > > > dw_pcie_ep_func. However, BAR configuration and iATU mappings are still > > > being managed globally in struct dw_pcie_ep, meaning all PFs shared the > > > same BAR-to-iATU mapping table. > > > > You are mentioning commit 47a062609a30 ("PCI: designware-ep: Modify MSI and > > MSIX CAP way of finding"), but you should probably also mention commit > > 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC") > > which added support for PFs in the DWC driver. > > > > That is the commit that is incorrect IMO. You cannot add support for PFs > > and then have a different EPFs over overwriting address translation for > > other EPFs. The design was simply broken from the start. > > > > Yes, the commit 24ede430fa49 is indeed the culprit for this issue. Maybe the > way I wrote this commit message confused you. I was just referring to the > commit 47a062609a30 saying that as this commit added the feature for each PF > to have its own MSI and MSI-X capability, I wanted to move bar_to_atu and > epf_bar per-PF as well, such that each PF should have its own epf_bar[] and > bar_to_atu[]. The index passed to these fields should be unique across > PF+BAR combinations while the current implementation is only keeping it > unique across BARs but not PFs. This results in overwriting address > translation regions across the PFs. > I will rephrase the commit message and add fixes tag for 24ede430fa49. > > Please provide your input whether the above explaination is sufficient. > Should I refer to commit 47a062609a30 as an example in the commit message, > or that would not be required given the above explaination is added? As long as you mention both commits, I think it is sufficient. Perhaps something like: Commit 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC") added support for multiple PFs in the DWC driver. However, this commit was incomplete, and did not properly support MSI/MSI-X for multiple PFs, which was fixed in commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP way of finding"). Even with both these commits, the multiple PF support in the DWC driver is severely broken, because one EPF for one PF will currently overwrite the address translation done by another EPF for a completely different PF. To fix this, just as commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP way of finding") did when it moved things to a per PF struct dw_pcie_ep_func, moving the data structures needed for address translation to struct dw_pcie_ep_func. Kind regards, Niklas ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support 2026-01-21 5:42 [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support Aksh Garg 2026-01-21 5:42 ` [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations Aksh Garg 2026-01-21 5:42 ` [PATCH 2/2] PCI: dwc: ep: Add per-PF BAR and iATU mapping support Aksh Garg @ 2026-01-21 8:50 ` Niklas Cassel 2026-01-21 9:27 ` Niklas Cassel 2 siblings, 1 reply; 14+ messages in thread From: Niklas Cassel @ 2026-01-21 8:50 UTC (permalink / raw) To: Aksh Garg Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar On Wed, Jan 21, 2026 at 11:12:12AM +0530, Aksh Garg wrote: > This series addresses multi-function endpoint configuration issues in > the DWC PCIe controller driver. The changes enable proper operations > for physical functions and enhance the multi-function endpoint support. Considering that the DWC driver design has always been broken with regards to multiple physical functions, as you explain yourself, the iATU to BAR mapping has always been per controller, not per PF. Thus, it would be nice if you explained how you have actually tested this. Does the controller you are using have both Resizable and Programmable BARs? Kind regards, Niklas ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support 2026-01-21 8:50 ` [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support Niklas Cassel @ 2026-01-21 9:27 ` Niklas Cassel 2026-01-21 11:04 ` [EXTERNAL] " Aksh Garg 0 siblings, 1 reply; 14+ messages in thread From: Niklas Cassel @ 2026-01-21 9:27 UTC (permalink / raw) To: Aksh Garg Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar On Wed, Jan 21, 2026 at 09:50:58AM +0100, Niklas Cassel wrote: > On Wed, Jan 21, 2026 at 11:12:12AM +0530, Aksh Garg wrote: > > This series addresses multi-function endpoint configuration issues in > > the DWC PCIe controller driver. The changes enable proper operations > > for physical functions and enhance the multi-function endpoint support. > > > Considering that the DWC driver design has always been broken with regards > to multiple physical functions, as you explain yourself, the iATU to BAR > mapping has always been per controller, not per PF. > > Thus, it would be nice if you explained how you have actually tested this. > > Does the controller you are using have both Resizable and Programmable BARs? If your PCIe controller supports multiple PFs, as you might know, while: const struct pci_epc_features* (*get_features)(struct pci_epc *epc, u8 func_no, u8 vfunc_no); takes both a PF and a VF... dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no, u8 vfunc_no) ignores both of these parameters and simply call: ep->ops->get_features(ep); For each PF in your controller, do all BARs have the exact same definition? e.g. all BARs for each PF would need to have the same type, e.g. if BAR0 is FIXED, BAR1 is programmable, and BAR2 is resizable, with the current design of dw_pcie_ep_get_features() that would need to be the case for all PFs. Kind regards, Niklas ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [EXTERNAL] Re: [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support 2026-01-21 9:27 ` Niklas Cassel @ 2026-01-21 11:04 ` Aksh Garg 0 siblings, 0 replies; 14+ messages in thread From: Aksh Garg @ 2026-01-21 11:04 UTC (permalink / raw) To: Niklas Cassel Cc: linux-pci, jingoohan1, mani, lpieralisi, kwilczynski, robh, bhelgaas, linux-kernel, s-vadapalli, danishanwar Hi Niklas, Thanks a lot for your comments and reviews! > On Wed, Jan 21, 2026 at 09:50:58AM +0100, Niklas Cassel wrote: >> On Wed, Jan 21, 2026 at 11:12:12AM +0530, Aksh Garg wrote: >> > This series addresses multi-function endpoint configuration issues in >> > the DWC PCIe controller driver. The changes enable proper operations >> > for physical functions and enhance the multi-function endpoint support. >> >> >> Considering that the DWC driver design has always been broken with regards >> to multiple physical functions, as you explain yourself, the iATU to BAR >> mapping has always been per controller, not per PF. >> >> Thus, it would be nice if you explained how you have actually tested this. >> >> Does the controller you are using have both Resizable and Programmable BARs? > > If your PCIe controller supports multiple PFs, as you might know, > while: > > const struct pci_epc_features* (*get_features)(struct pci_epc *epc, > u8 func_no, u8 vfunc_no); > > takes both a PF and a VF... > > dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no, u8 vfunc_no) > ignores both of these parameters and simply call: > ep->ops->get_features(ep); > > > For each PF in your controller, do all BARs have the exact same definition? All the BARs of all the PFs in my controller are resizable, hence can have the same features structure. However, this might need a fix as well if a controller comes out to have different BAR definitions for different PFs! Regards, Aksh Garg > > e.g. all BARs for each PF would need to have the same type, e.g. if BAR0 > is FIXED, BAR1 is programmable, and BAR2 is resizable, with the current > design of dw_pcie_ep_get_features() that would need to be the case for all > PFs. > > Kind regards, > Niklas > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-01-22 6:20 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-21 5:42 [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support Aksh Garg 2026-01-21 5:42 ` [PATCH 1/2] PCI: dwc: ep: Fix resizable BAR support for multi-PF configurations Aksh Garg 2026-01-21 9:58 ` Niklas Cassel 2026-01-21 11:40 ` Aksh Garg 2026-01-22 5:05 ` Aksh Garg 2026-01-22 5:56 ` Niklas Cassel 2026-01-22 6:20 ` Aksh Garg 2026-01-21 5:42 ` [PATCH 2/2] PCI: dwc: ep: Add per-PF BAR and iATU mapping support Aksh Garg 2026-01-21 9:20 ` Niklas Cassel 2026-01-21 11:35 ` [EXTERNAL] " Aksh Garg 2026-01-21 12:56 ` Niklas Cassel 2026-01-21 8:50 ` [PATCH 0/2] PCI: dwc: ep: Enhance multi-function endpoint support Niklas Cassel 2026-01-21 9:27 ` Niklas Cassel 2026-01-21 11:04 ` [EXTERNAL] " Aksh Garg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox