[PATCH V2] drivers: pci: dwc: configure multiple atu regions

[PATCH V2] drivers: pci: dwc: configure multiple atu regions

[Shyam Saini]

Before this change, the dwc PCIe driver configures only 1 ATU region,
which is sufficient for the devices with PCIe memory <= 4GB. However,
the driver probe fails when device uses more than 4GB of pcie memory.

Fix this by configuring multiple ATU regions for the devices which
use more than 4GB of PCIe memory.

Given each 4GB block of memory requires a new ATU region, the total
number of ATU regions are calculated using the size of PCIe device
tree node's MEM64 pref range size.

Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
---
 .../pci/controller/dwc/pcie-designware-host.c | 38 +++++++++++++++++--
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index d15a5c2d5b48..bed0b189b6ad 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -652,6 +652,33 @@ static struct pci_ops dw_pcie_ops = {
 	.write = pci_generic_config_write,
 };
 
+static int dw_pcie_num_atu_regions(struct resource_entry *entry)
+{
+	return DIV_ROUND_UP(resource_size(entry->res), SZ_4G);
+}
+
+static int dw_pcie_prog_outbound_atu_multi(struct dw_pcie *pci, int type,
+						struct resource_entry *entry)
+{
+	int idx, ret, num_regions;
+
+	num_regions = dw_pcie_num_atu_regions(entry);
+
+	for (idx = 0; idx < num_regions; idx++) {
+		dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, idx);
+		ret = dw_pcie_prog_outbound_atu(pci, idx, PCIE_ATU_TYPE_MEM,
+						entry->res->start,
+						entry->res->start - entry->offset,
+						resource_size(entry->res)/4);
+
+		if (ret)
+			goto err;
+	}
+
+err:
+	return ret;
+}
+
 static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -682,10 +709,13 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
 		if (pci->num_ob_windows <= ++i)
 			break;
 
-		ret = dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM,
-						entry->res->start,
-						entry->res->start - entry->offset,
-						resource_size(entry->res));
+		if (resource_size(entry->res) > SZ_4G)
+			ret = dw_pcie_prog_outbound_atu_multi(pci, PCIE_ATU_TYPE_MEM, entry);
+		else
+			ret = dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM,
+							entry->res->start,
+							entry->res->start - entry->offset,
+							resource_size(entry->res));
 		if (ret) {
 			dev_err(pci->dev, "Failed to set MEM range %pr\n",
 				entry->res);
-- 
2.34.1

Re: [PATCH V2] drivers: pci: dwc: configure multiple atu regions

[Manivannan Sadhasivam]

On Mon, Jun 10, 2024 at 04:50:48PM -0700, Shyam Saini wrote:

Subject should be 'PCI: dwc: ...'

> Before this change, the dwc PCIe driver configures only 1 ATU region,
> which is sufficient for the devices with PCIe memory <= 4GB. However,
> the driver probe fails when device uses more than 4GB of pcie memory.
> 

Something is not clear... This commit message implies that the driver used to
work on your hardware (you haven't mentioned which one it is) and broken by the
commit from Sergey.

As per Sergey's commit, we auto detect the dw_pcie::region_limit. If the IP is <
4.60, then the limit is 4G, otherwise depends on CX_ATU_MAX_REGION_SIZE set in
hw.

So if your IP is < 4.60, you cannot map > 4GB of outbound memory anyway. But if
it is > 4.60, you shouldn't see the failure that you reported for > 4G space
(well you can see the failure if the limit is less than the region size). In the
previous thread you mentioned that dw_pcie::region_limit is set to 4G. So how
come your driver was working previously?

PS: When reporting issue like this, please add info about your hardware also
(platform, controller driver used etc...).

- Mani

> Fix this by configuring multiple ATU regions for the devices which
> use more than 4GB of PCIe memory.
> 
> Given each 4GB block of memory requires a new ATU region, the total
> number of ATU regions are calculated using the size of PCIe device
> tree node's MEM64 pref range size.
> 
> Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 38 +++++++++++++++++--
>  1 file changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index d15a5c2d5b48..bed0b189b6ad 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -652,6 +652,33 @@ static struct pci_ops dw_pcie_ops = {
>  	.write = pci_generic_config_write,
>  };
>  
> +static int dw_pcie_num_atu_regions(struct resource_entry *entry)
> +{
> +	return DIV_ROUND_UP(resource_size(entry->res), SZ_4G);
> +}
> +
> +static int dw_pcie_prog_outbound_atu_multi(struct dw_pcie *pci, int type,
> +						struct resource_entry *entry)
> +{
> +	int idx, ret, num_regions;
> +
> +	num_regions = dw_pcie_num_atu_regions(entry);
> +
> +	for (idx = 0; idx < num_regions; idx++) {
> +		dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, idx);
> +		ret = dw_pcie_prog_outbound_atu(pci, idx, PCIE_ATU_TYPE_MEM,
> +						entry->res->start,
> +						entry->res->start - entry->offset,
> +						resource_size(entry->res)/4);
> +
> +		if (ret)
> +			goto err;
> +	}
> +
> +err:
> +	return ret;
> +}
> +
>  static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -682,10 +709,13 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  		if (pci->num_ob_windows <= ++i)
>  			break;
>  
> -		ret = dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM,
> -						entry->res->start,
> -						entry->res->start - entry->offset,
> -						resource_size(entry->res));
> +		if (resource_size(entry->res) > SZ_4G)
> +			ret = dw_pcie_prog_outbound_atu_multi(pci, PCIE_ATU_TYPE_MEM, entry);
> +		else
> +			ret = dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM,
> +							entry->res->start,
> +							entry->res->start - entry->offset,
> +							resource_size(entry->res));
>  		if (ret) {
>  			dev_err(pci->dev, "Failed to set MEM range %pr\n",
>  				entry->res);
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH V2] drivers: pci: dwc: configure multiple atu regions

