[PATCH u-boot 0/3] pci: Do not access Freescale PCI controllers BARs registers

[PATCH u-boot 0/3] pci: Do not access Freescale PCI controllers BARs registers

[Pali Rohár]

Freescale PCI and PCIe controllers export in config space in BARs offset
for BDF address 00:00.0 internal controller registers, instead of BAR
registers. Avoid access to these registers to prevent overwriting them.

Fixes autoconfiguration of PCI and PCIe devices on Freescale PowerPC
platforms.

Pali Rohár (3):
  pci: mpc85xx: Do not access PCI BARs registers of BDF address 00:00.0
  pci: fsl: Do not access PCI BAR0 register of PCIe Root Port
  pci: auto: Remove PCI_CLASS_PROCESSOR_POWERPC autoconfig case

 drivers/pci/pci_auto.c    |  4 ----
 drivers/pci/pci_mpc85xx.c | 12 ++++++++++++
 drivers/pci/pcie_fsl.c    | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.20.1

[PATCH u-boot 1/3] pci: mpc85xx: Do not access PCI BARs registers of BDF address 00:00.0

[Pali Rohár]

At BDF address 00:00.0 is fictional device which PCI configuration header
is for configuring mpc85xx PCI controller itself. PCI config space of this
device has ATMU inbound registers on position of PCI BARs. Trying to do PCI
auto configuration of this device cause rewriting ATMU inbound registers.
To avoid it, do not allow overwriting registers at BARs positions. And
because this device does not have any PCI memory, return zeros when trying
to read PCI BARs config space registers. It signals to auto configuration
tool to not allocate any PCI memory for this device.

This information is taken from MPC8544E Reference Manual, sections
17.3.1.3, 17.3.1.1.1, 17.3.2 and 17.3.2.11. Available at NXP website:
https://www.nxp.com/docs/en/reference-manual/MPC8544ERM.pdf

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_mpc85xx.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c
index 833de816c459..249cfe66466b 100644
--- a/drivers/pci/pci_mpc85xx.c
+++ b/drivers/pci/pci_mpc85xx.c
@@ -27,6 +27,13 @@ static int mpc85xx_pci_dm_read_config(const struct udevice *dev, pci_dev_t bdf,
 		return 0;
 	}
 
+	/* Skip mpc85xx PCI controller's ATMU inbound registers */
+	if (PCI_BUS(bdf) == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+	    (offset & ~3) >= PCI_BASE_ADDRESS_0 && (offset & ~3) <= PCI_BASE_ADDRESS_5) {
+		*value = 0;
+		return 0;
+	}
+
 	addr = PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
 	out_be32(priv->cfg_addr, addr);
 	sync();
@@ -56,6 +63,11 @@ static int mpc85xx_pci_dm_write_config(struct udevice *dev, pci_dev_t bdf,
 	if (offset > 0xff)
 		return 0;
 
+	/* Skip mpc85xx PCI controller's ATMU inbound registers */
+	if (PCI_BUS(bdf) == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+	    (offset & ~3) >= PCI_BASE_ADDRESS_0 && (offset & ~3) <= PCI_BASE_ADDRESS_5)
+		return 0;
+
 	addr = PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
 	out_be32(priv->cfg_addr, addr);
 	sync();
-- 
2.20.1

Re: [PATCH u-boot 1/3] pci: mpc85xx: Do not access PCI BARs registers of BDF address 00:00.0

[Heiko Schocher]

Hello Pali,

On 20.04.23 21:44, Pali Rohár wrote:
> At BDF address 00:00.0 is fictional device which PCI configuration header
> is for configuring mpc85xx PCI controller itself. PCI config space of this
> device has ATMU inbound registers on position of PCI BARs. Trying to do PCI
> auto configuration of this device cause rewriting ATMU inbound registers.
> To avoid it, do not allow overwriting registers at BARs positions. And
> because this device does not have any PCI memory, return zeros when trying
> to read PCI BARs config space registers. It signals to auto configuration
> tool to not allocate any PCI memory for this device.
> 
> This information is taken from MPC8544E Reference Manual, sections
> 17.3.1.3, 17.3.1.1.1, 17.3.2 and 17.3.2.11. Available at NXP website:
> https://www.nxp.com/docs/en/reference-manual/MPC8544ERM.pdf
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pci_mpc85xx.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Reviewed-by: Heiko Schocher <hs@denx.de>
Tested-by: Heiko Schocher <hs@denx.de>

Tested on the socrates board, it fixes the problem:

PCI: Failed autoconfig bar 18
PCI: Failed autoconfig bar 20

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 1/3] pci: mpc85xx: Do not access PCI BARs registers of BDF address 00:00.0

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

