All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-05-21  3:03 ` Wang Xingang
  0 siblings, 0 replies; 34+ messages in thread
From: Wang Xingang @ 2021-05-21  3:03 UTC (permalink / raw)
  To: robh, will, joro, helgaas
  Cc: robh+dt, gregkh, iommu, linux-kernel, linux-pci, xieyingtai,
	wangxingang5

From: Xingang Wang <wangxingang5@huawei.com>

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 <wangxingang5@huawei.com>
---
 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;
 
-- 
2.19.1


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-05-21  3:03 ` Wang Xingang
  0 siblings, 0 replies; 34+ messages in thread
From: Wang Xingang @ 2021-05-21  3:03 UTC (permalink / raw)
  To: robh, will, joro, helgaas
  Cc: xieyingtai, wangxingang5, gregkh, linux-kernel, iommu, robh+dt,
	linux-pci

From: Xingang Wang <wangxingang5@huawei.com>

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 <wangxingang5@huawei.com>
---
 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;
 
-- 
2.19.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-05-21  3:03 ` Wang Xingang
@ 2021-05-21 19:03   ` Rob Herring
  -1 siblings, 0 replies; 34+ messages in thread
From: Rob Herring @ 2021-05-21 19:03 UTC (permalink / raw)
  To: Wang Xingang
  Cc: Will Deacon, Joerg Roedel, Bjorn Helgaas, Greg Kroah-Hartman,
	Linux IOMMU, linux-kernel, PCI, xieyingtai

On Thu, May 20, 2021 at 10:03 PM Wang Xingang <wangxingang5@huawei.com> wrote:
>
> From: Xingang Wang <wangxingang5@huawei.com>
>
> 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 <wangxingang5@huawei.com>
> ---
>  drivers/iommu/of_iommu.c | 1 -
>  drivers/pci/of.c         | 8 +++++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-05-21 19:03   ` Rob Herring
  0 siblings, 0 replies; 34+ messages in thread
From: Rob Herring @ 2021-05-21 19:03 UTC (permalink / raw)
  To: Wang Xingang
  Cc: xieyingtai, Greg Kroah-Hartman, linux-kernel, Linux IOMMU,
	Bjorn Helgaas, PCI, Will Deacon

On Thu, May 20, 2021 at 10:03 PM Wang Xingang <wangxingang5@huawei.com> wrote:
>
> From: Xingang Wang <wangxingang5@huawei.com>
>
> 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 <wangxingang5@huawei.com>
> ---
>  drivers/iommu/of_iommu.c | 1 -
>  drivers/pci/of.c         | 8 +++++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

> 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;
>
> --
> 2.19.1
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-05-21 19:03   ` Rob Herring
@ 2021-05-31 11:24     ` Xingang Wang
  -1 siblings, 0 replies; 34+ messages in thread
From: Xingang Wang @ 2021-05-31 11:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: Will Deacon, Joerg Roedel, Bjorn Helgaas, Greg Kroah-Hartman,
	Linux IOMMU, linux-kernel, PCI, xieyingtai

Hi everyone,

On 2021/5/22 3:03, Rob Herring wrote:
> On Thu, May 20, 2021 at 10:03 PM Wang Xingang <wangxingang5@huawei.com> wrote:
>>
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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 <wangxingang5@huawei.com>
>> ---
>>   drivers/iommu/of_iommu.c | 1 -
>>   drivers/pci/of.c         | 8 +++++++-
>>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 

Ping, is this patch appropriate for mainline?

>> 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;
>>
>> --
>> 2.19.1
>>
> .
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-05-31 11:24     ` Xingang Wang
  0 siblings, 0 replies; 34+ messages in thread
From: Xingang Wang @ 2021-05-31 11:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: xieyingtai, Greg Kroah-Hartman, linux-kernel, Linux IOMMU,
	Bjorn Helgaas, PCI, Will Deacon

Hi everyone,

On 2021/5/22 3:03, Rob Herring wrote:
> On Thu, May 20, 2021 at 10:03 PM Wang Xingang <wangxingang5@huawei.com> wrote:
>>
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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 <wangxingang5@huawei.com>
>> ---
>>   drivers/iommu/of_iommu.c | 1 -
>>   drivers/pci/of.c         | 8 +++++++-
>>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 

Ping, is this patch appropriate for mainline?

>> 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;
>>
>> --
>> 2.19.1
>>
> .
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-05-21  3:03 ` Wang Xingang
@ 2021-06-04 15:36   ` Joerg Roedel
  -1 siblings, 0 replies; 34+ messages in thread
From: Joerg Roedel @ 2021-06-04 15:36 UTC (permalink / raw)
  To: Wang Xingang
  Cc: robh, will, helgaas, robh+dt, gregkh, iommu, linux-kernel,
	linux-pci, xieyingtai

On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
> 
> 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 <wangxingang5@huawei.com>
> ---
>  drivers/iommu/of_iommu.c | 1 -
>  drivers/pci/of.c         | 8 +++++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)

Should probably go through the PCI tree, so