[Shyam Saini]

Hi Manivannan,

>> Before this change, the dwc PCIe driver configures only 1 ATU region,
>> which is sufficient for the devices with PCIe memory <= 4GB. However,
>> the driver probe fails when device uses more than 4GB of pcie memory.
>> 
> Something is not clear... This commit message implies that the driver used to
> work on your hardware (you haven't mentioned which one it is) and broken by the
> commit from Sergey.

sorry for any confusion, the driver use to work in v5.10 kernel, with v6.0 kernel it
fails to probe with following messages:
layerscape-pcie xx00000.pcie: Failed to set MEM range ...
layerscape-pcie: probe of xx00000.pcie failed with error -22

By tracing code, I found that the probe was failing on [1] this check,
which was added in [2] upstream commit from Serge and finally, the ATU upper bound limit was
set to 4G in [3] commit

pci driver probe succeeds either when
         1) I remove [1] check
         2) or by setting the ATU limit to the size of Mem64 range (my original patch [4])

> As per Sergey's commit, we auto detect the dw_pcie::region_limit. If the IP is <
> 4.60, then the limit is 4G, otherwise depends on CX_ATU_MAX_REGION_SIZE set in
> hw.

I couldn't find any reference of CX_ATU_MAX_REGION_SIZE in my PCIe TRM, perhaps because it
is older than v4.60

> So if your IP is < 4.60, you cannot map > 4GB of outbound memory anyway. But if
> it is > 4.60, you shouldn't see the failure that you reported for > 4G space
> (well you can see the failure if the limit is less than the region size). In the
> previous thread you mentioned that dw_pcie::region_limit is set to 4G. So how
> come your driver was working previously?

The PCIe IP is from Synopsys and is older than 4.60, the board is from Freescale LX2
family. The board uses drivers/pci/controller/dwc/pci-layerscape.c driver
Given PCIe IP is older than 4.60, I am not sure why it was working earlier for a memory range larger than 4G
Highly appreciate your guidance and help on this.

Best regards,
Shyam

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-designware.c?h=v6.10-rc3#n480
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/controller/dwc?h=v6.10-rc3&id=edf408b946d37cc70fbf0db6ab85bbf67dae1894
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/controller/dwc?h=v6.10-rc3&id=89473aa9ab261948ed13b16effe841a675efed77
[4] https://lore.kernel.org/linux-pci/20240523001046.1413579-1-shyamsaini@linux.microsoft.com/

Re: [PATCH V2] drivers: pci: dwc: configure multiple atu regions

[Serge Semin]

Hi Shyam

