Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices

quoted hunk

On 21.05.2021 05:03, Wang Xingang wrote:
From: Xingang Wang <redacted>

When booting with devicetree, the pci_request_acs() is called after the
enumeration and initialization of PCI devices, thus the ACS is not
enabled. And ACS should be enabled when IOMMU is detected for the
PCI host bridge, so add check for IOMMU before probe of PCI host and call
pci_request_acs() to make sure ACS will be enabled when enumerating PCI
devices.

Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
configuring IOMMU linkage")
Signed-off-by: Xingang Wang <redacted>
This patch landed in linux-next as commit 57a4ab1584e6 ("iommu/of: Fix 
pci_request_acs() before enumerating PCI devices"). Sadly it breaks PCI 
operation on ARM Juno R1 board (arch/arm64/boot/dts/arm/juno-r1.dts). It 
looks that the IOMMU controller is not probed for some reasons:

# cat /sys/kernel/debug/devices_deferred
2b600000.iommu

Reverting this patch on top of current linux-next fixes this issue. If 
you need more information to debug this issue, just let me know.

---
  drivers/iommu/of_iommu.c | 1 -
  drivers/pci/of.c         | 8 +++++++-
  2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a9d2df001149..54a14da242cc 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -205,7 +205,6 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
  			.np = master_np,
  		};
  
-		pci_request_acs();
  		err = pci_for_each_dma_alias(to_pci_dev(dev),
  					     of_pci_iommu_init, &info);
  	} else {
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index da5b414d585a..2313c3f848b0 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -581,9 +581,15 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
  
  int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge)
  {
-	if (!dev->of_node)
+	struct device_node *node = dev->of_node;
+
+	if (!node)
  		return 0;
  
+	/* Detect IOMMU and make sure ACS will be enabled */
+	if (of_property_read_bool(node, "iommu-map"))
+		pci_request_acs();
+
  	bridge->swizzle_irq = pci_common_swizzle;
  	bridge->map_irq = of_irq_parse_and_map_pci;
  
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 2021-09-01 09:59, Marek Szyprowski wrote:
On 21.05.2021 05:03, Wang Xingang wrote:
From: Xingang Wang <redacted>

When booting with devicetree, the pci_request_acs() is called after the
enumeration and initialization of PCI devices, thus the ACS is not
enabled. And ACS should be enabled when IOMMU is detected for the
PCI host bridge, so add check for IOMMU before probe of PCI host and call
pci_request_acs() to make sure ACS will be enabled when enumerating PCI
devices.

Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
configuring IOMMU linkage")
Signed-off-by: Xingang Wang <redacted>
This patch landed in linux-next as commit 57a4ab1584e6 ("iommu/of: Fix
pci_request_acs() before enumerating PCI devices"). Sadly it breaks PCI
operation on ARM Juno R1 board (arch/arm64/boot/dts/arm/juno-r1.dts). It
looks that the IOMMU controller is not probed for some reasons:

# cat /sys/kernel/debug/devices_deferred
2b600000.iommu
That IOMMU belongs to the CoreSight debug subsystem and often gets stuck waiting for a power domain (especially if you have a mismatch of SCPI/SCMI expectations between the DT and SCP firmware). Unless you're trying to use CoreSight trace that shouldn't matter.

The PCIe on Juno doesn't support ACS anyway, so I'm puzzled why this should make any difference :/

Robin.

Reverting this patch on top of current linux-next fixes this issue. If
you need more information to debug this issue, just let me know.

---
   drivers/iommu/of_iommu.c | 1 -
   drivers/pci/of.c         | 8 +++++++-
   2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a9d2df001149..54a14da242cc 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -205,7 +205,6 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
   			.np = master_np,
   		};
   
-		pci_request_acs();
   		err = pci_for_each_dma_alias(to_pci_dev(dev),
   					     of_pci_iommu_init, &info);
   	} else {
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index da5b414d585a..2313c3f848b0 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -581,9 +581,15 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
   
   int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge)
   {
-	if (!dev->of_node)
+	struct device_node *node = dev->of_node;
+
+	if (!node)
   		return 0;
   
+	/* Detect IOMMU and make sure ACS will be enabled */
+	if (of_property_read_bool(node, "iommu-map"))
+		pci_request_acs();
+
   	bridge->swizzle_irq = pci_common_swizzle;
   	bridge->map_irq = of_irq_parse_and_map_pci;
   
Best regards
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, 1 Sept 2021 at 11:58, Robin Murphy [off-list ref] wrote:
On 2021-09-01 09:59, Marek Szyprowski wrote:
On 21.05.2021 05:03, Wang Xingang wrote:
From: Xingang Wang <redacted>

When booting with devicetree, the pci_request_acs() is called after the
enumeration and initialization of PCI devices, thus the ACS is not
enabled. And ACS should be enabled when IOMMU is detected for the
PCI host bridge, so add check for IOMMU before probe of PCI host and call
pci_request_acs() to make sure ACS will be enabled when enumerating PCI
devices.

Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
configuring IOMMU linkage")
Signed-off-by: Xingang Wang <redacted>
This patch landed in linux-next as commit 57a4ab1584e6 ("iommu/of: Fix
pci_request_acs() before enumerating PCI devices"). Sadly it breaks PCI
operation on ARM Juno R1 board (arch/arm64/boot/dts/arm/juno-r1.dts). It
We've seen this on ARM Juno R2 boards too in the Linaro testfarm.

The problem is that the device can't get the "SATA link up" while booting.

see https://lkft.validation.linaro.org/scheduler/job/3416767#L577

When reverting this patch we are able to see the "SATA link up".

Cheers,
Anders

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 2021-09-02 13:51, Anders Roxell wrote:
On Wed, 1 Sept 2021 at 11:58, Robin Murphy [off-list ref] wrote:
On 2021-09-01 09:59, Marek Szyprowski wrote:
On 21.05.2021 05:03, Wang Xingang wrote:
From: Xingang Wang <redacted>

When booting with devicetree, the pci_request_acs() is called after the
enumeration and initialization of PCI devices, thus the ACS is not
enabled. And ACS should be enabled when IOMMU is detected for the
PCI host bridge, so add check for IOMMU before probe of PCI host and call
pci_request_acs() to make sure ACS will be enabled when enumerating PCI
devices.

Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
configuring IOMMU linkage")
Signed-off-by: Xingang Wang <redacted>
This patch landed in linux-next as commit 57a4ab1584e6 ("iommu/of: Fix
pci_request_acs() before enumerating PCI devices"). Sadly it breaks PCI
operation on ARM Juno R1 board (arch/arm64/boot/dts/arm/juno-r1.dts). It
We've seen this on ARM Juno R2 boards too in the Linaro testfarm.

The problem is that the device can't get the "SATA link up" while booting.

see https://lkft.validation.linaro.org/scheduler/job/3416767#L577
Hmm, what's odd there is that you don't seem to be even detecting any of the endpoints there. Notably, the switch (which both the slots and the on-board endpoints are behind) *does* support ACS even though the Root Complex doesn't, so I wonder if it's getting enabled there and causing it to forward TLPs with ACS bits set which the RC doesn't like?

I'm far from a PCI expert, but I might try running this patch on my board to see if anything else stands out.

Robin.

When reverting this patch we are able to see the "SATA link up".

Cheers,
Anders
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help