On Thu, Apr 20, 2023 at 09:44:23PM +0200, Pali Rohár wrote:

> At BDF address 00:00.0 is fictional device which PCI configuration header
> is for configuring mpc85xx PCI controller itself. PCI config space of this
> device has ATMU inbound registers on position of PCI BARs. Trying to do PCI
> auto configuration of this device cause rewriting ATMU inbound registers.
> To avoid it, do not allow overwriting registers at BARs positions. And
> because this device does not have any PCI memory, return zeros when trying
> to read PCI BARs config space registers. It signals to auto configuration
> tool to not allocate any PCI memory for this device.
> 
> This information is taken from MPC8544E Reference Manual, sections
> 17.3.1.3, 17.3.1.1.1, 17.3.2 and 17.3.2.11. Available at NXP website:
> https://www.nxp.com/docs/en/reference-manual/MPC8544ERM.pdf
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> Tested-by: Heiko Schocher <hs@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot 2/3] pci: fsl: Do not access PCI BAR0 register of PCIe Root Port

[Pali Rohár]

Freescale PCIe Root Port has PEXCSRBAR register at position of PCI BAR0.
PCIe Root Port does not have any PCIe memory, so returns zero when trying
to read from PCIe Root Port BAR0 and ignore any writes.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pcie_fsl.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c
index 4600652f2b1b..06601840da85 100644
--- a/drivers/pci/pcie_fsl.c
+++ b/drivers/pci/pcie_fsl.c
@@ -58,6 +58,14 @@ static int fsl_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 		return 0;
 	}
 
+	/* Skip Freescale PCIe controller's PEXCSRBAR register */
+	if (PCI_BUS(bdf) - dev_seq(bus) == 0 &&
+	    PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+	    (offset & ~3) == PCI_BASE_ADDRESS_0) {
+		*value = 0;
+		return 0;
+	}
+
 	val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus),
 				    PCI_DEV(bdf), PCI_FUNC(bdf),
 				    offset);
@@ -95,6 +103,12 @@ static int fsl_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 	if (fsl_pcie_addr_valid(pcie, bdf))
 		return 0;
 
+	/* Skip Freescale PCIe controller's PEXCSRBAR register */
+	if (PCI_BUS(bdf) - dev_seq(bus) == 0 &&
+	    PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+	    (offset & ~3) == PCI_BASE_ADDRESS_0)
+		return 0;
+
 	val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus),
 				    PCI_DEV(bdf), PCI_FUNC(bdf),
 				    offset);
-- 
2.20.1

Re: [PATCH u-boot 2/3] pci: fsl: Do not access PCI BAR0 register of PCIe Root Port

[Heiko Schocher]

Hello Plai,

On 20.04.23 21:44, Pali Rohár wrote:
> Freescale PCIe Root Port has PEXCSRBAR register at position of PCI BAR0.
> PCIe Root Port does not have any PCIe memory, so returns zero when trying
> to read from PCIe Root Port BAR0 and ignore any writes.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pcie_fsl.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Reviewed-by: Heiko Schocher <hs@denx.de>

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 2/3] pci: fsl: Do not access PCI BAR0 register of PCIe Root Port

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