Acked-by: Joerg Roedel <jroedel@suse.de>


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-06-04 15:36   ` Joerg Roedel
  0 siblings, 0 replies; 34+ messages in thread
From: Joerg Roedel @ 2021-06-04 15:36 UTC (permalink / raw)
  To: Wang Xingang
  Cc: xieyingtai, robh, gregkh, linux-kernel, iommu, robh+dt, helgaas,
	linux-pci, will

On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
> 
> 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 <wangxingang5@huawei.com>
> ---
>  drivers/iommu/of_iommu.c | 1 -
>  drivers/pci/of.c         | 8 +++++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)

Should probably go through the PCI tree, so

Acked-by: Joerg Roedel <jroedel@suse.de>

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-05-21  3:03 ` Wang Xingang
@ 2021-06-04 19:04   ` Bjorn Helgaas
  -1 siblings, 0 replies; 34+ messages in thread
From: Bjorn Helgaas @ 2021-06-04 19:04 UTC (permalink / raw)
  To: Wang Xingang
  Cc: robh, will, joro, robh+dt, gregkh, iommu, linux-kernel,
	linux-pci, xieyingtai, John Garry

[+cc John, who tested 6bf6c24720d3]

On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
> 
> 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.

I'm happy to apply this, but I'm a little puzzled about 6bf6c24720d3
("iommu/of: Request ACS from the PCI core when configuring IOMMU
linkage").  It was tested and fixed a problem, but I don't understand
how.

6bf6c24720d3 added the call to pci_request_acs() in
of_iommu_configure() so it currently looks like this:

  of_iommu_configure(dev, ...)
  {
    if (dev_is_pci(dev))
      pci_request_acs();

pci_request_acs() sets pci_acs_enable, which tells us to enable ACS
when enumerating PCI devices in the future.  But we only call
pci_request_acs() if we already *have* a PCI device.

So maybe 6bf6c24720d3 fixed a problem for *some* PCI devices, but not
all?  E.g., did we call of_iommu_configure() for one PCI device before
enumerating the rest?

> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
> configuring IOMMU linkage")
> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
> ---
>  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;
>  
> -- 
> 2.19.1
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-06-04 19:04   ` Bjorn Helgaas
  0 siblings, 0 replies; 34+ messages in thread
From: Bjorn Helgaas @ 2021-06-04 19:04 UTC (permalink / raw)
  To: Wang Xingang
  Cc: xieyingtai, robh, gregkh, linux-kernel, iommu, robh+dt, linux-pci, will

[+cc John, who tested 6bf6c24720d3]

On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
> 
> 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.

