* [PATCH] iommu/of: Remove PCI host bridge node check
@ 2017-09-21 10:16 Robin Murphy
2017-09-21 10:20 ` [PATCH v2] " Robin Murphy
0 siblings, 1 reply; 6+ messages in thread
From: Robin Murphy @ 2017-09-21 10:16 UTC (permalink / raw)
To: joro; +Cc: iommu, linux-kernel, jean-philippe.brucker
of_pci_iommu_init() tries to be clever and stop its alias walk at the
device represented by master_np, in case of weird PCI topologies where
the bridge to the IOMMU and the rest of the system is not at the root.
It turns out this is a bit short-sighted, since there are plenty of
other callers of pci_for_each_dma_alias() which would also need the same
behaviour in that situation, and the only platform so far with such a
topology (Cavium ThunderX2) already solves it more generally via a PCI
quirk. As this check is effectively redundant, and returning a boolean
value as an int is a bit broken anyway, let's just get rid of it.
Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/of_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index e60e3dba85a0..6f34dea8aef9 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -160,7 +160,7 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
if (err)
return err;
- return info->np == pdev->bus->dev.of_node;
+ return 0;
}
const struct iommu_ops *of_iommu_configure(struct device *dev,
--
2.13.4.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] iommu/of: Remove PCI host bridge node check
2017-09-21 10:16 [PATCH] iommu/of: Remove PCI host bridge node check Robin Murphy
@ 2017-09-21 10:20 ` Robin Murphy
2017-09-21 11:08 ` Jean-Philippe Brucker
2017-09-22 8:56 ` Joerg Roedel
0 siblings, 2 replies; 6+ messages in thread
From: Robin Murphy @ 2017-09-21 10:20 UTC (permalink / raw)
To: joro; +Cc: iommu, linux-kernel, jean-philippe.brucker
of_pci_iommu_init() tries to be clever and stop its alias walk at the
device represented by master_np, in case of weird PCI topologies where
the bridge to the IOMMU and the rest of the system is not at the root.
It turns out this is a bit short-sighted, since there are plenty of
other callers of pci_for_each_dma_alias() which would also need the same
behaviour in that situation, and the only platform so far with such a
topology (Cavium ThunderX2) already solves it more generally via a PCI
quirk. As this check is effectively redundant, and returning a boolean
value as an int is a bit broken anyway, let's just get rid of it.
Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
Ugh, I'm really failing to spot the obvious today...
drivers/iommu/of_iommu.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index e60e3dba85a0..50947ebb6d17 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -157,10 +157,7 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
err = of_iommu_xlate(info->dev, &iommu_spec);
of_node_put(iommu_spec.np);
- if (err)
- return err;
-
- return info->np == pdev->bus->dev.of_node;
+ return err;
}
const struct iommu_ops *of_iommu_configure(struct device *dev,
--
2.13.4.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu/of: Remove PCI host bridge node check
2017-09-21 10:20 ` [PATCH v2] " Robin Murphy
@ 2017-09-21 11:08 ` Jean-Philippe Brucker
2017-09-22 8:56 ` Joerg Roedel
1 sibling, 0 replies; 6+ messages in thread
From: Jean-Philippe Brucker @ 2017-09-21 11:08 UTC (permalink / raw)
To: Robin Murphy, joro@8bytes.org
Cc: iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org
On 21/09/17 11:20, Robin Murphy wrote:
> of_pci_iommu_init() tries to be clever and stop its alias walk at the
> device represented by master_np, in case of weird PCI topologies where
> the bridge to the IOMMU and the rest of the system is not at the root.
> It turns out this is a bit short-sighted, since there are plenty of
> other callers of pci_for_each_dma_alias() which would also need the same
> behaviour in that situation, and the only platform so far with such a
> topology (Cavium ThunderX2) already solves it more generally via a PCI
> quirk. As this check is effectively redundant, and returning a boolean
> value as an int is a bit broken anyway, let's just get rid of it.
>
> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
This fixes the 4.14-rc1 issue I had with PCI probing on the FastModel
Tested-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>
> Ugh, I'm really failing to spot the obvious today...
>
> drivers/iommu/of_iommu.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index e60e3dba85a0..50947ebb6d17 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -157,10 +157,7 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
>
> err = of_iommu_xlate(info->dev, &iommu_spec);
> of_node_put(iommu_spec.np);
> - if (err)
> - return err;
> -
> - return info->np == pdev->bus->dev.of_node;
> + return err;
> }
>
> const struct iommu_ops *of_iommu_configure(struct device *dev,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu/of: Remove PCI host bridge node check
2017-09-21 10:20 ` [PATCH v2] " Robin Murphy
2017-09-21 11:08 ` Jean-Philippe Brucker
@ 2017-09-22 8:56 ` Joerg Roedel
2017-09-22 9:51 ` Robin Murphy
1 sibling, 1 reply; 6+ messages in thread
From: Joerg Roedel @ 2017-09-22 8:56 UTC (permalink / raw)
To: Robin Murphy; +Cc: iommu, linux-kernel, jean-philippe.brucker
Hey Robin,
On Thu, Sep 21, 2017 at 11:20:58AM +0100, Robin Murphy wrote:
> of_pci_iommu_init() tries to be clever and stop its alias walk at the
> device represented by master_np, in case of weird PCI topologies where
> the bridge to the IOMMU and the rest of the system is not at the root.
> It turns out this is a bit short-sighted, since there are plenty of
> other callers of pci_for_each_dma_alias() which would also need the same
> behaviour in that situation, and the only platform so far with such a
> topology (Cavium ThunderX2) already solves it more generally via a PCI
> quirk. As this check is effectively redundant, and returning a boolean
> value as an int is a bit broken anyway, let's just get rid of it.
>
> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Can you send me a Fixes: tag for this please? No need to resend the
whole patch, I just need the tag.
Thanks,
Joerg
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu/of: Remove PCI host bridge node check
2017-09-22 8:56 ` Joerg Roedel
@ 2017-09-22 9:51 ` Robin Murphy
2017-09-22 10:06 ` Joerg Roedel
0 siblings, 1 reply; 6+ messages in thread
From: Robin Murphy @ 2017-09-22 9:51 UTC (permalink / raw)
To: Joerg Roedel; +Cc: iommu, linux-kernel, jean-philippe.brucker
On 22/09/17 09:56, Joerg Roedel wrote:
> Hey Robin,
>
> On Thu, Sep 21, 2017 at 11:20:58AM +0100, Robin Murphy wrote:
>> of_pci_iommu_init() tries to be clever and stop its alias walk at the
>> device represented by master_np, in case of weird PCI topologies where
>> the bridge to the IOMMU and the rest of the system is not at the root.
>> It turns out this is a bit short-sighted, since there are plenty of
>> other callers of pci_for_each_dma_alias() which would also need the same
>> behaviour in that situation, and the only platform so far with such a
>> topology (Cavium ThunderX2) already solves it more generally via a PCI
>> quirk. As this check is effectively redundant, and returning a boolean
>> value as an int is a bit broken anyway, let's just get rid of it.
>>
>> Reported-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>
> Can you send me a Fixes: tag for this please? No need to resend the
> whole patch, I just need the tag.
Sure, I'd go for:
Fixes: d87beb749281 ("iommu/of: Handle PCI aliases properly")
The check itself originally dates back to b996444cf35e ("iommu/of:
Handle iommu-map property for PCI") but it's really not worth
backporting past the above refactoring - it's only with da4b02750a9f
("iommu/of: Fix of_iommu_configure() for disabled IOMMUs") in 4.14-rc1
that it started to have noticeable ill effects.
Thanks,
Robin.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] iommu/of: Remove PCI host bridge node check
2017-09-22 9:51 ` Robin Murphy
@ 2017-09-22 10:06 ` Joerg Roedel
0 siblings, 0 replies; 6+ messages in thread
From: Joerg Roedel @ 2017-09-22 10:06 UTC (permalink / raw)
To: Robin Murphy; +Cc: iommu, linux-kernel, jean-philippe.brucker
On Fri, Sep 22, 2017 at 10:51:30AM +0100, Robin Murphy wrote:
>
> Fixes: d87beb749281 ("iommu/of: Handle PCI aliases properly")
>
> The check itself originally dates back to b996444cf35e ("iommu/of:
> Handle iommu-map property for PCI") but it's really not worth
> backporting past the above refactoring - it's only with da4b02750a9f
> ("iommu/of: Fix of_iommu_configure() for disabled IOMMUs") in 4.14-rc1
> that it started to have noticeable ill effects.
Thanks, applied to iommu/fixes.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-22 10:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-21 10:16 [PATCH] iommu/of: Remove PCI host bridge node check Robin Murphy
2017-09-21 10:20 ` [PATCH v2] " Robin Murphy
2017-09-21 11:08 ` Jean-Philippe Brucker
2017-09-22 8:56 ` Joerg Roedel
2017-09-22 9:51 ` Robin Murphy
2017-09-22 10:06 ` Joerg Roedel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox