[PATCH v2 3/3] PCI: dwc: Fix missing iATU setup when ECAM is enabled

Krishna Chaitanya Chundru posted 3 patches 1 month, 1 week ago
[PATCH v2 3/3] PCI: dwc: Fix missing iATU setup when ECAM is enabled
Posted by Krishna Chaitanya Chundru 1 month, 1 week ago
When ECAM is enabled, the driver skipped calling dw_pcie_iatu_setup()
before configuring ECAM iATU entries. This left IO and MEM outbound
windows unprogrammed, resulting in broken IO transactions. Additionally,
dw_pcie_config_ecam_iatu() was only called during host initialization,
so ECAM-related iATU entries were not restored after suspend/resume,
leading to failures in configuration space access

To resolve these issues, the ECAM iATU configuration is moved into
dw_pcie_setup_rc(). At the same time, dw_pcie_iatu_setup() is invoked
when ECAM is enabled.

Rename msg_atu_index to ob_atu_index to track the next available outbound
iATU index for ECAM and MSG TLP windows. Furthermore, an error check is
added in dw_pcie_prog_outbound_atu() to avoid programming beyond
num_ob_windows.

Fixes: f6fd357f7afb ("PCI: dwc: Prepare the driver for enabling ECAM mechanism using iATU 'CFG Shift Feature'")
Reported-by: Maciej W. Rozycki <macro@orcam.me.uk>
Closes: https://lore.kernel.org/all/alpine.DEB.2.21.2511280256260.36486@angie.orcam.me.uk/
Tested-by: Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 41 ++++++++++++++---------
 drivers/pci/controller/dwc/pcie-designware.c      |  3 ++
 drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
 3 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 88b6ace0607e97bf6dd6bf7886baaa13bf267e6e..cb1b5b2a2fe61eb5901e57a60f8f333b1c3e766b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -430,10 +430,10 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
 	/*
 	 * Root bus under the host bridge doesn't require any iATU configuration
 	 * as DBI region will be used to access root bus config space.
-	 * Immediate bus under Root Bus, needs type 0 iATU configuration and
+	 * Immediate bus under Root Bus needs type 0 iATU configuration and
 	 * remaining buses need type 1 iATU configuration.
 	 */
-	atu.index = 0;
+	atu.index = pci->ob_atu_index;
 	atu.type = PCIE_ATU_TYPE_CFG0;
 	atu.parent_bus_addr = pp->cfg0_base + SZ_1M;
 	/* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
@@ -443,6 +443,8 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
 	if (ret)
 		return ret;
 
+	pci->ob_atu_index++;
+
 	bus_range_max = resource_size(bus->res);
 
 	if (bus_range_max < 2)
@@ -455,7 +457,13 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
 	atu.size = (SZ_1M * bus_range_max) - SZ_2M;
 	atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
 
-	return dw_pcie_prog_outbound_atu(pci, &atu);
+	ret = dw_pcie_prog_outbound_atu(pci, &atu);
+	if (ret)
+		return ret;
+
+	pci->ob_atu_index++;
+
+	return 0;
 }
 
 static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
@@ -630,14 +638,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
 	if (ret)
 		goto err_free_msi;
 
-	if (pp->ecam_enabled) {
-		ret = dw_pcie_config_ecam_iatu(pp);
-		if (ret) {
-			dev_err(dev, "Failed to configure iATU in ECAM mode\n");
-			goto err_free_msi;
-		}
-	}
-
 	/*
 	 * Allocate the resource for MSG TLP before programming the iATU
 	 * outbound window in dw_pcie_setup_rc(). Since the allocation depends
@@ -942,7 +942,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 		dev_warn(pci->dev, "Ranges exceed outbound iATU size (%d)\n",
 			 pci->num_ob_windows);
 
-	pp->msg_atu_index = ++i;
+	pci->ob_atu_index = ++i;
 
 	i = 0;
 	resource_list_for_each_entry(entry, &pp->bridge->dma_ranges) {
@@ -1084,14 +1084,23 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
 	/*
 	 * If the platform provides its own child bus config accesses, it means
 	 * the platform uses its own address translation component rather than
-	 * ATU, so we should not program the ATU here.
+	 * ATU, so we should not program the ATU here. If ECAM is enabled,
+	 * config space access goes through ATU, so set up ATU here.
 	 */