On Thu, Apr 20, 2023 at 09:44:24PM +0200, Pali Rohár wrote:

> Freescale PCIe Root Port has PEXCSRBAR register at position of PCI BAR0.
> PCIe Root Port does not have any PCIe memory, so returns zero when trying
> to read from PCIe Root Port BAR0 and ignore any writes.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> ---
>  drivers/pci/pcie_fsl.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c
> index 4600652f2b1b..06601840da85 100644
> --- a/drivers/pci/pcie_fsl.c
> +++ b/drivers/pci/pcie_fsl.c
> @@ -58,6 +58,14 @@ static int fsl_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>  		return 0;
>  	}
>  
> +	/* Skip Freescale PCIe controller's PEXCSRBAR register */
> +	if (PCI_BUS(bdf) - dev_seq(bus) == 0 &&
> +	    PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
> +	    (offset & ~3) == PCI_BASE_ADDRESS_0) {
> +		*value = 0;
> +		return 0;
> +	}
> +
>  	val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus),
>  				    PCI_DEV(bdf), PCI_FUNC(bdf),
>  				    offset);

There is no "value" in this function, only "val", which in turn is local
to the function, and *valuep which we are passed by the caller, please
re-work and send v2, thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 u-boot] pci: fsl: Do not access PCI BAR0 register of PCIe Root Port

[Pali Rohár]

Freescale PCIe Root Port has PEXCSRBAR register at position of PCI BAR0.
PCIe Root Port does not have any PCIe memory, so returns zero when trying
to read from PCIe Root Port BAR0 and ignore any writes.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pcie_fsl.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c
index 4600652f2b1b..8d89a1e5919c 100644
--- a/drivers/pci/pcie_fsl.c
+++ b/drivers/pci/pcie_fsl.c
@@ -58,6 +58,14 @@ static int fsl_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 		return 0;
 	}
 
+	/* Skip Freescale PCIe controller's PEXCSRBAR register */
+	if (PCI_BUS(bdf) - dev_seq(bus) == 0 &&
+	    PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+	    (offset & ~3) == PCI_BASE_ADDRESS_0) {
+		*valuep = 0;
+		return 0;
+	}
+
 	val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus),
 				    PCI_DEV(bdf), PCI_FUNC(bdf),
 				    offset);
@@ -95,6 +103,12 @@ static int fsl_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 	if (fsl_pcie_addr_valid(pcie, bdf))
 		return 0;
 
+	/* Skip Freescale PCIe controller's PEXCSRBAR register */
+	if (PCI_BUS(bdf) - dev_seq(bus) == 0 &&
+	    PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+	    (offset & ~3) == PCI_BASE_ADDRESS_0)
+		return 0;
+
 	val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus),
 				    PCI_DEV(bdf), PCI_FUNC(bdf),
 				    offset);
-- 
2.20.1

Re: [PATCH v2 u-boot] pci: fsl: Do not access PCI BAR0 register of PCIe Root Port

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 375 bytes --]

On Tue, May 02, 2023 at 07:53:57PM +0200, Pali Rohár wrote:

> Freescale PCIe Root Port has PEXCSRBAR register at position of PCI BAR0.
> PCIe Root Port does not have any PCIe memory, so returns zero when trying
> to read from PCIe Root Port BAR0 and ignore any writes.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot 3/3] pci: auto: Remove PCI_CLASS_PROCESSOR_POWERPC autoconfig case

[Pali Rohár]

PCI autoconfig case for PCI_CLASS_PROCESSOR_POWERPC just prints debug
message and then calls autoconfig setup code like for any other standard
endpoint device. We do not need special debug message for it, so remove
this case and handle PCI_CLASS_PROCESSOR_POWERPC via default code path.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_auto.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 14fd3bbf679e..01230360bad2 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -580,10 +580,6 @@ int dm_pciauto_config_device(struct udevice *dev)
 		break;
 #endif
 