On Thu, Jun 13, 2024 at 03:47:47PM -0700, Shyam Saini wrote:
> Hi Manivannan,
> 
> >> Before this change, the dwc PCIe driver configures only 1 ATU region,
> >> which is sufficient for the devices with PCIe memory <= 4GB. However,
> >> the driver probe fails when device uses more than 4GB of pcie memory.
> >> 
> > Something is not clear... This commit message implies that the driver used to
> > work on your hardware (you haven't mentioned which one it is) and broken by the
> > commit from Sergey.
> 
> sorry for any confusion, the driver use to work in v5.10 kernel, with v6.0 kernel it
> fails to probe with following messages:
> layerscape-pcie xx00000.pcie: Failed to set MEM range ...
> layerscape-pcie: probe of xx00000.pcie failed with error -22
> 
> By tracing code, I found that the probe was failing on [1] this check,
> which was added in [2] upstream commit from Serge and finally, the ATU upper bound limit was
> set to 4G in [3] commit
> 
> pci driver probe succeeds either when
>          1) I remove [1] check
>          2) or by setting the ATU limit to the size of Mem64 range (my original patch [4])
> 
> > As per Sergey's commit, we auto detect the dw_pcie::region_limit. If the IP is <
> > 4.60, then the limit is 4G, otherwise depends on CX_ATU_MAX_REGION_SIZE set in
> > hw.
> 

> I couldn't find any reference of CX_ATU_MAX_REGION_SIZE in my PCIe TRM, perhaps because it
> is older than v4.60

Please find the line containing "iATU: unroll " in the boot log and
copy it here. Also please provide a content of the dw_pcie::version
field _after_ the dw_pcie_version_detect() method is executed.

> 
> > So if your IP is < 4.60, you cannot map > 4GB of outbound memory anyway. But if
> > it is > 4.60, you shouldn't see the failure that you reported for > 4G space
> > (well you can see the failure if the limit is less than the region size). In the
> > previous thread you mentioned that dw_pcie::region_limit is set to 4G. So how
> > come your driver was working previously?
> 

> The PCIe IP is from Synopsys and is older than 4.60,

If you know what the actual IP-core version is and it's older than
4.70a, then it's better to pre-define the version in the
drivers/pci/controller/dwc/pci-layerscape.c probe procedure (like it's
done in drivers/pci/controller/dwc/pci-keystone.c,
drivers/pci/controller/dwc/pcie-bt1.c).

> the board is from Freescale LX2
> family.

I failed to find the LX2 PCIe controller support in the
drivers/pci/controller/dwc/pci-layerscape.c driver. AFAICS the
drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c driver is responsible
for working with the LX2 PCIe host. Am I missing something?

Anyway I've checked all the DT-nodes defined for the Freescale Layerscape
PCIe host controllers. None of them have the PCIe ranges defined with
the window size greater than 4G (actually all of them of the 1G size).
So the next question is: what DT source have you been using on your
platform and what is the DT-node defined for the PCIe host controller
in there?

> The board uses drivers/pci/controller/dwc/pci-layerscape.c driver
> Given PCIe IP is older than 4.60, I am not sure why it was working earlier for a memory range larger than 4G
> Highly appreciate your guidance and help on this.

It has been working earlier because the kernel 5.10 didn't have
PCI ranges check for not exceeding the iATU regions maximum limit.

But once again, the DW PCIe Root-port driver has been designed to 
allocate a _single_ iATU region for each PCIe memory ranges. Thus if the
ranges exceeds the maximum limit, the mapping won't work for the
addresses greater than the maximum limit. I've already explained it
here:
https://lore.kernel.org/linux-pci/hxluc6qth4temdyxloekbhoy4iielyvxmmhp3u47qwtcxb5t5v@v5hdzvqmrsyv

Moreover the IP-core databook not only explicitly prohibits to define the iATU
ranges greater than 4GB on the DW PCIe device older than 4.60a (text
from v4.21a databook), but has more strict constraint of the regions
not crossing the 4GB boundary:
"The upper 32 bits of the Target Address register always forms the
upper 32 bits of the translated address because:
■ The maximum region size is 4 GB.
■ A region must not cross a 4 GB boundary."

(It's obvious though since the iATU Limit Address register is of the
32-bit size on the old controllers.)

So you either need to allocate several iATU regions to cover the
requested PCIe ranges, or fix the PCIe controller DT-node to having the
ranges not exceeding the maximum limit.

-Serge(y)

