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

[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(-)

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

[Joerg Roedel]

On Fri, Oct 28, 2016 at 05:01:48PM +0100, Will Deacon wrote:
>       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

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

[Will Deacon]

Hi Joerg,

On Thu, Nov 03, 2016 at 09:33:04AM -0600, Joerg Roedel wrote:
> On Fri, Oct 28, 2016 at 05:01:48PM +0100, Will Deacon wrote:
> >       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

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

[Joerg Roedel]

On Thu, Nov 03, 2016 at 04:00:06PM +0000, Will Deacon wrote:
> 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

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

[Robin Murphy]

On 03/11/16 16:14, Joerg Roedel wrote:
> On Thu, Nov 03, 2016 at 04:00:06PM +0000, Will Deacon wrote:
>> 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.

> 
> 
> 	Joerg
> 

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

[Joerg Roedel]

On Thu, Nov 03, 2016 at 04:22:04PM +0000, Robin Murphy wrote:
> 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

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

[Robin Murphy]

On 03/11/16 16:29, Joerg Roedel wrote:
> On Thu, Nov 03, 2016 at 04:22:04PM +0000, Robin Murphy wrote:
>> 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.

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

[Joerg Roedel]

On Thu, Nov 03, 2016 at 04:57:40PM +0000, Robin Murphy wrote:
> 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