[GIT PULL] iommu/arm-smmu: Fixes for 4.9

[Will Deacon]

Hi Joerg,

Please pull the following pair of fixes from Robin for the ARM SMMU
drivers.

Thanks,

Will

--->8

The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:

Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-joerg/arm-smmu/fixes

for you to fetch changes up to 5a5a057d2ba73ae4990b6dc283465b4886d93993:

iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s (2016-10-26 19:21:38 +0100)

----------------------------------------------------------------
Robin Murphy (2):
iommu/arm-smmu: Work around ARM DMA configuration
iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s

drivers/iommu/arm-smmu-v3.c | 11 ++++-------
drivers/iommu/arm-smmu.c | 10 ++++++++++
2 files changed, 14 insertions(+), 7 deletions(-)

[Joerg Roedel]

Post by Will Deacon
iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s

Hmm, this patch is pretty ugly. Wouldn't it be better to have
hardware-independent init-routine in the arm-smmu-v3 driver that checks
DT whether there is an SMMU at all and if yes, sets the per-bus
iommu-ops?



Joerg

[Will Deacon]

Hi Joerg,

Post by Joerg Roedel

Post by Will Deacon
iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s

Hmm, this patch is pretty ugly. Wouldn't it be better to have
hardware-independent init-routine in the arm-smmu-v3 driver that checks
DT whether there is an SMMU at all and if yes, sets the per-bus
iommu-ops?

We're basically doing that already, since the bus_set_iommu call happens in
the probe routine, which won't run unless an SMMUv3 has been found in the
DT. The issue we're trying to avoid is failing the probe of a second SMMUv3
in the system, because the bus will already have the iommu ops set by the
first SMMUv3 that probed.

I suppose we could go and compare bus->iommu_ops with &arm_smmu_ops, but
given that we can't support different IOMMU types on a single bus, I don't
think we gain anything from that.

Will

[Joerg Roedel]

Post by Will Deacon
We're basically doing that already, since the bus_set_iommu call happens in
the probe routine, which won't run unless an SMMUv3 has been found in the
DT. The issue we're trying to avoid is failing the probe of a second SMMUv3
in the system, because the bus will already have the iommu ops set by the
first SMMUv3 that probed.
I suppose we could go and compare bus->iommu_ops with &arm_smmu_ops, but
given that we can't support different IOMMU types on a single bus, I don't
think we gain anything from that.

Can you instead check whether there is already another smmu probed and
skip the bus_set_iommu call then?


Joerg

[Robin Murphy]

Post by Joerg Roedel

Post by Will Deacon
We're basically doing that already, since the bus_set_iommu call happens in
the probe routine, which won't run unless an SMMUv3 has been found in the
DT. The issue we're trying to avoid is failing the probe of a second SMMUv3
in the system, because the bus will already have the iommu ops set by the
first SMMUv3 that probed.
I suppose we could go and compare bus->iommu_ops with &arm_smmu_ops, but
given that we can't support different IOMMU types on a single bus, I don't
think we gain anything from that.

Can you instead check whether there is already another smmu probed and
skip the bus_set_iommu call then?

But bus_set_iommu() is already checking whether another SMMU (in this
case) has probed, by virtue of bus->iommu_ops being non-NULL, and
returning without doing anything if so. What's the value of adding a
whole bunch more code to effectively duplicate that in a less elegant
manner?

Robin.

Post by Joerg Roedel
Joerg

[Joerg Roedel]

Post by Robin Murphy
But bus_set_iommu() is already checking whether another SMMU (in this
case) has probed, by virtue of bus->iommu_ops being non-NULL, and
returning without doing anything if so. What's the value of adding a
whole bunch more code to effectively duplicate that in a less elegant
manner?

No, bus_set_iommu() checks whether there is _any_ other IOMMU already
registered. This doesn't need to be an smmu. So I think the return value
of bus_set_iommu shouldn't be generally ignored.


Joerg

[Robin Murphy]

Post by Joerg Roedel

Post by Robin Murphy
But bus_set_iommu() is already checking whether another SMMU (in this
case) has probed, by virtue of bus->iommu_ops being non-NULL, and
returning without doing anything if so. What's the value of adding a
whole bunch more code to effectively duplicate that in a less elegant
manner?

No, bus_set_iommu() checks whether there is _any_ other IOMMU already
registered. This doesn't need to be an smmu. So I think the return value
of bus_set_iommu shouldn't be generally ignored.

But if it is someone else's ops, then all that means is that the SMMU
driver isn't going to get notified about devices on that bus, or get
called with them later, so I still don't see where the problem is. If
there are devices on that bus which the SMMU *is* supposed to be
managing, then that system can't be supported with the current API anyway.

Robin.

[Joerg Roedel]

Post by Robin Murphy
But if it is someone else's ops, then all that means is that the SMMU
driver isn't going to get notified about devices on that bus, or get
called with them later, so I still don't see where the problem is. If
there are devices on that bus which the SMMU *is* supposed to be
managing, then that system can't be supported with the current API anyway.

If another IOMMU is already probed and set its iommu_ops, than there is
no point in probing the smmu anyway, no? The probe function should
return the failed probe in that case and not proceed silently.


Joerg