> 
> Best regards,
> Shyam
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-designware.c?h=v6.10-rc3#n480
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/controller/dwc?h=v6.10-rc3&id=edf408b946d37cc70fbf0db6ab85bbf67dae1894
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/controller/dwc?h=v6.10-rc3&id=89473aa9ab261948ed13b16effe841a675efed77
> [4] https://lore.kernel.org/linux-pci/20240523001046.1413579-1-shyamsaini@linux.microsoft.com/
> 
> 
> 
> 

[PATCH V2] drivers: pci: dwc: configure multiple atu regions

[Shyam Saini]

Hi Serge,

Apologies for delayed response.

> > Hi Manivannan,
> > 
> > >> Before this change, the dwc PCIe driver configures only 1 ATU region,
> > >> which is sufficient for the devices with PCIe memory <= 4GB. However,
> > >> the driver probe fails when device uses more than 4GB of pcie memory.
> > >> 
> > > Something is not clear... This commit message implies that the driver used to
> > > work on your hardware (you haven't mentioned which one it is) and broken by the
> > > commit from Sergey.
> > 
> > sorry for any confusion, the driver use to work in v5.10 kernel, with v6.0 kernel it
> > fails to probe with following messages:
> > layerscape-pcie xx00000.pcie: Failed to set MEM range ...
> > layerscape-pcie: probe of xx00000.pcie failed with error -22
> > 
> > By tracing code, I found that the probe was failing on [1] this check,
> > which was added in [2] upstream commit from Serge and finally, the ATU upper bound limit was
> > set to 4G in [3] commit
> > 
> > pci driver probe succeeds either when
> >          1) I remove [1] check
> >          2) or by setting the ATU limit to the size of Mem64 range (my original patch [4])
> > 
> > > As per Sergey's commit, we auto detect the dw_pcie::region_limit. If the IP is <
> > > 4.60, then the limit is 4G, otherwise depends on CX_ATU_MAX_REGION_SIZE set in
> > > hw.
> > 
> 
> > I couldn't find any reference of CX_ATU_MAX_REGION_SIZE in my PCIe TRM, perhaps because it
> > is older than v4.60
> 
> Please find the line containing "iATU: unroll " in the boot log and

layerscape-pcie xx00000.pcie: iATU: unroll F, 256 ob, 24 ib, align 4K, limit 4G

> copy it here. Also please provide a content of the dw_pcie::version
> field _after_ the dw_pcie_version_detect() method is executed.
version is set as 0

i see that my driver always has [1] max = 0
While I am awaiting official info about CX_ATU_MAX_REGION_SIZE, but I think CX_ATU_MAX_REGION_SIZE may not exist

> > 
> > > So if your IP is < 4.60, you cannot map > 4GB of outbound memory anyway. But if
> > > it is > 4.60, you shouldn't see the failure that you reported for > 4G space
> > > (well you can see the failure if the limit is less than the region size). In the
> > > previous thread you mentioned that dw_pcie::region_limit is set to 4G. So how
> > > come your driver was working previously?
> > 
> 
> > The PCIe IP is from Synopsys and is older than 4.60,
> 
> If you know what the actual IP-core version is and it's older than
> 4.70a, then it's better to pre-define the version in the
> drivers/pci/controller/dwc/pci-layerscape.c probe procedure (like it's
> done in drivers/pci/controller/dwc/pci-keystone.c,
> drivers/pci/controller/dwc/pcie-bt1.c).

sure, I will look into that

> > the board is from Freescale LX2
> > family.
> 
> I failed to find the LX2 PCIe controller support in the
> drivers/pci/controller/dwc/pci-layerscape.c driver. AFAICS the
> drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c driver is responsible
> for working with the LX2 PCIe host. Am I missing something?

driver is drivers/pci/controller/dwc/pci-layerscape.c
device-tree compatible string is "fsl,ls2088a-pcie"

> Anyway I've checked all the DT-nodes defined for the Freescale Layerscape
> PCIe host controllers. None of them have the PCIe ranges defined with
> the window size greater than 4G (actually all of them of the 1G size).
> So the next question is: what DT source have you been using on your
> platform and what is the DT-node defined for the PCIe host controller
> in there?

It is internal and custom pcie range. It is not present in any upstream boards dts files.

> > The board uses drivers/pci/controller/dwc/pci-layerscape.c driver
> > Given PCIe IP is older than 4.60, I am not sure why it was working earlier for a memory range larger than 4G
> > Highly appreciate your guidance and help on this.
> 
> It has been working earlier because the kernel 5.10 didn't have
> PCI ranges check for not exceeding the iATU regions maximum limit.