I'm happy to apply this, but I'm a little puzzled about 6bf6c24720d3
("iommu/of: Request ACS from the PCI core when configuring IOMMU
linkage").  It was tested and fixed a problem, but I don't understand
how.

6bf6c24720d3 added the call to pci_request_acs() in
of_iommu_configure() so it currently looks like this:

  of_iommu_configure(dev, ...)
  {
    if (dev_is_pci(dev))
      pci_request_acs();

pci_request_acs() sets pci_acs_enable, which tells us to enable ACS
when enumerating PCI devices in the future.  But we only call
pci_request_acs() if we already *have* a PCI device.

So maybe 6bf6c24720d3 fixed a problem for *some* PCI devices, but not
all?  E.g., did we call of_iommu_configure() for one PCI device before
enumerating the rest?

> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
> configuring IOMMU linkage")
> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
> ---
>  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;
>  
> -- 
> 2.19.1
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-06-04 15:36   ` Joerg Roedel
@ 2021-06-07 12:43     ` Xingang Wang
  -1 siblings, 0 replies; 34+ messages in thread
From: Xingang Wang @ 2021-06-07 12:43 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: robh, will, helgaas, robh+dt, gregkh, iommu, linux-kernel,
	linux-pci, xieyingtai

On 2021/6/4 23:36, Joerg Roedel wrote:
> On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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 <wangxingang5@huawei.com>
>> ---
>>   drivers/iommu/of_iommu.c | 1 -
>>   drivers/pci/of.c         | 8 +++++++-
>>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> Should probably go through the PCI tree, so
> 
> Acked-by: Joerg Roedel <jroedel@suse.de>
> 
> .
> 

Thanks

.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-06-07 12:43     ` Xingang Wang
  0 siblings, 0 replies; 34+ messages in thread
From: Xingang Wang @ 2021-06-07 12:43 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: xieyingtai, robh, gregkh, linux-kernel, iommu, robh+dt, helgaas,
	linux-pci, will

On 2021/6/4 23:36, Joerg Roedel wrote:
> On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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 <wangxingang5@huawei.com>
>> ---
>>   drivers/iommu/of_iommu.c | 1 -
>>   drivers/pci/of.c         | 8 +++++++-
>>   2 files changed, 7 insertions(+), 2 deletions(-)
> 
> Should probably go through the PCI tree, so
> 
> Acked-by: Joerg Roedel <jroedel@suse.de>
> 
> .
> 

Thanks

.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-06-04 19:04   ` Bjorn Helgaas
@ 2021-06-07 12:58     ` Xingang Wang
  -1 siblings, 0 replies; 34+ messages in thread
From: Xingang Wang @ 2021-06-07 12:58 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: robh, will, joro, robh+dt, gregkh, iommu, linux-kernel,
	linux-pci, xieyingtai, John Garry

On 2021/6/5 3:04, Bjorn Helgaas wrote:
> [+cc John, who tested 6bf6c24720d3]
> 
> On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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.
> 
> I'm happy to apply this, but I'm a little puzzled about 6bf6c24720d3
> ("iommu/of: Request ACS from the PCI core when configuring IOMMU
> linkage").  It was tested and fixed a problem, but I don't understand
> how.
> 
> 6bf6c24720d3 added the call to pci_request_acs() in
> of_iommu_configure() so it currently looks like this:
> 
>    of_iommu_configure(dev, ...)
>    {
>      if (dev_is_pci(dev))
>        pci_request_acs();
> 
> pci_request_acs() sets pci_acs_enable, which tells us to enable ACS
> when enumerating PCI devices in the future.  But we only call
> pci_request_acs() if we already *have* a PCI device.
> 
> So maybe 6bf6c24720d3 fixed a problem for *some* PCI devices, but not
> all?  E.g., did we call of_iommu_configure() for one PCI device before
> enumerating the rest?
> 
I test the kernel on an arm platform with qemu:

qemu-system-aarch64 \
  -cpu host \
  -kernel arch/arm64/boot/Image \
  -enable-kvm \
  -m 8G \
  -smp 2,sockets=2,cores=1,threads=1 	\
  -machine virt,kernel_irqchip=on,gic-version=3,iommu=smmuv3\
  -initrd rootfs.cpio.gz \
  -nographic \
  -append "rdinit=init console=ttyAMA0 earlycon=pl011,0x9000000 nokaslr" \
  -device pcie-root-port,port=0x1,chassis=1,id=pci.1,addr=0x8 \
  -netdev user,id=hostnet0 \
  -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=08:13:3a:5a:22:5b,bus=pci.1,addr=0x0 
\

And find that the of_iommu_configure is called after the enumeration
of the pcie-root-port. And this might only infect the first device, when 
enumerating
the rest devices, the pci_acs_enable has already be enabled.

But to make sure that the pci_acs_enable will always be set before all 
PCI devices,
it would be better to set it in initialization of PCI bridges.

Thanks

Xingang

>> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
>> configuring IOMMU linkage")
>> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
>> ---
>>   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;
>>   
>> -- 
>> 2.19.1
>>
> .
> 

.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-06-07 12:58     ` Xingang Wang
  0 siblings, 0 replies; 34+ messages in thread
From: Xingang Wang @ 2021-06-07 12:58 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: xieyingtai, robh, gregkh, linux-kernel, iommu, robh+dt, linux-pci, will

On 2021/6/5 3:04, Bjorn Helgaas wrote:
> [+cc John, who tested 6bf6c24720d3]
> 
> On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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.
> 
> I'm happy to apply this, but I'm a little puzzled about 6bf6c24720d3
> ("iommu/of: Request ACS from the PCI core when configuring IOMMU
> linkage").  It was tested and fixed a problem, but I don't understand
> how.
> 
> 6bf6c24720d3 added the call to pci_request_acs() in
> of_iommu_configure() so it currently looks like this:
> 
>    of_iommu_configure(dev, ...)
>    {
>      if (dev_is_pci(dev))
>        pci_request_acs();
> 
> pci_request_acs() sets pci_acs_enable, which tells us to enable ACS
> when enumerating PCI devices in the future.  But we only call
> pci_request_acs() if we already *have* a PCI device.
> 
> So maybe 6bf6c24720d3 fixed a problem for *some* PCI devices, but not
> all?  E.g., did we call of_iommu_configure() for one PCI device before
> enumerating the rest?
> 
I test the kernel on an arm platform with qemu:

qemu-system-aarch64 \
  -cpu host \
  -kernel arch/arm64/boot/Image \
  -enable-kvm \
  -m 8G \
  -smp 2,sockets=2,cores=1,threads=1 	\
  -machine virt,kernel_irqchip=on,gic-version=3,iommu=smmuv3\
  -initrd rootfs.cpio.gz \
  -nographic \
  -append "rdinit=init console=ttyAMA0 earlycon=pl011,0x9000000 nokaslr" \
  -device pcie-root-port,port=0x1,chassis=1,id=pci.1,addr=0x8 \
  -netdev user,id=hostnet0 \
  -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=08:13:3a:5a:22:5b,bus=pci.1,addr=0x0 
\

And find that the of_iommu_configure is called after the enumeration
of the pcie-root-port. And this might only infect the first device, when 
enumerating
the rest devices, the pci_acs_enable has already be enabled.

But to make sure that the pci_acs_enable will always be set before all 
PCI devices,
it would be better to set it in initialization of PCI bridges.

Thanks

Xingang

>> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
>> configuring IOMMU linkage")
>> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
>> ---
>>   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;
>>   
>> -- 
>> 2.19.1
>>
> .
> 

.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-06-07 12:58     ` Xingang Wang
@ 2021-06-08 10:23       ` John Garry
  -1 siblings, 0 replies; 34+ messages in thread
From: John Garry @ 2021-06-08 10:23 UTC (permalink / raw)
  To: Xingang Wang, Bjorn Helgaas
  Cc: robh, will, joro, robh+dt, gregkh, iommu, linux-kernel,
	linux-pci, xieyingtai

On 07/06/2021 13:58, Xingang Wang wrote:
> On 2021/6/5 3:04, Bjorn Helgaas wrote:
>> [+cc John, who tested 6bf6c24720d3]

I think that whoever committed this patch just blanket applied my TB 
tag. I would not have tested anything supporting ACS or even DT:
https://lore.kernel.org/linux-iommu/c8eb97b1-dab5-cc25-7669-2819f64a885d@huawei.com/

Thanks,
John

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-06-08 10:23       ` John Garry
  0 siblings, 0 replies; 34+ messages in thread
From: John Garry @ 2021-06-08 10:23 UTC (permalink / raw)
  To: Xingang Wang, Bjorn Helgaas
  Cc: xieyingtai, robh, gregkh, linux-kernel, iommu, robh+dt, linux-pci, will

On 07/06/2021 13:58, Xingang Wang wrote:
> On 2021/6/5 3:04, Bjorn Helgaas wrote:
>> [+cc John, who tested 6bf6c24720d3]

I think that whoever committed this patch just blanket applied my TB 
tag. I would not have tested anything supporting ACS or even DT:
https://lore.kernel.org/linux-iommu/c8eb97b1-dab5-cc25-7669-2819f64a885d@huawei.com/

Thanks,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-06-07 12:58     ` Xingang Wang
@ 2021-06-28 13:20       ` Xingang Wang
  -1 siblings, 0 replies; 34+ messages in thread
From: Xingang Wang @ 2021-06-28 13:20 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: robh, will, joro, robh+dt, gregkh, iommu, linux-kernel,
	linux-pci, xieyingtai, John Garry, Auger Eric, jean-philippe

Hi Bjorn,

I would like to add more explanation about what problem this patch
fixed.

I am testing the SVA/vSVA series patches written by @Jean and @Brucker.
I test with the following qemu command line, with a hisilicon SEC device
attached on a pcie-root-port.

$QEMU/qemu-system-aarch64 \
     -enable-kvm \
     -kernel $LINUX/arch/arm64/boot/Image \
     -m 16G \
     -smp cores=8,threads=1,sockets=2 	\
     -machine virt,kernel_irqchip=on,gic-version=3,iommu=smmuv3 \
     -device 
pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 \
     -device vfio-pci,host=75:00.0,bus=pci.1,addr=0x0,id=acc2 \
     -net none \
     -initrd ./mfs.cpio.gz \
     -cpu host \
     -nographic \
     -append "rdinit=init console=ttyAMA0 earlycon=pl011,0x9000000" \

And I got the guest PCI configuration:
00:00.0 Class 0600: Device 1b36:0008 		# root bus
00:01.0 Class 0604: Device 1b36:000c 		# root port
	Capabilities: [148 v1] Access Control Services
		ACSCap:	SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ UpstreamFwd+ 
EgressCtrl- DirectTrans+
		ACSCtl:	SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- 
EgressCtrl- DirectTrans-
	Kernel driver in use: pcieport
01:00.0 Class 1000: Device 19e5:a255 (rev 21) 	# SEC

The PCI configuration shows that the ACS of the pcie root port is
not enabled, while it should have.

Then when I insmod device driver and init the SVA feature, I got

[   24.342450] hisi_sec2 0000:01:00.0: cannot attach to incompatible 
domain (0 SSID bits != 10)
[   24.343731] hisi_sec2 0000:01:00.0: Failed to add to iommu group 0: -22
[   24.345243] hisi_sec2 0000:01:00.0: enabling device (0000 -> 0002)
qemu-system-aarch64: vfio_enable_vectors failed to register S1 MSI 
binding for vector 0(-2)
qemu-system-aarch64: vfio: Error: Failed to setup MSI fds: Interrupted 
system call
qemu-system-aarch64: vfio: Error: Failed to enable MSI

I figured out that this error occurs in the arm_smmu_attach_dev
when checking ssid_bits for SVA feature,
the master->ssid_bits != smmu_domain->s1_cfg.s1cdmax caused this 
problem. This is becuase the ACS of pcie-root-port is not enabled, the 
pcie-root-port and SEC device share the same domain.
And SEC's ssid_bits is 10, while pcie-root-port's s1cdmax is zero, this 
cause the problem.

And about why the ACS is not enabled in kernel, I have explained as the 
following:

On 2021/6/7 20:58, Xingang Wang wrote:
> On 2021/6/5 3:04, Bjorn Helgaas wrote:
>> [+cc John, who tested 6bf6c24720d3]
>>
>> On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
>>> From: Xingang Wang <wangxingang5@huawei.com>
>>>
>>> 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.
>>
>> I'm happy to apply this, but I'm a little puzzled about 6bf6c24720d3
>> ("iommu/of: Request ACS from the PCI core when configuring IOMMU
>> linkage").  It was tested and fixed a problem, but I don't understand
>> how.
>>
>> 6bf6c24720d3 added the call to pci_request_acs() in
>> of_iommu_configure() so it currently looks like this:
>>
>>    of_iommu_configure(dev, ...)
>>    {
>>      if (dev_is_pci(dev))
>>        pci_request_acs();
>>
>> pci_request_acs() sets pci_acs_enable, which tells us to enable ACS
>> when enumerating PCI devices in the future.  But we only call
>> pci_request_acs() if we already *have* a PCI device.
>>
>> So maybe 6bf6c24720d3 fixed a problem for *some* PCI devices, but not
>> all?  E.g., did we call of_iommu_configure() for one PCI device before
>> enumerating the rest?
>>
> I test the kernel on an arm platform with qemu:
> 
> qemu-system-aarch64 \
>   -cpu host \
>   -kernel arch/arm64/boot/Image \
>   -enable-kvm \
>   -m 8G \
>   -smp 2,sockets=2,cores=1,threads=1     \
>   -machine virt,kernel_irqchip=on,gic-version=3,iommu=smmuv3\
>   -initrd rootfs.cpio.gz \
>   -nographic \
>   -append "rdinit=init console=ttyAMA0 earlycon=pl011,0x9000000 nokaslr" \
>   -device pcie-root-port,port=0x1,chassis=1,id=pci.1,addr=0x8 \
>   -netdev user,id=hostnet0 \
>   -device 
> virtio-net-pci,netdev=hostnet0,id=net0,mac=08:13:3a:5a:22:5b,bus=pci.1,addr=0x0 
> \
> 
> And find that the of_iommu_configure is called after the enumeration
> of the pcie-root-port. And this might only infect the first device, when 
> enumerating
> the rest devices, the pci_acs_enable has already be enabled.
> 
> But to make sure that the pci_acs_enable will always be set before all 
> PCI devices,
> it would be better to set it in initialization of PCI bridges.
> 
> Thanks
> 
> Xingang
> 
>>> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
>>> configuring IOMMU linkage")
>>> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
>>> ---
>>>   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;
>>> -- 
>>> 2.19.1
>>>
>> .
>>
> 
> .

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-06-28 13:20       ` Xingang Wang
  0 siblings, 0 replies; 34+ messages in thread
From: Xingang Wang @ 2021-06-28 13:20 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: xieyingtai, robh, jean-philippe, gregkh, linux-kernel, iommu,
	robh+dt, linux-pci, will

Hi Bjorn,

I would like to add more explanation about what problem this patch
fixed.

I am testing the SVA/vSVA series patches written by @Jean and @Brucker.
I test with the following qemu command line, with a hisilicon SEC device
attached on a pcie-root-port.

$QEMU/qemu-system-aarch64 \
     -enable-kvm \
     -kernel $LINUX/arch/arm64/boot/Image \
     -m 16G \
     -smp cores=8,threads=1,sockets=2 	\
     -machine virt,kernel_irqchip=on,gic-version=3,iommu=smmuv3 \
     -device 
pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 \
     -device vfio-pci,host=75:00.0,bus=pci.1,addr=0x0,id=acc2 \
     -net none \
     -initrd ./mfs.cpio.gz \
     -cpu host \
     -nographic \
     -append "rdinit=init console=ttyAMA0 earlycon=pl011,0x9000000" \

And I got the guest PCI configuration:
00:00.0 Class 0600: Device 1b36:0008 		# root bus
00:01.0 Class 0604: Device 1b36:000c 		# root port
	Capabilities: [148 v1] Access Control Services
		ACSCap:	SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ UpstreamFwd+ 
EgressCtrl- DirectTrans+
		ACSCtl:	SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- 
EgressCtrl- DirectTrans-
	Kernel driver in use: pcieport
01:00.0 Class 1000: Device 19e5:a255 (rev 21) 	# SEC

The PCI configuration shows that the ACS of the pcie root port is
not enabled, while it should have.

Then when I insmod device driver and init the SVA feature, I got

[   24.342450] hisi_sec2 0000:01:00.0: cannot attach to incompatible 
domain (0 SSID bits != 10)
[   24.343731] hisi_sec2 0000:01:00.0: Failed to add to iommu group 0: -22
[   24.345243] hisi_sec2 0000:01:00.0: enabling device (0000 -> 0002)
qemu-system-aarch64: vfio_enable_vectors failed to register S1 MSI 
binding for vector 0(-2)
qemu-system-aarch64: vfio: Error: Failed to setup MSI fds: Interrupted 
system call
qemu-system-aarch64: vfio: Error: Failed to enable MSI

I figured out that this error occurs in the arm_smmu_attach_dev
when checking ssid_bits for SVA feature,
the master->ssid_bits != smmu_domain->s1_cfg.s1cdmax caused this 
problem. This is becuase the ACS of pcie-root-port is not enabled, the 
pcie-root-port and SEC device share the same domain.
And SEC's ssid_bits is 10, while pcie-root-port's s1cdmax is zero, this 
cause the problem.

And about why the ACS is not enabled in kernel, I have explained as the 
following:

On 2021/6/7 20:58, Xingang Wang wrote:
> On 2021/6/5 3:04, Bjorn Helgaas wrote:
>> [+cc John, who tested 6bf6c24720d3]
>>
>> On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
>>> From: Xingang Wang <wangxingang5@huawei.com>
>>>
>>> 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.
>>
>> I'm happy to apply this, but I'm a little puzzled about 6bf6c24720d3
>> ("iommu/of: Request ACS from the PCI core when configuring IOMMU
>> linkage").  It was tested and fixed a problem, but I don't understand
>> how.
>>
>> 6bf6c24720d3 added the call to pci_request_acs() in
>> of_iommu_configure() so it currently looks like this:
>>
>>    of_iommu_configure(dev, ...)
>>    {
>>      if (dev_is_pci(dev))
>>        pci_request_acs();
>>
>> pci_request_acs() sets pci_acs_enable, which tells us to enable ACS
>> when enumerating PCI devices in the future.  But we only call
>> pci_request_acs() if we already *have* a PCI device.
>>
>> So maybe 6bf6c24720d3 fixed a problem for *some* PCI devices, but not
>> all?  E.g., did we call of_iommu_configure() for one PCI device before
>> enumerating the rest?
>>
> I test the kernel on an arm platform with qemu:
> 
> qemu-system-aarch64 \
>   -cpu host \
>   -kernel arch/arm64/boot/Image \
>   -enable-kvm \
>   -m 8G \
>   -smp 2,sockets=2,cores=1,threads=1     \
>   -machine virt,kernel_irqchip=on,gic-version=3,iommu=smmuv3\
>   -initrd rootfs.cpio.gz \
>   -nographic \
>   -append "rdinit=init console=ttyAMA0 earlycon=pl011,0x9000000 nokaslr" \
>   -device pcie-root-port,port=0x1,chassis=1,id=pci.1,addr=0x8 \
>   -netdev user,id=hostnet0 \
>   -device 
> virtio-net-pci,netdev=hostnet0,id=net0,mac=08:13:3a:5a:22:5b,bus=pci.1,addr=0x0 
> \
> 
> And find that the of_iommu_configure is called after the enumeration
> of the pcie-root-port. And this might only infect the first device, when 
> enumerating
> the rest devices, the pci_acs_enable has already be enabled.
> 
> But to make sure that the pci_acs_enable will always be set before all 
> PCI devices,
> it would be better to set it in initialization of PCI bridges.
> 
> Thanks
> 
> Xingang
> 
>>> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
>>> configuring IOMMU linkage")
>>> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
>>> ---
>>>   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;
>>> -- 
>>> 2.19.1
>>>
>> .
>>
> 
> .
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-05-21  3:03 ` Wang Xingang
@ 2021-08-20 19:57   ` Bjorn Helgaas
  -1 siblings, 0 replies; 34+ messages in thread
From: Bjorn Helgaas @ 2021-08-20 19:57 UTC (permalink / raw)
  To: Wang Xingang
  Cc: robh, will, joro, robh+dt, gregkh, iommu, linux-kernel,
	linux-pci, xieyingtai

On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
> 
> 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 <wangxingang5@huawei.com>

Applied to pci/virtualization for v5.15, thanks!

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-08-20 19:57   ` Bjorn Helgaas
  0 siblings, 0 replies; 34+ messages in thread
From: Bjorn Helgaas @ 2021-08-20 19:57 UTC (permalink / raw)
  To: Wang Xingang
  Cc: xieyingtai, robh, gregkh, linux-kernel, iommu, robh+dt, linux-pci, will

On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
> 
> 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 <wangxingang5@huawei.com>

Applied to pci/virtualization for v5.15, thanks!

> ---
>  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;
>  
> -- 
> 2.19.1
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
       [not found] ` <CGME20210901085937eucas1p2d02da65cac797706ca3a10b8a2eb8ba2@eucas1p2.samsung.com>
  2021-09-01  8:59     ` Marek Szyprowski
@ 2021-09-01  8:59     ` Marek Szyprowski
  0 siblings, 0 replies; 34+ messages in thread
From: Marek Szyprowski @ 2021-09-01  8:59 UTC (permalink / raw)
  To: Wang Xingang, robh, will, joro, helgaas
  Cc: robh+dt, gregkh, iommu, linux-kernel, linux-pci, xieyingtai,
	linux-arm-kernel, Robin Murphy

On 21.05.2021 05:03, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
>
> 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 <wangxingang5@huawei.com>

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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-01  8:59     ` Marek Szyprowski
  0 siblings, 0 replies; 34+ messages in thread
From: Marek Szyprowski @ 2021-09-01  8:59 UTC (permalink / raw)
  To: Wang Xingang, robh, will, joro, helgaas
  Cc: xieyingtai, gregkh, linux-kernel, iommu, robh+dt, linux-pci,
	Robin Murphy, linux-arm-kernel

On 21.05.2021 05:03, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
>
> 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 <wangxingang5@huawei.com>

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

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-01  8:59     ` Marek Szyprowski
  0 siblings, 0 replies; 34+ messages in thread
From: Marek Szyprowski @ 2021-09-01  8:59 UTC (permalink / raw)
  To: Wang Xingang, robh, will, joro, helgaas
  Cc: robh+dt, gregkh, iommu, linux-kernel, linux-pci, xieyingtai,
	linux-arm-kernel, Robin Murphy

On 21.05.2021 05:03, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
>
> 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 <wangxingang5@huawei.com>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-09-01  8:59     ` Marek Szyprowski
  (?)
@ 2021-09-01  9:58       ` Robin Murphy
  -1 siblings, 0 replies; 34+ messages in thread
From: Robin Murphy @ 2021-09-01  9:58 UTC (permalink / raw)
  To: Marek Szyprowski, Wang Xingang, robh, will, joro, helgaas
  Cc: robh+dt, gregkh, iommu, linux-kernel, linux-pci, xieyingtai,
	linux-arm-kernel

On 2021-09-01 09:59, Marek Szyprowski wrote:
> On 21.05.2021 05:03, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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 <wangxingang5@huawei.com>
> 
> 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
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-01  9:58       ` Robin Murphy
  0 siblings, 0 replies; 34+ messages in thread
From: Robin Murphy @ 2021-09-01  9:58 UTC (permalink / raw)
  To: Marek Szyprowski, Wang Xingang, robh, will, joro, helgaas
  Cc: xieyingtai, gregkh, linux-kernel, iommu, robh+dt, linux-pci,
	linux-arm-kernel

On 2021-09-01 09:59, Marek Szyprowski wrote:
> On 21.05.2021 05:03, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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 <wangxingang5@huawei.com>
> 
> 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
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-01  9:58       ` Robin Murphy
  0 siblings, 0 replies; 34+ messages in thread
From: Robin Murphy @ 2021-09-01  9:58 UTC (permalink / raw)
  To: Marek Szyprowski, Wang Xingang, robh, will, joro, helgaas
  Cc: robh+dt, gregkh, iommu, linux-kernel, linux-pci, xieyingtai,
	linux-arm-kernel

On 2021-09-01 09:59, Marek Szyprowski wrote:
> On 21.05.2021 05:03, Wang Xingang wrote:
>> From: Xingang Wang <wangxingang5@huawei.com>
>>
>> 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 <wangxingang5@huawei.com>
> 
> 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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-09-01  9:58       ` Robin Murphy
  (?)
@ 2021-09-02 12:51         ` Anders Roxell
  -1 siblings, 0 replies; 34+ messages in thread
From: Anders Roxell @ 2021-09-02 12:51 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Marek Szyprowski, Wang Xingang, Rob Herring, Will Deacon, joro,
	helgaas, Rob Herring, Greg Kroah-Hartman, iommu,
	Linux Kernel Mailing List, linux-pci, xieyingtai,
	linux-arm-kernel, Linux-Next Mailing List

On Wed, 1 Sept 2021 at 11:58, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-09-01 09:59, Marek Szyprowski wrote:
> > On 21.05.2021 05:03, Wang Xingang wrote:
> >> From: Xingang Wang <wangxingang5@huawei.com>
> >>
> >> 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 <wangxingang5@huawei.com>
> >
> > 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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-02 12:51         ` Anders Roxell
  0 siblings, 0 replies; 34+ messages in thread
From: Anders Roxell @ 2021-09-02 12:51 UTC (permalink / raw)
  To: Robin Murphy
  Cc: xieyingtai, Rob Herring, Wang Xingang, Greg Kroah-Hartman,
	Linux Kernel Mailing List, iommu, Rob Herring, helgaas,
	Linux-Next Mailing List, linux-pci, Will Deacon,
	linux-arm-kernel

On Wed, 1 Sept 2021 at 11:58, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-09-01 09:59, Marek Szyprowski wrote:
> > On 21.05.2021 05:03, Wang Xingang wrote:
> >> From: Xingang Wang <wangxingang5@huawei.com>
> >>
> >> 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 <wangxingang5@huawei.com>
> >
> > 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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-02 12:51         ` Anders Roxell
  0 siblings, 0 replies; 34+ messages in thread
From: Anders Roxell @ 2021-09-02 12:51 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Marek Szyprowski, Wang Xingang, Rob Herring, Will Deacon, joro,
	helgaas, Rob Herring, Greg Kroah-Hartman, iommu,
	Linux Kernel Mailing List, linux-pci, xieyingtai,
	linux-arm-kernel, Linux-Next Mailing List

On Wed, 1 Sept 2021 at 11:58, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-09-01 09:59, Marek Szyprowski wrote:
> > On 21.05.2021 05:03, Wang Xingang wrote:
> >> From: Xingang Wang <wangxingang5@huawei.com>
> >>
> >> 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 <wangxingang5@huawei.com>
> >
> > 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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-09-02 12:51         ` Anders Roxell
  (?)
@ 2021-09-02 13:07           ` Robin Murphy
  -1 siblings, 0 replies; 34+ messages in thread
From: Robin Murphy @ 2021-09-02 13:07 UTC (permalink / raw)
  To: Anders Roxell
  Cc: Marek Szyprowski, Wang Xingang, Rob Herring, Will Deacon, joro,
	helgaas, Rob Herring, Greg Kroah-Hartman, iommu,
	Linux Kernel Mailing List, linux-pci, xieyingtai,
	linux-arm-kernel, Linux-Next Mailing List

On 2021-09-02 13:51, Anders Roxell wrote:
> On Wed, 1 Sept 2021 at 11:58, Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-09-01 09:59, Marek Szyprowski wrote:
>>> On 21.05.2021 05:03, Wang Xingang wrote:
>>>> From: Xingang Wang <wangxingang5@huawei.com>
>>>>
>>>> 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 <wangxingang5@huawei.com>
>>>
>>> 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
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-02 13:07           ` Robin Murphy
  0 siblings, 0 replies; 34+ messages in thread
From: Robin Murphy @ 2021-09-02 13:07 UTC (permalink / raw)
  To: Anders Roxell
  Cc: xieyingtai, Rob Herring, Wang Xingang, Greg Kroah-Hartman,
	Linux Kernel Mailing List, iommu, Rob Herring, helgaas,
	Linux-Next Mailing List, linux-pci, Will Deacon,
	linux-arm-kernel

On 2021-09-02 13:51, Anders Roxell wrote:
> On Wed, 1 Sept 2021 at 11:58, Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-09-01 09:59, Marek Szyprowski wrote:
>>> On 21.05.2021 05:03, Wang Xingang wrote:
>>>> From: Xingang Wang <wangxingang5@huawei.com>
>>>>
>>>> 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 <wangxingang5@huawei.com>
>>>
>>> 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
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-02 13:07           ` Robin Murphy
  0 siblings, 0 replies; 34+ messages in thread
From: Robin Murphy @ 2021-09-02 13:07 UTC (permalink / raw)
  To: Anders Roxell
  Cc: Marek Szyprowski, Wang Xingang, Rob Herring, Will Deacon, joro,
	helgaas, Rob Herring, Greg Kroah-Hartman, iommu,
	Linux Kernel Mailing List, linux-pci, xieyingtai,
	linux-arm-kernel, Linux-Next Mailing List

On 2021-09-02 13:51, Anders Roxell wrote:
> On Wed, 1 Sept 2021 at 11:58, Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-09-01 09:59, Marek Szyprowski wrote:
>>> On 21.05.2021 05:03, Wang Xingang wrote:
>>>> From: Xingang Wang <wangxingang5@huawei.com>
>>>>
>>>> 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 <wangxingang5@huawei.com>
>>>
>>> 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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
  2021-08-20 19:57   ` Bjorn Helgaas
@ 2021-09-02 14:46     ` Bjorn Helgaas
  -1 siblings, 0 replies; 34+ messages in thread
From: Bjorn Helgaas @ 2021-09-02 14:46 UTC (permalink / raw)
  To: Wang Xingang
  Cc: robh, will, joro, robh+dt, gregkh, iommu, linux-kernel,
	linux-pci, xieyingtai, Marek Szyprowski, Anders Roxell,
	Robin Murphy

[+cc Marek, Anders, Robin]

On Fri, Aug 20, 2021 at 02:57:12PM -0500, Bjorn Helgaas wrote:
> On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
> > From: Xingang Wang <wangxingang5@huawei.com>
> > 
> > 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 <wangxingang5@huawei.com>
> 
> Applied to pci/virtualization for v5.15, thanks!

I dropped this for now, until the problems reported by Marek and
Anders get sorted out.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices
@ 2021-09-02 14:46     ` Bjorn Helgaas
  0 siblings, 0 replies; 34+ messages in thread
From: Bjorn Helgaas @ 2021-09-02 14:46 UTC (permalink / raw)
  To: Wang Xingang
  Cc: xieyingtai, robh, Anders Roxell, gregkh, linux-kernel, iommu,
	robh+dt, linux-pci, Robin Murphy, will

[+cc Marek, Anders, Robin]

On Fri, Aug 20, 2021 at 02:57:12PM -0500, Bjorn Helgaas wrote:
> On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang wrote:
> > From: Xingang Wang <wangxingang5@huawei.com>
> > 
> > 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 <wangxingang5@huawei.com>
> 
> Applied to pci/virtualization for v5.15, thanks!

I dropped this for now, until the problems reported by Marek and
Anders get sorted out.

> > ---
> >  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;
> >  
> > -- 
> > 2.19.1
> > 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2021-09-02 14:46 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21  3:03 [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices Wang Xingang
2021-05-21  3:03 ` Wang Xingang
2021-05-21 19:03 ` Rob Herring
2021-05-21 19:03   ` Rob Herring
2021-05-31 11:24   ` Xingang Wang
2021-05-31 11:24     ` Xingang Wang
2021-06-04 15:36 ` Joerg Roedel
2021-06-04 15:36   ` Joerg Roedel
2021-06-07 12:43   ` Xingang Wang
2021-06-07 12:43     ` Xingang Wang
2021-06-04 19:04 ` Bjorn Helgaas
2021-06-04 19:04   ` Bjorn Helgaas
2021-06-07 12:58   ` Xingang Wang
2021-06-07 12:58     ` Xingang Wang
2021-06-08 10:23     ` John Garry
2021-06-08 10:23       ` John Garry
2021-06-28 13:20     ` Xingang Wang
2021-06-28 13:20       ` Xingang Wang
2021-08-20 19:57 ` Bjorn Helgaas
2021-08-20 19:57   ` Bjorn Helgaas
2021-09-02 14:46   ` Bjorn Helgaas
2021-09-02 14:46     ` Bjorn Helgaas
     [not found] ` <CGME20210901085937eucas1p2d02da65cac797706ca3a10b8a2eb8ba2@eucas1p2.samsung.com>
2021-09-01  8:59   ` Marek Szyprowski
2021-09-01  8:59     ` Marek Szyprowski
2021-09-01  8:59     ` Marek Szyprowski
2021-09-01  9:58     ` Robin Murphy
2021-09-01  9:58       ` Robin Murphy
2021-09-01  9:58       ` Robin Murphy
2021-09-02 12:51       ` Anders Roxell
2021-09-02 12:51         ` Anders Roxell
2021-09-02 12:51         ` Anders Roxell
2021-09-02 13:07         ` Robin Murphy
2021-09-02 13:07           ` Robin Murphy
2021-09-02 13:07           ` Robin Murphy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.