-	case PCI_CLASS_PROCESSOR_POWERPC: /* an agent or end-point */
-		debug("PCI AutoConfig: Found PowerPC device\n");
-		/* fall through */
-
 	default:
 		dm_pciauto_setup_device(dev, pci_mem, pci_prefetch, pci_io);
 		break;
-- 
2.20.1

Re: [PATCH u-boot 3/3] pci: auto: Remove PCI_CLASS_PROCESSOR_POWERPC autoconfig case

[Heiko Schocher]

Hello Pali,

On 20.04.23 21:44, Pali Rohár wrote:
> PCI autoconfig case for PCI_CLASS_PROCESSOR_POWERPC just prints debug
> message and then calls autoconfig setup code like for any other standard
> endpoint device. We do not need special debug message for it, so remove
> this case and handle PCI_CLASS_PROCESSOR_POWERPC via default code path.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pci_auto.c | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 3/3] pci: auto: Remove PCI_CLASS_PROCESSOR_POWERPC autoconfig case

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On Thu, Apr 20, 2023 at 09:44:25PM +0200, Pali Rohár wrote:

> PCI autoconfig case for PCI_CLASS_PROCESSOR_POWERPC just prints debug
> message and then calls autoconfig setup code like for any other standard
> endpoint device. We do not need special debug message for it, so remove
> this case and handle PCI_CLASS_PROCESSOR_POWERPC via default code path.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Heiko Schocher <hs@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH u-boot 0/3] pci: Do not access Freescale PCI controllers BARs registers

[Heiko Schocher]

Hello Pali,

On 20.04.23 21:44, Pali Rohár wrote:
> Freescale PCI and PCIe controllers export in config space in BARs offset
> for BDF address 00:00.0 internal controller registers, instead of BAR
> registers. Avoid access to these registers to prevent overwriting them.
> 
> Fixes autoconfiguration of PCI and PCIe devices on Freescale PowerPC
> platforms.
> 
> Pali Rohár (3):
>   pci: mpc85xx: Do not access PCI BARs registers of BDF address 00:00.0
>   pci: fsl: Do not access PCI BAR0 register of PCIe Root Port
>   pci: auto: Remove PCI_CLASS_PROCESSOR_POWERPC autoconfig case
> 
>  drivers/pci/pci_auto.c    |  4 ----
>  drivers/pci/pci_mpc85xx.c | 12 ++++++++++++
>  drivers/pci/pcie_fsl.c    | 14 ++++++++++++++
>  3 files changed, 26 insertions(+), 4 deletions(-)

Many thanks for this series!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

Re: [PATCH u-boot 0/3] pci: Do not access Freescale PCI controllers BARs registers

[Pali Rohár]

On Friday 21 April 2023 05:54:29 Heiko Schocher wrote:
> Hello Pali,
> 
> On 20.04.23 21:44, Pali Rohár wrote:
> > Freescale PCI and PCIe controllers export in config space in BARs offset
> > for BDF address 00:00.0 internal controller registers, instead of BAR
> > registers. Avoid access to these registers to prevent overwriting them.
> > 
> > Fixes autoconfiguration of PCI and PCIe devices on Freescale PowerPC
> > platforms.
> > 
> > Pali Rohár (3):
> >   pci: mpc85xx: Do not access PCI BARs registers of BDF address 00:00.0
> >   pci: fsl: Do not access PCI BAR0 register of PCIe Root Port
> >   pci: auto: Remove PCI_CLASS_PROCESSOR_POWERPC autoconfig case
> > 
> >  drivers/pci/pci_auto.c    |  4 ----
> >  drivers/pci/pci_mpc85xx.c | 12 ++++++++++++
> >  drivers/pci/pcie_fsl.c    | 14 ++++++++++++++
> >  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> Many thanks for this series!

Ok, is something more needed?

> bye,
> Heiko
> -- 
> DENX Software Engineering GmbH,      Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de