true

> But once again, the DW PCIe Root-port driver has been designed to 
> allocate a _single_ iATU region for each PCIe memory ranges. Thus if the
> ranges exceeds the maximum limit, the mapping won't work for the
> addresses greater than the maximum limit. I've already explained it
> here:
> https://lore.kernel.org/linux-pci/hxluc6qth4temdyxloekbhoy4iielyvxmmhp3u47qwtcxb5t5v@v5hdzvqmrsyv
> 
> Moreover the IP-core databook not only explicitly prohibits to define the iATU
> ranges greater than 4GB on the DW PCIe device older than 4.60a (text
> from v4.21a databook), but has more strict constraint of the regions
> not crossing the 4GB boundary:
> "The upper 32 bits of the Target Address register always forms the
> upper 32 bits of the translated address because:
> ■ The maximum region size is 4 GB.
> ■ A region must not cross a 4 GB boundary."
> 
> (It's obvious though since the iATU Limit Address register is of the
> 32-bit size on the old controllers.)
> 
> So you either need to allocate several iATU regions to cover the
> requested PCIe ranges, or fix the PCIe controller DT-node to having the
> ranges not exceeding the maximum limit.

we have requirement for memory ranmge greater than 4G, so it seems range can't be changed.

I am looking into your other suggestions, highly appreciate your feedback

Thanks,
Shyam

[1] https://elixir.bootlin.com/linux/v6.10-rc5/source/drivers/pci/controller/dwc/pcie-designware.c#L832

Re: [PATCH V2] drivers: pci: dwc: configure multiple atu regions

[Serge Semin]

On Wed, Jun 26, 2024 at 01:26:27AM -0700, Shyam Saini wrote:
> Hi Serge,
> 
> Apologies for delayed response.
> 
> > > Hi Manivannan,
> > > 
> > > >> Before this change, the dwc PCIe driver configures only 1 ATU region,
> > > >> which is sufficient for the devices with PCIe memory <= 4GB. However,
> > > >> the driver probe fails when device uses more than 4GB of pcie memory.
> > > >> 
> > > > Something is not clear... This commit message implies that the driver used to
> > > > work on your hardware (you haven't mentioned which one it is) and broken by the
> > > > commit from Sergey.
> > > 
> > > sorry for any confusion, the driver use to work in v5.10 kernel, with v6.0 kernel it
> > > fails to probe with following messages:
> > > layerscape-pcie xx00000.pcie: Failed to set MEM range ...
> > > layerscape-pcie: probe of xx00000.pcie failed with error -22
> > > 
> > > By tracing code, I found that the probe was failing on [1] this check,
> > > which was added in [2] upstream commit from Serge and finally, the ATU upper bound limit was
> > > set to 4G in [3] commit
> > > 
> > > pci driver probe succeeds either when
> > >          1) I remove [1] check
> > >          2) or by setting the ATU limit to the size of Mem64 range (my original patch [4])
> > > 
> > > > As per Sergey's commit, we auto detect the dw_pcie::region_limit. If the IP is <
> > > > 4.60, then the limit is 4G, otherwise depends on CX_ATU_MAX_REGION_SIZE set in
> > > > hw.
> > > 
> > 
> > > I couldn't find any reference of CX_ATU_MAX_REGION_SIZE in my PCIe TRM, perhaps because it
> > > is older than v4.60
> > 
> > Please find the line containing "iATU: unroll " in the boot log and
> 

> layerscape-pcie xx00000.pcie: iATU: unroll F, 256 ob, 24 ib, align 4K, limit 4G

Omg, 256 outbound iATU regions. It's at most _1TB_ PCIe memory
mapping. The hardware engineers much have thoroughly considered all
the possible controller use-cases.)

> 
> > copy it here. Also please provide a content of the dw_pcie::version
> > field _after_ the dw_pcie_version_detect() method is executed.

> version is set as 0
> 
> i see that my driver always has [1] max = 0
> While I am awaiting official info about CX_ATU_MAX_REGION_SIZE, but I think CX_ATU_MAX_REGION_SIZE may not exist

It's likely so. The parameter was introduced in IP-core v4.60a. Based
on that and not having the PCIE_VERSION_NUMBER_OFF register your
IP-core must be older than 4.60a.

