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

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(-)

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();

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

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!

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);

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

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.

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);

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!

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;

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

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!

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

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
participants (3)
-
Heiko Schocher
-
Pali Rohár
-
Tom Rini