Message-ID: <aXzunVBXeJUrIe/K@lizhi-Precision-Tower-5810>
Date: Fri, 30 Jan 2026 12:47:09 -0500
From: Frank Li <Frank.li@....com>
To: Aksh Garg <a-garg7@...com>
Cc: linux-pci@...r.kernel.org, jingoohan1@...il.com, mani@...nel.org,
	lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org,
	bhelgaas@...gle.com, cassel@...nel.org, Zhiqiang.Hou@....com,
	gustavo.pimentel@...opsys.com, den@...inux.co.jp,
	linux-kernel@...r.kernel.org, s-vadapalli@...com,
	danishanwar@...com
Subject: Re: [PATCH v5 2/3] PCI: dwc: ep: Add per-PF BAR and inbound ATU
 mapping support

On Fri, Jan 30, 2026 at 05:25:15PM +0530, Aksh Garg wrote:
> The commit 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support
> for DWC") added support for multiple PFs in the DWC driver, but the
> implementation was incomplete. It did not properly support MSI/MSI-X,
> as well as BAR and inbound ATU mapping for multiple PFs. The MSI/MSI-X
> issue was later fixed by commit 47a062609a30 ("PCI: designware-ep:
> Modify MSI and MSIX CAP way of finding") by introducing a per-PF
> struct dw_pcie_ep_func.
>
> However, even with both commits, the multiple PF support in the driver
> remains broken because BAR configuration and ATU mappings are managed
> globally in struct dw_pcie_ep, meaning all PFs share the same BAR-to-ATU
> mapping table. This causes one PF's EPF to overwrite the address
> translation of another PF's EPF in the internal ATU region,
> creating conflicts when multiple physical functions attempt to
> configure their BARs independently.
>
> The commit cfbc98dbf44d ("PCI: dwc: ep: Support BAR subrange inbound
> mapping via Address Match Mode iATU") later introduced Address Match
> Mode support, which suffers from the same multi-PF conflict issue.
>
> Fix this by moving the required members from struct dw_pcie_ep to
> struct dw_pcie_ep_func, similar to what commit 47a062609a30
> ("PCI: designware-ep: Modify MSI and MSIX CAP way of finding") did for
> MSI/MSI-X capability support, to allow proper multi-function endpoint
> operation, where each PF can configure its BARs and corresponding
> internal ATU region without interfering with other PFs.
>
> Fixes: 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC")
> Fixes: cfbc98dbf44d ("PCI: dwc: ep: Support BAR subrange inbound mapping via Address Match Mode iATU")
> Signed-off-by: Aksh Garg <a-garg7@...com>
> Reviewed-by: Niklas Cassel <cassel@...nel.org>
> ---

not related with patch, Does your hardware have ITS?

Reviewed-by: Frank Li <Frank.Li@....com>

>
> Changes from v4 to v5:
> - Added NULL checker for ep_func in dw_pcie_ep_clear_ib_maps()
>
> Changes from v3 to v4:
> - Fix the similar conflict created by the commit cfbc98dbf44d
>
> Changes from v2 to v3:
> - None
>
> Changes from v1 to v2:
> - Fixed the suggested nits
> - Rephrased the commit message with a proper Fixes tag
>
> v4: https://lore.kernel.org/all/20260129091753.490167-3-a-garg7@ti.com/
> v3: https://lore.kernel.org/all/20260127085010.446116-3-a-garg7@ti.com/
> v2: https://lore.kernel.org/all/20260122082538.309122-3-a-garg7@ti.com/
> v1: https://lore.kernel.org/all/20260121054214.274429-3-a-garg7@ti.com/
>
>  .../pci/controller/dwc/pcie-designware-ep.c   | 79 +++++++++++--------
>  drivers/pci/controller/dwc/pcie-designware.h  | 12 +--
>  2 files changed, 54 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 1cc2985bab03..6d3c35dd280f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -115,11 +115,15 @@ static int dw_pcie_ep_ib_atu_bar(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 = dw_pcie_ep_get_func_from_ep(ep, func_no);
>
> -	if (!ep->bar_to_atu[bar])
> +	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");
> @@ -137,33 +141,37 @@ static int dw_pcie_ep_ib_atu_bar(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;
>  }
>
> -static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, enum pci_barno bar)
> +static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, u8 func_no, enum pci_barno bar)
>  {
> +	struct dw_pcie_ep_func *ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	struct device *dev = pci->dev;
>  	unsigned int i, num;
>  	u32 atu_index;
>  	u32 *indexes;
>
> +	if (!ep_func)
> +		return;
> +
>  	/* Tear down the BAR Match Mode mapping, if any. */
> -	if (ep->bar_to_atu[bar]) {
> -		atu_index = ep->bar_to_atu[bar] - 1;
> +	if (ep_func->bar_to_atu[bar]) {
> +		atu_index = ep_func->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;
> +		ep_func->bar_to_atu[bar] = 0;
>  	}
>
>  	/* Tear down all Address Match Mode mappings, if any. */
> -	indexes = ep->ib_atu_indexes[bar];
> -	num = ep->num_ib_atu_indexes[bar];
> -	ep->ib_atu_indexes[bar] = NULL;
> -	ep->num_ib_atu_indexes[bar] = 0;
> +	indexes = ep_func->ib_atu_indexes[bar];
> +	num = ep_func->num_ib_atu_indexes[bar];
> +	ep_func->ib_atu_indexes[bar] = NULL;
> +	ep_func->num_ib_atu_indexes[bar] = 0;
>  	if (!indexes)
>  		return;
>  	for (i = 0; i < num; i++) {
> @@ -248,6 +256,7 @@ static int dw_pcie_ep_validate_submap(struct dw_pcie_ep *ep,
>  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)
>  {
> +	struct dw_pcie_ep_func *ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
>  	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;
> @@ -258,7 +267,7 @@ static int dw_pcie_ep_ib_atu_addr(struct dw_pcie_ep *ep, u8 func_no, int type,
>  	unsigned int i;
>  	u32 *indexes;
>
> -	if (!epf_bar->num_submap || !submap || !epf_bar->size)
> +	if (!ep_func || !epf_bar->num_submap || !submap || !epf_bar->size)
>  		return -EINVAL;
>
>  	ret = dw_pcie_ep_validate_submap(ep, submap, epf_bar->num_submap,
> @@ -279,8 +288,8 @@ static int dw_pcie_ep_ib_atu_addr(struct dw_pcie_ep *ep, u8 func_no, int type,
>  	if (!indexes)
>  		return -ENOMEM;
>
> -	ep->ib_atu_indexes[bar] = indexes;
> -	ep->num_ib_atu_indexes[bar] = 0;
> +	ep_func->ib_atu_indexes[bar] = indexes;
> +	ep_func->num_ib_atu_indexes[bar] = 0;
>
>  	for (i = 0; i < epf_bar->num_submap; i++) {
>  		size = submap[i].size;
> @@ -308,11 +317,11 @@ static int dw_pcie_ep_ib_atu_addr(struct dw_pcie_ep *ep, u8 func_no, int type,
>
>  		set_bit(free_win, ep->ib_window_map);
>  		indexes[i] = free_win;
> -		ep->num_ib_atu_indexes[bar] = i + 1;
> +		ep_func->num_ib_atu_indexes[bar] = i + 1;
>  	}
>  	return 0;
>  err:
> -	dw_pcie_ep_clear_ib_maps(ep, bar);
> +	dw_pcie_ep_clear_ib_maps(ep, func_no, bar);
>  	return ret;
>  }
>
> @@ -346,15 +355,16 @@ 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;
> +	struct dw_pcie_ep_func *ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
>
> -	if (!ep->epf_bar[bar])
> +	if (!ep_func || !ep_func->epf_bar[bar])
>  		return;
>
>  	__dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
>
> -	dw_pcie_ep_clear_ib_maps(ep, bar);
> +	dw_pcie_ep_clear_ib_maps(ep, func_no, bar);
>
> -	ep->epf_bar[bar] = NULL;
> +	ep_func->epf_bar[bar] = NULL;
>  }
>
>  static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no,
> @@ -481,12 +491,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);
> +	struct dw_pcie_ep_func *ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
>  	enum pci_barno bar = epf_bar->barno;
>  	size_t size = epf_bar->size;
>  	enum pci_epc_bar_type bar_type;
>  	int flags = epf_bar->flags;
>  	int ret, type;
>
> +	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.
> @@ -500,22 +514,22 @@ 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;
>
>  		/*
>  		 * When dynamically changing a BAR, tear down any existing
>  		 * mappings before re-programming.
>  		 */
> -		if (ep->epf_bar[bar]->num_submap || epf_bar->num_submap)
> -			dw_pcie_ep_clear_ib_maps(ep, bar);
> +		if (ep_func->epf_bar[bar]->num_submap || epf_bar->num_submap)
> +			dw_pcie_ep_clear_ib_maps(ep, func_no, bar);
>
>  		/*
>  		 * When dynamically changing a BAR, skip writing the BAR reg, as
> @@ -574,7 +588,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;
>  }
> @@ -969,7 +983,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;
> @@ -1032,11 +1046,14 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
>
>  static void dw_pcie_ep_init_rebar_registers(struct dw_pcie_ep *ep, u8 func_no)
>  {
> -	unsigned int offset;
> -	unsigned int nbars;
> +	struct dw_pcie_ep_func *ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> +	unsigned int offset, nbars;
>  	enum pci_barno bar;
>  	u32 reg, i, val;
>
> +	if (!ep_func)
> +		return;
> +
>  	offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR);
>
>  	if (offset) {
> @@ -1063,8 +1080,8 @@ static void dw_pcie_ep_init_rebar_registers(struct dw_pcie_ep *ep, u8 func_no)
>  			 */
>  			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 8f170122ad78..43d7606bc987 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -471,6 +471,12 @@ 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];
> +
> +	/* Only for Address Match Mode inbound iATU */
> +	u32			*ib_atu_indexes[PCI_STD_NUM_BARS];
> +	unsigned int		num_ib_atu_indexes[PCI_STD_NUM_BARS];
>  };
>
>  struct dw_pcie_ep {
> @@ -480,17 +486,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];
> -
> -	/* Only for Address Match Mode inbound iATU */
> -	u32			*ib_atu_indexes[PCI_STD_NUM_BARS];
> -	unsigned int		num_ib_atu_indexes[PCI_STD_NUM_BARS];
>
>  	/* MSI outbound iATU state */
>  	bool			msi_iatu_mapped;
> --
> 2.34.1
>

Powered by blists - more mailing lists