> 
> > > 
> > > > So if your IP is < 4.60, you cannot map > 4GB of outbound memory anyway. But if
> > > > it is > 4.60, you shouldn't see the failure that you reported for > 4G space
> > > > (well you can see the failure if the limit is less than the region size). In the
> > > > previous thread you mentioned that dw_pcie::region_limit is set to 4G. So how
> > > > come your driver was working previously?
> > > 
> > 
> > > The PCIe IP is from Synopsys and is older than 4.60,
> > 
> > If you know what the actual IP-core version is and it's older than
> > 4.70a, then it's better to pre-define the version in the
> > drivers/pci/controller/dwc/pci-layerscape.c probe procedure (like it's
> > done in drivers/pci/controller/dwc/pci-keystone.c,
> > drivers/pci/controller/dwc/pcie-bt1.c).
> 

> sure, I will look into that

Great! Thanks.

> 
> > > the board is from Freescale LX2
> > > family.
> > 
> > I failed to find the LX2 PCIe controller support in the
> > drivers/pci/controller/dwc/pci-layerscape.c driver. AFAICS the
> > drivers/pci/controller/mobiveil/pcie-layerscape-gen4.c driver is responsible
> > for working with the LX2 PCIe host. Am I missing something?
> 
> driver is drivers/pci/controller/dwc/pci-layerscape.c
> device-tree compatible string is "fsl,ls2088a-pcie"

Got it.

> 
> > Anyway I've checked all the DT-nodes defined for the Freescale Layerscape
> > PCIe host controllers. None of them have the PCIe ranges defined with
> > the window size greater than 4G (actually all of them of the 1G size).
> > So the next question is: what DT source have you been using on your
> > platform and what is the DT-node defined for the PCIe host controller
> > in there?
> 
> It is internal and custom pcie range. It is not present in any upstream boards dts files.
> 
> > > The board uses drivers/pci/controller/dwc/pci-layerscape.c driver
> > > Given PCIe IP is older than 4.60, I am not sure why it was working earlier for a memory range larger than 4G
> > > Highly appreciate your guidance and help on this.
> > 
> > It has been working earlier because the kernel 5.10 didn't have
> > PCI ranges check for not exceeding the iATU regions maximum limit.
> 
> true
> 
> > But once again, the DW PCIe Root-port driver has been designed to 
> > allocate a _single_ iATU region for each PCIe memory ranges. Thus if the
> > ranges exceeds the maximum limit, the mapping won't work for the
> > addresses greater than the maximum limit. I've already explained it
> > here:
> > https://lore.kernel.org/linux-pci/hxluc6qth4temdyxloekbhoy4iielyvxmmhp3u47qwtcxb5t5v@v5hdzvqmrsyv
> > 
> > Moreover the IP-core databook not only explicitly prohibits to define the iATU
> > ranges greater than 4GB on the DW PCIe device older than 4.60a (text
> > from v4.21a databook), but has more strict constraint of the regions
> > not crossing the 4GB boundary:
> > "The upper 32 bits of the Target Address register always forms the
> > upper 32 bits of the translated address because:
> > ■ The maximum region size is 4 GB.
> > ■ A region must not cross a 4 GB boundary."
> > 
> > (It's obvious though since the iATU Limit Address register is of the
> > 32-bit size on the old controllers.)
> > 
> > So you either need to allocate several iATU regions to cover the
> > requested PCIe ranges, or fix the PCIe controller DT-node to having the
> > ranges not exceeding the maximum limit.
> 

> we have requirement for memory ranmge greater than 4G, so it seems range can't be changed.
> 
> I am looking into your other suggestions, highly appreciate your feedback

Ok. Thanks.

-Serge(y)

> 
> Thanks,
> Shyam
> 
> [1] https://elixir.bootlin.com/linux/v6.10-rc5/source/drivers/pci/controller/dwc/pcie-designware.c#L832
> 

Re: [PATCH V2] drivers: pci: dwc: configure multiple atu regions

[Serge Semin]