-	if (pp->bridge->child_ops == &dw_child_pcie_ops) {
+	if (pp->bridge->child_ops == &dw_child_pcie_ops || pp->ecam_enabled) {
 		ret = dw_pcie_iatu_setup(pp);
 		if (ret)
 			return ret;
 	}
 
+	if (pp->ecam_enabled) {
+		ret = dw_pcie_config_ecam_iatu(pp);
+		if (ret) {
+			dev_err(pci->dev, "Failed to configure iATU in ECAM mode\n");
+			return ret;
+		}
+	}
+
 	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0);
 
 	/* Program correct class for RC */
@@ -1113,7 +1122,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
 	void __iomem *mem;
 	int ret;
 
-	if (pci->num_ob_windows <= pci->pp.msg_atu_index) {
+	if (pci->num_ob_windows <= pci->ob_atu_index) {
 		dev_err(pci->dev, "No available iATU enteries\n");
 		return -ENOSPC;
 	}
@@ -1127,7 +1136,7 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
 	atu.routing = PCIE_MSG_TYPE_R_BC;
 	atu.type = PCIE_ATU_TYPE_MSG;
 	atu.size = resource_size(pci->pp.msg_res);
-	atu.index = pci->pp.msg_atu_index;
+	atu.index = pci->ob_atu_index;
 
 	atu.parent_bus_addr = pci->pp.msg_res->start - pci->parent_bus_offset;
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c644216995f69cbf065e61a0392bf1e5e32cf56e..f9f3c2f3532e0d0e9f8e4f42d8c5c9db68d55272 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -476,6 +476,9 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
 	u32 retries, val;
 	u64 limit_addr;
 
+	if (atu->index > pci->num_ob_windows)
+		return -ENOSPC;
+
 	limit_addr = parent_bus_addr + atu->size - 1;
 
 	if ((limit_addr & ~pci->region_limit) != (parent_bus_addr & ~pci->region_limit) ||
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index e995f692a1ecd10130d3be3358827f801811387f..efbcc141a26e179cb2e4acf6d2d19d75535ddb91 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -424,7 +424,6 @@ struct dw_pcie_rp {
 	raw_spinlock_t		lock;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
 	bool			use_atu_msg;
-	int			msg_atu_index;
 	struct resource		*msg_res;
 	bool			use_linkup_irq;
 	struct pci_eq_presets	presets;
@@ -502,6 +501,7 @@ struct dw_pcie {
 	resource_size_t		atu_phys_addr;
 	size_t			atu_size;
 	resource_size_t		parent_bus_offset;
+	int			ob_atu_index;
 	u32			num_ib_windows;
 	u32			num_ob_windows;
 	u32			region_align;

-- 
2.34.1
Re: [PATCH v2 3/3] PCI: dwc: Fix missing iATU setup when ECAM is enabled
Posted by Niklas Cassel 2 weeks, 3 days ago
On Mon, Dec 29, 2025 at 04:12:43PM +0530, Krishna Chaitanya Chundru wrote:
> @@ -476,6 +476,9 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
>  	u32 retries, val;
>  	u64 limit_addr;
>  
> +	if (atu->index > pci->num_ob_windows)

This should be:

if (atu->index >= pci->num_ob_windows)


> +		return -ENOSPC;
> +
>  	limit_addr = parent_bus_addr + atu->size - 1;
>  
>  	if ((limit_addr & ~pci->region_limit) != (parent_bus_addr & ~pci->region_limit) ||
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index e995f692a1ecd10130d3be3358827f801811387f..efbcc141a26e179cb2e4acf6d2d19d75535ddb91 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -424,7 +424,6 @@ struct dw_pcie_rp {
>  	raw_spinlock_t		lock;
>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>  	bool			use_atu_msg;
> -	int			msg_atu_index;
>  	struct resource		*msg_res;
>  	bool			use_linkup_irq;
>  	struct pci_eq_presets	presets;
> @@ -502,6 +501,7 @@ struct dw_pcie {
>  	resource_size_t		atu_phys_addr;
>  	size_t			atu_size;
>  	resource_size_t		parent_bus_offset;
> +	int			ob_atu_index;
>  	u32			num_ib_windows;
>  	u32			num_ob_windows;
>  	u32			region_align;
> 
> -- 
> 2.34.1
>
Re: [PATCH v2 3/3] PCI: dwc: Fix missing iATU setup when ECAM is enabled
Posted by Niklas Cassel 2 weeks, 3 days ago
On Mon, Dec 29, 2025 at 04:12:43PM +0530, Krishna Chaitanya Chundru wrote:
> When ECAM is enabled, the driver skipped calling dw_pcie_iatu_setup()
> before configuring ECAM iATU entries. This left IO and MEM outbound
> windows unprogrammed, resulting in broken IO transactions. Additionally,
> dw_pcie_config_ecam_iatu() was only called during host initialization,
> so ECAM-related iATU entries were not restored after suspend/resume,
> leading to failures in configuration space access
> 
> To resolve these issues, the ECAM iATU configuration is moved into
> dw_pcie_setup_rc(). At the same time, dw_pcie_iatu_setup() is invoked
> when ECAM is enabled.
> 
> Rename msg_atu_index to ob_atu_index to track the next available outbound
> iATU index for ECAM and MSG TLP windows. Furthermore, an error check is
> added in dw_pcie_prog_outbound_atu() to avoid programming beyond
> num_ob_windows.
> 
> Fixes: f6fd357f7afb ("PCI: dwc: Prepare the driver for enabling ECAM mechanism using iATU 'CFG Shift Feature'")
> Reported-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Closes: https://lore.kernel.org/all/alpine.DEB.2.21.2511280256260.36486@angie.orcam.me.uk/
> Tested-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 41 ++++++++++++++---------
>  drivers/pci/controller/dwc/pcie-designware.c      |  3 ++
>  drivers/pci/controller/dwc/pcie-designware.h      |  2 +-
>  3 files changed, 29 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 88b6ace0607e97bf6dd6bf7886baaa13bf267e6e..cb1b5b2a2fe61eb5901e57a60f8f333b1c3e766b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -430,10 +430,10 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>  	/*
>  	 * Root bus under the host bridge doesn't require any iATU configuration
>  	 * as DBI region will be used to access root bus config space.
> -	 * Immediate bus under Root Bus, needs type 0 iATU configuration and
> +	 * Immediate bus under Root Bus needs type 0 iATU configuration and
>  	 * remaining buses need type 1 iATU configuration.
>  	 */
> -	atu.index = 0;
> +	atu.index = pci->ob_atu_index;

Here you change atu.index = pp->ob_atu_index;

>  	atu.type = PCIE_ATU_TYPE_CFG0;
>  	atu.parent_bus_addr = pp->cfg0_base + SZ_1M;
>  	/* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
> @@ -443,6 +443,8 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>  	if (ret)
>  		return ret;
>  
> +	pci->ob_atu_index++;
> +

Here you increment pp->ob_atu_index;

>  	bus_range_max = resource_size(bus->res);
>  
>  	if (bus_range_max < 2)
> @@ -455,7 +457,13 @@ static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>  	atu.size = (SZ_1M * bus_range_max) - SZ_2M;
>  	atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;

Yet here you do not assign atu.index = pp->ob_atu_index;

this means that you will overwrite the settings for the iATU that you just
configured above, rather than using a different iATU.

So this seems broken.

I have posted a patch proposal that avoids introducing a new struct member.

Perhaps you can help test it:
https://lore.kernel.org/linux-pci/aXKUW8euDVaRJofR@ryzen/


Kind regards,
Niklas

>  
> -	return dw_pcie_prog_outbound_atu(pci, &atu);
> +	ret = dw_pcie_prog_outbound_atu(pci, &atu);
> +	if (ret)
> +		return ret;
> +
> +	pci->ob_atu_index++;
> +
> +	return 0;
>  }
>  
>  static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)