iommu/arm-smmu: Fixes for 4.9
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(-)
Quoted 1 line · 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
Hi Joerg,
Quoted 7 lines · Joerg Roedel
Quoted 1 line · 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
Quoted 9 lines · Will Deacon
We're basically doing that already, since the
bus_set_iommucall 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_opswith&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
Quoted 7 lines · Joerg Roedel
Quoted 6 lines · Will Deacon
We're basically doing that already, since the
bus_set_iommucall 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.
Quoted 6 lines
Quoted 3 lines
I suppose we could go and compare
bus->iommu_opswith&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_iommucall 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.
Quoted 4 lines
Joerg
Quoted 5 lines · Robin Murphy
But
bus_set_iommu()is already checking whether another SMMU (in this case) has probed, by virtue ofbus->iommu_opsbeing 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
Quoted 10 lines · Joerg Roedel
Quoted 5 lines · Robin Murphy
But
bus_set_iommu()is already checking whether another SMMU (in this case) has probed, by virtue ofbus->iommu_opsbeing 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 ofbus_set_iommushouldn'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.
Quoted 5 lines · 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