On Mon, Jun 10, 2024 at 04:50:48PM -0700, Shyam Saini wrote:
> Before this change, the dwc PCIe driver configures only 1 ATU region,
> which is sufficient for the devices with PCIe memory <= 4GB. However,
> the driver probe fails when device uses more than 4GB of pcie memory.
> 
> Fix this by configuring multiple ATU regions for the devices which
> use more than 4GB of PCIe memory.
> 
> Given each 4GB block of memory requires a new ATU region, the total
> number of ATU regions are calculated using the size of PCIe device
> tree node's MEM64 pref range size.
> 
> Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 38 +++++++++++++++++--
>  1 file changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index d15a5c2d5b48..bed0b189b6ad 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -652,6 +652,33 @@ static struct pci_ops dw_pcie_ops = {
>  	.write = pci_generic_config_write,
>  };
>  
> +static int dw_pcie_num_atu_regions(struct resource_entry *entry)
> +{
> +	return DIV_ROUND_UP(resource_size(entry->res), SZ_4G);
> +}
> +
> +static int dw_pcie_prog_outbound_atu_multi(struct dw_pcie *pci, int type,
> +						struct resource_entry *entry)
> +{
> +	int idx, ret, num_regions;
> +
> +	num_regions = dw_pcie_num_atu_regions(entry);
> +
> +	for (idx = 0; idx < num_regions; idx++) {
> +		dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, idx);
> +		ret = dw_pcie_prog_outbound_atu(pci, idx, PCIE_ATU_TYPE_MEM,
> +						entry->res->start,
> +						entry->res->start - entry->offset,
> +						resource_size(entry->res)/4);
> +
> +		if (ret)
> +			goto err;
> +	}
> +
> +err:
> +	return ret;
> +}
> +
>  static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -682,10 +709,13 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
>  		if (pci->num_ob_windows <= ++i)
>  			break;
>  
> -		ret = dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM,
> -						entry->res->start,
> -						entry->res->start - entry->offset,
> -						resource_size(entry->res));
> +		if (resource_size(entry->res) > SZ_4G)
> +			ret = dw_pcie_prog_outbound_atu_multi(pci, PCIE_ATU_TYPE_MEM, entry);
> +		else
> +			ret = dw_pcie_prog_outbound_atu(pci, i, PCIE_ATU_TYPE_MEM,
> +							entry->res->start,
> +							entry->res->start - entry->offset,
> +							resource_size(entry->res));

Sorry, but the change doesn't look correct.

First of all, you suggest to split up _all_ the PCIe ranges greater
than 4GB. As I already mentioned several times the DW PCIe controller
of v4.60a and younger may have the maximum limit address greater than
4GB. So the change is wrong for the modern IP-core which have the
CX_ATU_MAX_REGION_SIZE synthesis parameter set with a number greater
than 4GB.

Secondly the dw_pcie_iatu_setup() method is looping around all the
PCIe memory ranges and allocates the iATU regions one after another.
So should the dw_pcie_prog_outbound_atu_multi() allocated more than
one region, the second, third, etc iATU regions setup will be
overwritten with the subsequent PCIe memory ranges data.

Thirdly your dw_pcie_prog_outbound_atu_multi() initializes the iATU
regions starting from zero each time it's called. So each next call
will override the initialization performed in the framework of the
previous one.

AFAICS it's not that easy as it seemed at the first glance. One of the
possible solution is to fix the __dw_pcie_prog_outbound_atu() method
in a way so instead of failing on the
((limit_addr & ~pci->region_limit) != (pci_addr & ~pci->region_limit))
conditional statement the method would initialize the requested region from
cpu_addr
up to
limit_addr = clamp(limit_addr, cpu_addr, cpu_addr | pci->region_limit)
and returned (limit_addr - cpu_addr + 1) from __dw_pcie_prog_outbound_atu().

The dw_pcie_prog_outbound_atu() caller shall check the returned value.
If it's negative then error happened. If the size is the same as the
requested one, then the memory window region was successfully
initialized. If the returned value is smaller than the requested one,
then the memory window only partly covers the requested region and the
next free outbound iATU region needs to be allocated and initialized.

Note 1. The suggested semantics need to be propagated to all the
dw_pcie_prog_outbound_atu() and dw_pcie_prog_ep_outbound_atu()
callers.

Note 2. The same procedure can be implemented for the inbound iATU
regions initialization procedure in dw_pcie_prog_inbound_atu(). The
difference is only in the untranslated address argument. In
dw_pcie_prog_inbound_atu() it's pci_addr. So the initialization would
be based on the PCI-address space from
pci_addr
up to
limit_addr = clamp(limit_addr, pci_addr, pci_addr | pci->region_limit)
with returning (limit_addr - pci_addr + 1).

-Serge(y)

