[PATCH] pci: layerscape: Fix spurious writes and panic
The fdt_fixup_pcie_ls() scans all PCI devices and assumes that all PCI root devices are layerscape PCIe controllers. Unfortunately, this is not true for the LS1028A. There is one additional static PCI root complex (this contains the networking devices) which has nothing to do with the layerscape PCIe controllers. On recent U-Boot versions this results in the following panic:
"Synchronous Abort" handler, esr 0x96000044 elr: 000000009602fa04 lr : 000000009602f9f4 (reloc) elr: 00000000fbd73a04 lr : 00000000fbd739f4 x0 : 0080000002000101 x1 : 0000000000000000 x2 : 00000000fbde9000 x3 : 0000000000000001 x4 : 0000000000000000 x5 : 0000000000000030 x6 : 00000000fbdbd460 x7 : 00000000fbb3d3a0 x8 : 0000000000000002 x9 : 000000000000000c x10: 00000000ffffffe8 x11: 0000000000000006 x12: 000000000001869f x13: 0000000000000a2c x14: 00000000fbb3d2cc x15: 00000000ffffffff x16: 0000000000010000 x17: 0000000000000000 x18: 00000000fbb3fda0 x19: 0000000000000800 x20: 0000000000000000 x21: 00000001f0000000 x22: 0000000000000800 x23: 0000000000000009 x24: 00000000fbdc3c1b x25: 00000000fbdc28e5 x26: 00000000fbdcc008 x27: 00000000fbdc16e2 x28: 000000000f000000 x29: 00000000fbb3d3a0
Code: 394072a1 f94006a0 34000041 5ac00a94 (b8336814) Resetting CPU ...
This bug already existed in former versions, but the spurious write was never trapped, because the destination address was a valid address (by pure luck).
Make sure the PCI root is actually one of the expected PCIe layerscape controllers by matching its compatible string.
Signed-off-by: Michael Walle michael@walle.cc --- drivers/pci/pcie_layerscape_fixup.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/pci/pcie_layerscape_fixup.c b/drivers/pci/pcie_layerscape_fixup.c index 94de4edaf4..76706487e2 100644 --- a/drivers/pci/pcie_layerscape_fixup.c +++ b/drivers/pci/pcie_layerscape_fixup.c @@ -187,6 +187,12 @@ static void fdt_fixup_pcie_ls(void *blob) pci_find_next_device(&dev)) { for (bus = dev; device_is_on_pci_bus(bus);) bus = bus->parent; + + /* Only do the fixups for layerscape PCIe controllers */ + if (!device_is_compatible(bus, "fsl,ls-pcie") && + !device_is_compatible(bus, CONFIG_FSL_PCIE_COMPAT)) + continue; + pcie_rc = dev_get_priv(bus);
streamid = pcie_next_streamid(pcie_rc->stream_id_cur,
Hi Michael,
On Tue, Aug 04, 2020 at 12:16:33AM +0200, Michael Walle wrote:
The fdt_fixup_pcie_ls() scans all PCI devices and assumes that all PCI root devices are layerscape PCIe controllers. Unfortunately, this is not true for the LS1028A. There is one additional static PCI root complex (this contains the networking devices) which has nothing to do with the layerscape PCIe controllers. On recent U-Boot versions this results in the following panic:
"Synchronous Abort" handler, esr 0x96000044 elr: 000000009602fa04 lr : 000000009602f9f4 (reloc) elr: 00000000fbd73a04 lr : 00000000fbd739f4 x0 : 0080000002000101 x1 : 0000000000000000 x2 : 00000000fbde9000 x3 : 0000000000000001 x4 : 0000000000000000 x5 : 0000000000000030 x6 : 00000000fbdbd460 x7 : 00000000fbb3d3a0 x8 : 0000000000000002 x9 : 000000000000000c x10: 00000000ffffffe8 x11: 0000000000000006 x12: 000000000001869f x13: 0000000000000a2c x14: 00000000fbb3d2cc x15: 00000000ffffffff x16: 0000000000010000 x17: 0000000000000000 x18: 00000000fbb3fda0 x19: 0000000000000800 x20: 0000000000000000 x21: 00000001f0000000 x22: 0000000000000800 x23: 0000000000000009 x24: 00000000fbdc3c1b x25: 00000000fbdc28e5 x26: 00000000fbdcc008 x27: 00000000fbdc16e2 x28: 000000000f000000 x29: 00000000fbb3d3a0
Code: 394072a1 f94006a0 34000041 5ac00a94 (b8336814) Resetting CPU ...
This bug already existed in former versions, but the spurious write was never trapped, because the destination address was a valid address (by pure luck).
Make sure the PCI root is actually one of the expected PCIe layerscape controllers by matching its compatible string.
Signed-off-by: Michael Walle michael@walle.cc
This is equivalent to: https://github.com/openil/u-boot/commit/6b5f833381a06503bb5fef3ef933cb62d317...
Sorry for not upstreaming it (and a bunch of other patches), I simply gave up because the maintainer response time in U-Boot is so slow.
Reviewed-by: Vladimir Oltean vladimir.oltean@nxp.com
drivers/pci/pcie_layerscape_fixup.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/pci/pcie_layerscape_fixup.c b/drivers/pci/pcie_layerscape_fixup.c index 94de4edaf4..76706487e2 100644 --- a/drivers/pci/pcie_layerscape_fixup.c +++ b/drivers/pci/pcie_layerscape_fixup.c @@ -187,6 +187,12 @@ static void fdt_fixup_pcie_ls(void *blob) pci_find_next_device(&dev)) { for (bus = dev; device_is_on_pci_bus(bus);) bus = bus->parent;
/* Only do the fixups for layerscape PCIe controllers */if (!device_is_compatible(bus, "fsl,ls-pcie") &&!device_is_compatible(bus, CONFIG_FSL_PCIE_COMPAT))continue;pcie_rc = dev_get_priv(bus);
streamid = pcie_next_streamid(pcie_rc->stream_id_cur,
-- 2.20.1
Thanks, -Vladimir
Hi Michael, Hi Vladimir,
Am Di., 4. Aug. 2020 um 00:24 Uhr schrieb Vladimir Oltean olteanv@gmail.com:
Hi Michael,
On Tue, Aug 04, 2020 at 12:16:33AM +0200, Michael Walle wrote:
The fdt_fixup_pcie_ls() scans all PCI devices and assumes that all PCI root devices are layerscape PCIe controllers. Unfortunately, this is not true for the LS1028A. There is one additional static PCI root complex (this contains the networking devices) which has nothing to do with the layerscape PCIe controllers. On recent U-Boot versions this results in the following panic:
"Synchronous Abort" handler, esr 0x96000044 elr: 000000009602fa04 lr : 000000009602f9f4 (reloc) elr: 00000000fbd73a04 lr : 00000000fbd739f4 x0 : 0080000002000101 x1 : 0000000000000000 x2 : 00000000fbde9000 x3 : 0000000000000001 x4 : 0000000000000000 x5 : 0000000000000030 x6 : 00000000fbdbd460 x7 : 00000000fbb3d3a0 x8 : 0000000000000002 x9 : 000000000000000c x10: 00000000ffffffe8 x11: 0000000000000006 x12: 000000000001869f x13: 0000000000000a2c x14: 00000000fbb3d2cc x15: 00000000ffffffff x16: 0000000000010000 x17: 0000000000000000 x18: 00000000fbb3fda0 x19: 0000000000000800 x20: 0000000000000000 x21: 00000001f0000000 x22: 0000000000000800 x23: 0000000000000009 x24: 00000000fbdc3c1b x25: 00000000fbdc28e5 x26: 00000000fbdcc008 x27: 00000000fbdc16e2 x28: 000000000f000000 x29: 00000000fbb3d3a0
Code: 394072a1 f94006a0 34000041 5ac00a94 (b8336814) Resetting CPU ...
This bug already existed in former versions, but the spurious write was never trapped, because the destination address was a valid address (by pure luck).
Make sure the PCI root is actually one of the expected PCIe layerscape controllers by matching its compatible string.
Signed-off-by: Michael Walle michael@walle.cc
This is equivalent to: https://github.com/openil/u-boot/commit/6b5f833381a06503bb5fef3ef933cb62d317...
Sorry for not upstreaming it (and a bunch of other patches), I simply gave up because the maintainer response time in U-Boot is so slow.
Reviewed-by: Vladimir Oltean vladimir.oltean@nxp.com
Tested-by: Heiko Thiery heiko.thiery@gmail.com
drivers/pci/pcie_layerscape_fixup.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/pci/pcie_layerscape_fixup.c b/drivers/pci/pcie_layerscape_fixup.c index 94de4edaf4..76706487e2 100644 --- a/drivers/pci/pcie_layerscape_fixup.c +++ b/drivers/pci/pcie_layerscape_fixup.c @@ -187,6 +187,12 @@ static void fdt_fixup_pcie_ls(void *blob) pci_find_next_device(&dev)) { for (bus = dev; device_is_on_pci_bus(bus);) bus = bus->parent;
/* Only do the fixups for layerscape PCIe controllers */if (!device_is_compatible(bus, "fsl,ls-pcie") &&!device_is_compatible(bus, CONFIG_FSL_PCIE_COMPAT))continue;pcie_rc = dev_get_priv(bus); streamid = pcie_next_streamid(pcie_rc->stream_id_cur,-- 2.20.1
Thanks, -Vladimir
Hi Tom, Hi Priyanka,
Am 2020-08-04 00:16, schrieb Michael Walle:
The fdt_fixup_pcie_ls() scans all PCI devices and assumes that all PCI root devices are layerscape PCIe controllers. Unfortunately, this is not true for the LS1028A. There is one additional static PCI root complex (this contains the networking devices) which has nothing to do with the layerscape PCIe controllers. On recent U-Boot versions this results in the following panic:
"Synchronous Abort" handler, esr 0x96000044 elr: 000000009602fa04 lr : 000000009602f9f4 (reloc) elr: 00000000fbd73a04 lr : 00000000fbd739f4 x0 : 0080000002000101 x1 : 0000000000000000 x2 : 00000000fbde9000 x3 : 0000000000000001 x4 : 0000000000000000 x5 : 0000000000000030 x6 : 00000000fbdbd460 x7 : 00000000fbb3d3a0 x8 : 0000000000000002 x9 : 000000000000000c x10: 00000000ffffffe8 x11: 0000000000000006 x12: 000000000001869f x13: 0000000000000a2c x14: 00000000fbb3d2cc x15: 00000000ffffffff x16: 0000000000010000 x17: 0000000000000000 x18: 00000000fbb3fda0 x19: 0000000000000800 x20: 0000000000000000 x21: 00000001f0000000 x22: 0000000000000800 x23: 0000000000000009 x24: 00000000fbdc3c1b x25: 00000000fbdc28e5 x26: 00000000fbdcc008 x27: 00000000fbdc16e2 x28: 000000000f000000 x29: 00000000fbb3d3a0
Code: 394072a1 f94006a0 34000041 5ac00a94 (b8336814) Resetting CPU ...
This bug already existed in former versions, but the spurious write was never trapped, because the destination address was a valid address (by pure luck).
Can we get this into the -rc ?
-michael
participants (3)
-
Heiko Thiery -
Michael Walle -
Vladimir Oltean