>  		if (ret) {
>  			dev_err(pci->dev, "Failed to set MEM range %pr\n",
>  				entry->res);
> -- 
> 2.34.1
> 

Re: [PATCH V2] drivers: pci: dwc: configure multiple atu regions

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20240610235048.319266-1-shyamsaini@linux.microsoft.com>
References: <20240610235048.319266-1-shyamsaini@linux.microsoft.com>
TO: Shyam Saini <shyamsaini@linux.microsoft.com>
TO: jingoohan1@gmail.com
CC: Sergey.Semin@baikalelectronics.ru
CC: fancer.lancer@gmail.com
CC: manivannan.sadhasivam@linaro.org
CC: robh@kernel.org
CC: linux-pci@vger.kernel.org
CC: code@tyhicks.com
CC: apais@linux.microsoft.com
CC: bboscaccy@linux.microsoft.com
CC: okaya@kernel.org
CC: shyamsaini@linux.microsoft.com
CC: srivatsa@csail.mit.edu
CC: tballasi@linux.microsoft.com
CC: vijayb@linux.microsoft.com

Hi Shyam,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/for-linus]
[also build test WARNING on linus/master v6.10-rc4]
[cannot apply to pci/next next-20240613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shyam-Saini/drivers-pci-dwc-configure-multiple-atu-regions/20240611-075252
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git for-linus
patch link:    https://lore.kernel.org/r/20240610235048.319266-1-shyamsaini%40linux.microsoft.com
patch subject: [PATCH V2] drivers: pci: dwc: configure multiple atu regions
:::::: branch date: 7 days ago
:::::: commit date: 7 days ago
config: i386-randconfig-141-20240616 (https://download.01.org/0day-ci/archive/20240618/202406180107.DRkw1UpJ-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202406180107.DRkw1UpJ-lkp@intel.com/

smatch warnings:
drivers/pci/controller/dwc/pcie-designware-host.c:679 dw_pcie_prog_outbound_atu_multi() error: uninitialized symbol 'ret'.

vim +/ret +679 drivers/pci/controller/dwc/pcie-designware-host.c

6bf4bd430cedf8 Shyam Saini 2024-06-10  659  
6bf4bd430cedf8 Shyam Saini 2024-06-10  660  static int dw_pcie_prog_outbound_atu_multi(struct dw_pcie *pci, int type,
6bf4bd430cedf8 Shyam Saini 2024-06-10  661  						struct resource_entry *entry)
6bf4bd430cedf8 Shyam Saini 2024-06-10  662  {
6bf4bd430cedf8 Shyam Saini 2024-06-10  663  	int idx, ret, num_regions;
6bf4bd430cedf8 Shyam Saini 2024-06-10  664  
6bf4bd430cedf8 Shyam Saini 2024-06-10  665  	num_regions = dw_pcie_num_atu_regions(entry);
6bf4bd430cedf8 Shyam Saini 2024-06-10  666  
6bf4bd430cedf8 Shyam Saini 2024-06-10  667  	for (idx = 0; idx < num_regions; idx++) {
6bf4bd430cedf8 Shyam Saini 2024-06-10  668  		dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, idx);
6bf4bd430cedf8 Shyam Saini 2024-06-10  669  		ret = dw_pcie_prog_outbound_atu(pci, idx, PCIE_ATU_TYPE_MEM,
6bf4bd430cedf8 Shyam Saini 2024-06-10  670  						entry->res->start,
6bf4bd430cedf8 Shyam Saini 2024-06-10  671  						entry->res->start - entry->offset,
6bf4bd430cedf8 Shyam Saini 2024-06-10  672  						resource_size(entry->res)/4);
6bf4bd430cedf8 Shyam Saini 2024-06-10  673  
6bf4bd430cedf8 Shyam Saini 2024-06-10  674  		if (ret)
6bf4bd430cedf8 Shyam Saini 2024-06-10  675  			goto err;
6bf4bd430cedf8 Shyam Saini 2024-06-10  676  	}
6bf4bd430cedf8 Shyam Saini 2024-06-10  677  
6bf4bd430cedf8 Shyam Saini 2024-06-10  678  err:
6bf4bd430cedf8 Shyam Saini 2024-06-10 @679  	return ret;
6bf4bd430cedf8 Shyam Saini 2024-06-10  680  }
6bf4bd430cedf8 Shyam Saini 2024-06-10  681  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki