[PATCH v2 2/2] Documentation/kernel-parameters.txt : Remove IA-64 from iommu=pt doc

2016-10-17 Thread Glenn Serre 

Remove IA-64 from iommu=pt doc

There is no sign of either iommu parameter parsing
or use of iommu_pass_through in arch/ia64/kernel/pci-dma.c,
so remove IA-64 from iommu=pt documentation.

This patch depends on patch iommu_default_pt.patch.

Signed-off-by: Glenn Serre 
---
 Documentation/kernel-parameters.txt |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1747,7 +1747,7 @@
nomerge
forcesac
soft
-   pt  [x86, IA-64]
+   pt  [x86]
nopt[x86]
nobypass[PPC/POWERNV]
Disable IOMMU bypass, using IOMMU for PCI devices.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 1/2] arch/x86: Add CONFIG_IOMMU_DEFAULT_PT for iommu=pt

2016-10-17 Thread Glenn Serre 

Add CONFIG_IOMMU_DEFAULT_PT for iommu=pt

Allow default to iommu=pt without adding to command line.
Add iommu=nopt to turn off default.

The reason we use iommu=pt is to support using the DPDK igb_uio driver,
which we need to do on servers that have an iommu but lack ACS.

Signed-off-by: Glenn Serre 
---
 Documentation/kernel-parameters.txt | 3 ++-
 arch/x86/Kconfig| 7 +++
 arch/x86/kernel/pci-dma.c   | 6 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1748,6 +1748,7 @@
forcesac
soft
pt  [x86, IA-64]
+   nopt[x86]
nobypass[PPC/POWERNV]
Disable IOMMU bypass, using IOMMU for PCI devices.

--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -882,6 +882,13 @@
def_bool y
depends on CALGARY_IOMMU || GART_IOMMU || SWIOTLB || AMD_IOMMU

+config IOMMU_DEFAULT_PT
+   def_bool n
+   prompt "Set iommu=pt by default"
+   help
+ Selecting this option will set iommu passthrough (iommu=pt)
+ by default.  Use iommu=nopt to override.
+
 config MAXSMP
bool "Enable Maximum number of SMP Processors and NUMA Nodes"
depends on X86_64 && SMP && DEBUG_KERNEL
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -43,7 +43,7 @@
  * useful if a user wants to use an IOMMU only for KVM device assignment to
  * guests and not for driver dma translation.
  */
-int iommu_pass_through __read_mostly;
+int iommu_pass_through __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_PT);

 extern struct iommu_table_entry __iommu_table[], __iommu_table_end[];

@@ -196,6 +196,8 @@
 #endif
if (!strncmp(p, "pt", 2))
iommu_pass_through = 1;
+   if (!strncmp(p, "nopt", 4))
+   iommu_pass_through = 0;

gart_parse_options(p);


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

Re: [PATCH 1/2] iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s

2016-10-17 Thread Lorenzo Pieralisi 
On Mon, Oct 17, 2016 at 03:19:46PM +0100, Robin Murphy wrote:
> Hi Lorenzo,
> 
> On 17/10/16 14:21, Lorenzo Pieralisi wrote:
> > On Mon, Oct 17, 2016 at 12:06:20PM +0100, Robin Murphy wrote:
> >> We now delay installing our per-bus iommu_ops until we know an SMMU has
> >> successfully probed, as they don't serve much purpose beforehand, and
> >> doing so also avoids fights between multiple IOMMU drivers in a single
> >> kernel. However, the upshot of passing the return value of bus_set_iommu()
> >> back from our probe function is that if there happens to be more than
> >> one SMMUv3 device in a system, the second and subsequent probes will
> >> wind up returning -EBUSY to the driver core and getting torn down again.
> >>
> >> There are essentially 3 cases in which bus_set_iommu() returns nonzero:
> >> 1. The bus already has iommu_ops installed
> >> 2. One of the add_device callbacks from the initial notifier failed
> >> 3. Allocating or installing the notifier itself failed
> >>
> >> The first two are down to devices other than the SMMU in question, so
> >> shouldn't abort an otherwise-successful SMMU probe, whilst the third is
> >> indicative of the kind of catastrophic system failure which isn't going
> >> to get much further anyway. Consequently, there is little harm in
> >> ignoring the return value either way.
> >>
> >> CC: Lorenzo Pieralisi 
> >> Signed-off-by: Robin Murphy 
> >> ---
> >>  drivers/iommu/arm-smmu-v3.c | 11 ---
> >>  1 file changed, 4 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> >> index 15c01c3cd540..74fbef384deb 100644
> >> --- a/drivers/iommu/arm-smmu-v3.c
> >> +++ b/drivers/iommu/arm-smmu-v3.c
> >> @@ -2637,16 +2637,13 @@ static int arm_smmu_device_dt_probe(struct 
> >> platform_device *pdev)
> >>of_iommu_set_ops(dev->of_node, _smmu_ops);
> >>  #ifdef CONFIG_PCI
> >>pci_request_acs();
> >> -  ret = bus_set_iommu(_bus_type, _smmu_ops);
> >> -  if (ret)
> >> -  return ret;
> >> +  bus_set_iommu(_bus_type, _smmu_ops);
> >>  #endif
> >>  #ifdef CONFIG_ARM_AMBA
> >> -  ret = bus_set_iommu(_bustype, _smmu_ops);
> >> -  if (ret)
> >> -  return ret;
> >> +  bus_set_iommu(_bustype, _smmu_ops);
> >>  #endif
> >> -  return bus_set_iommu(_bus_type, _smmu_ops);
> >> +  bus_set_iommu(_bus_type, _smmu_ops);
> >> +  return 0;
> > 
> > Nit: I do not see why you would not take the same approach as
> > the ARM SMMUv1/v2, namely checking if ops are already set and
> > skip the call if that's the case.
> 
> Well, I'd say it really goes the other way around - since the very first
> thing bus_set_iommu() does is check if ops are present, and return if
> so, and the v2 driver already doesn't care about that return value,
> there's not really any need for it to duplicate the check either. I
> didn't change it at the time to avoid cluttering the gigantic rework any
> further, but I could spin a cleanup patch if you like.

No worries, it was to understand if there was a reason to keep
the code different and after another look I agree with what
you are saying (by checking if ops are present you could eg
avoid calling pci_request_acs() every probe but that's a detail).

Thanks for fixing it !
Lorenzo

> > Anyway:
> > 
> > Acked-by: Lorenzo Pieralisi 
> 
> Thanks!
> 
> Robin.
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu: fix double spin_lock_irqsave on `device_domain_lock'

2016-10-17 Thread Iago Abal 
From: Iago Abal 

The EBA code analyzer (https://github.com/models-team/eba) reported
the following double lock:

1. In function `disable_dmar_iommu' defined at 1706;
2. the lock `device_domain_lock' is first taken in line 1714:

   // FIRST
   spin_lock_irqsave(_domain_lock, flags);

3. enter the `list_for_each_entry_safe' loop at 1715;
4. call function `dmar_remove_one_dev_info' (defined at 4851) in line 1726;
5. finally, the lock is taken a second time in line 4857:

   // SECOND: DOUBLE LOCK !!!
   spin_lock_irqsave(_domain_lock, flags);

In addition, within that same loop, there is also a call to `domain_exit', which
calls to `domain_remove_dev_info', which also spin_lock on `device_domain_lock'.

I fixed the potential deadlock by releasing the `device_domain_lock' during the
execution of the loop body. This seems to respect the locking assumptions made
by the rest of the code: both `dmar_remove_one_dev_info' and `domain_exit' will
(directly or indiretly) take that look, so they should not be called with it 
held.
Function `domain_type_is_vm_or_si' just checks `domain->flags' and there seem
to be no concurrent writes to this field.

Signed-off-by: Iago Abal 
---
 drivers/iommu/intel-iommu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index a4407ea..05796a8 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1721,12 +1721,16 @@ static void disable_dmar_iommu(struct intel_iommu 
*iommu)
if (!info->dev || !info->domain)
continue;
 
+   spin_unlock_irqrestore(_domain_lock, flags);
+
domain = info->domain;
 
dmar_remove_one_dev_info(domain, info->dev);
 
if (!domain_type_is_vm_or_si(domain))
domain_exit(domain);
+
+   spin_lock_irqsave(_domain_lock, flags);
}
spin_unlock_irqrestore(_domain_lock, flags);
 
-- 
1.9.1

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

Re: [PATCH v14 00/16] KVM PCIe/MSI passthrough on ARM/ARM64

2016-10-17 Thread Punit Agrawal 
Auger Eric  writes:

> Hi Punit,
>
> On 14/10/2016 13:24, Punit Agrawal wrote:
>> Hi Eric,
>> 
>> I am a bit late in joining, but I've tried to familiarise
>> myself with earlier discussions on the series.
>> 
>> Eric Auger  writes:
>> 
>>> This is the second respin on top of Robin's series [1], addressing Alex' 
>>> comments.
>>>
>>> Major changes are:
>>> - MSI-doorbell API now is moved to DMA IOMMU API following Alex suggestion
>>>   to put all API pieces at the same place (so eventually in the IOMMU
>>>   subsystem)
>> 
>> IMHO, this is headed in the opposite direction, i.e., away from the
>> owner of the information - the doorbells are the property of the MSI
>> controller. The MSI controllers know the location, size and interrupt
>> remapping capability as well. On the consumer side, VFIO needs access to
>> the doorbells to allow userspace to carve out a region in the IOVA.
>> 
>> I quite liked what you had in v13, though I think you can go further
>> though. Instead of adding new doorbell API [un]registration calls, how
>> about adding a callback to the irq_domain_ops? The callback will be
>> populated for irqdomains registered by MSI controllers.
>
> Thank you for jumping into that thread. Any help/feedback is greatly
> appreciated.
>
> Regarding your suggestion, the irq_domain looks dedicated to the
> translation between linux irq and HW irq. I tend to think adding an ops
> to retrieve the MSI doorbell info at that level looks far from the
> original goal of the infrastructure. Obviously the fact there is a list
> of such domain is tempting but I preferred to add a separate struct and
> separate list.

Guilty as charged - the suggestion was definitely borne out of
convenience. I originally looked at adding this functionality to the MSI
domains but couldn't find an obvious way to enumerate all of them.

Also, the way the domain hierarchy is setup for MSI controllers, the
actual doorbell is not available to the MSI domains.

>
> In the v14 release I moved the "doorbell API" in the dma-iommu API since
> Alex recommended to offer a unified API where all pieces would be at the
> same place.
>
> Anyway I will follow the guidance of maintainers.

Ok, makes sense.

>
>
>> 
>> From VFIO, we can calculate the required aperture reservation by
>> iterating over the irqdomains (something like irq_domain_for_each). The
>> same callback can also provide information about support for interrupt
>> remapping.
>> 
>> For systems where there are no separate MSI controllers, i.e., the IOMMU
>> has a fixed reservation, no MSI callbacks will be populated - which
>> tells userspace that no separate MSI reservation is required. IIUC, this
>> was one of Alex' concerns on the prior version.
>
> I'am working on a separate series to report to the user-space the usable
> IOVA range(s).

Is that the alternative to reporting the aperture size that needs to be
reserved?

>
> Thanks
>
> Eric
>> 
>> Thoughts, opinions?
>> 
>> Punit
>> 
>>> - new iommu_domain_msi_resv struct and accessor through DOMAIN_ATTR_MSI_RESV
>>>   domain with mirror VFIO capability
>>> - more robustness I think in the VFIO layer
>>> - added "iommu/iova: fix __alloc_and_insert_iova_range" since with the 
>>> current
>>>   code I failed allocating an IOVA page in a single page domain with upper 
>>> part
>>>   reserved
>>>
>>> IOVA range exclusion will be handled in a separate series
>>>
>>> The priority really is to discuss and freeze the API and especially the MSI
>>> doorbell's handling. Do we agree to put that in DMA IOMMU?
>>>
>>> Note: the size computation does not take into account possible page overlaps
>>> between doorbells but it would add quite a lot of complexity i think.
>>>
>>> Tested on AMD Overdrive (single GICv2m frame) with I350 VF assignment.
>>>
>>> dependency:
>>> the series depends on Robin's generic-v7 branch:
>>> [1] [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
>>> http://www.spinics.net/lists/arm-kernel/msg531110.html
>>>
>>> Best Regards
>>>
>>> Eric
>>>
>>> Git: complete series available at
>>> https://github.com/eauger/linux/tree/generic-v7-pcie-passthru-v14
>>>
>>> the above branch includes a temporary patch to work around a ThunderX pci
>>> bus reset crash (which I think unrelated to this series):
>>> "vfio: pci: HACK! workaround thunderx pci_try_reset_bus crash"
>>> Do not take this one for other platforms.
>>>
>>>
>>> Eric Auger (15):
>>>   iommu/iova: fix __alloc_and_insert_iova_range
>>>   iommu: Introduce DOMAIN_ATTR_MSI_RESV
>>>   iommu/dma: MSI doorbell alloc/free
>>>   iommu/dma: Introduce iommu_calc_msi_resv
>>>   iommu/arm-smmu: Implement domain_get_attr for DOMAIN_ATTR_MSI_RESV
>>>   irqchip/gic-v2m: Register the MSI doorbell
>>>   irqchip/gicv3-its: Register the MSI doorbell
>>>   vfio: Introduce a vfio_dma type field
>>>   vfio/type1: vfio_find_dma accepting a type argument
>>>   vfio/type1: Implement recursive vfio_find_dma_from_node
>>>   vfio/type1:

Re: [PATCH v14 04/16] iommu/dma: MSI doorbell alloc/free

2016-10-17 Thread Auger Eric 
Hi Punit,

On 14/10/2016 13:25, Punit Agrawal wrote:
> Hi Eric,
> 
> One query and a comment below.
> 
> Eric Auger  writes:
> 
>> We introduce the capability to (un)register MSI doorbells.
>>
>> A doorbell region is characterized by its physical address base, size,
>> and whether it its safe (ie. it implements IRQ remapping). A doorbell
>> can be per-cpu or global. We currently only care about global doorbells.
>>
>> A function returns whether all registered doorbells are safe.
>>
>> MSI controllers likely to work along with IOMMU that translate MSI
>> transaction must register their doorbells to allow device assignment
>> with MSI support.  Otherwise the MSI transactions will cause IOMMU faults.
>>
>> Signed-off-by: Eric Auger 
>>
>> ---
>>
>> v13 -> v14:
>> - previously in msi-doorbell.h/c
>> ---
>>  drivers/iommu/dma-iommu.c | 75 
>> +++
>>  include/linux/dma-iommu.h | 41 ++
>>  2 files changed, 116 insertions(+)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index d45f9a0..d8a7d86 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -43,6 +43,38 @@ struct iommu_dma_cookie {
>>  spinlock_t  msi_lock;
>>  };
>>  
>> +/**
>> + * struct iommu_msi_doorbell_info - MSI doorbell region descriptor
>> + * @percpu_doorbells: per cpu doorbell base address
>> + * @global_doorbell: base address of the doorbell
>> + * @doorbell_is_percpu: is the doorbell per cpu or global?
>> + * @safe: true if irq remapping is implemented
>> + * @size: size of the doorbell
>> + */
>> +struct iommu_msi_doorbell_info {
>> +union {
>> +phys_addr_t __percpu*percpu_doorbells;
> 
> Out of curiosity, have you come across systems that have per-cpu
> doorbells? I couldn't find a system that'd help solidify my
> understanding on it's usage.

This came out after a discussion With Marc. However at the moment I am
not aware of any MSI controller featuring per-cpu doorbell. Not sure
whether it stays relevant to keep this notion at that stage.

> 
>> +phys_addr_t global_doorbell;
>> +};
>> +booldoorbell_is_percpu;
>> +boolsafe;
> 
> Although you've got the comment above, 'safe' doesn't quite convey it's
> purpose. Can this be renamed to something more descriptive -
> 'intr_remapping' or 'intr_isolation' perhaps?

Yes definitively

Thanks

Eric
> 
> Thanks,
> Punit
> 
> 
> [...]
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v14 00/16] KVM PCIe/MSI passthrough on ARM/ARM64

2016-10-17 Thread Auger Eric 
Hi Punit,

On 14/10/2016 13:24, Punit Agrawal wrote:
> Hi Eric,
> 
> I am a bit late in joining, but I've tried to familiarise
> myself with earlier discussions on the series.
> 
> Eric Auger  writes:
> 
>> This is the second respin on top of Robin's series [1], addressing Alex' 
>> comments.
>>
>> Major changes are:
>> - MSI-doorbell API now is moved to DMA IOMMU API following Alex suggestion
>>   to put all API pieces at the same place (so eventually in the IOMMU
>>   subsystem)
> 
> IMHO, this is headed in the opposite direction, i.e., away from the
> owner of the information - the doorbells are the property of the MSI
> controller. The MSI controllers know the location, size and interrupt
> remapping capability as well. On the consumer side, VFIO needs access to
> the doorbells to allow userspace to carve out a region in the IOVA.
> 
> I quite liked what you had in v13, though I think you can go further
> though. Instead of adding new doorbell API [un]registration calls, how
> about adding a callback to the irq_domain_ops? The callback will be
> populated for irqdomains registered by MSI controllers.

Thank you for jumping into that thread. Any help/feedback is greatly
appreciated.

Regarding your suggestion, the irq_domain looks dedicated to the
translation between linux irq and HW irq. I tend to think adding an ops
to retrieve the MSI doorbell info at that level looks far from the
original goal of the infrastructure. Obviously the fact there is a list
of such domain is tempting but I preferred to add a separate struct and
separate list.

In the v14 release I moved the "doorbell API" in the dma-iommu API since
Alex recommended to offer a unified API where all pieces would be at the
same place.

Anyway I will follow the guidance of maintainers.


> 
> From VFIO, we can calculate the required aperture reservation by
> iterating over the irqdomains (something like irq_domain_for_each). The
> same callback can also provide information about support for interrupt
> remapping.
> 
> For systems where there are no separate MSI controllers, i.e., the IOMMU
> has a fixed reservation, no MSI callbacks will be populated - which
> tells userspace that no separate MSI reservation is required. IIUC, this
> was one of Alex' concerns on the prior version.

I'am working on a separate series to report to the user-space the usable
IOVA range(s).

Thanks

Eric
> 
> Thoughts, opinions?
> 
> Punit
> 
>> - new iommu_domain_msi_resv struct and accessor through DOMAIN_ATTR_MSI_RESV
>>   domain with mirror VFIO capability
>> - more robustness I think in the VFIO layer
>> - added "iommu/iova: fix __alloc_and_insert_iova_range" since with the 
>> current
>>   code I failed allocating an IOVA page in a single page domain with upper 
>> part
>>   reserved
>>
>> IOVA range exclusion will be handled in a separate series
>>
>> The priority really is to discuss and freeze the API and especially the MSI
>> doorbell's handling. Do we agree to put that in DMA IOMMU?
>>
>> Note: the size computation does not take into account possible page overlaps
>> between doorbells but it would add quite a lot of complexity i think.
>>
>> Tested on AMD Overdrive (single GICv2m frame) with I350 VF assignment.
>>
>> dependency:
>> the series depends on Robin's generic-v7 branch:
>> [1] [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
>> http://www.spinics.net/lists/arm-kernel/msg531110.html
>>
>> Best Regards
>>
>> Eric
>>
>> Git: complete series available at
>> https://github.com/eauger/linux/tree/generic-v7-pcie-passthru-v14
>>
>> the above branch includes a temporary patch to work around a ThunderX pci
>> bus reset crash (which I think unrelated to this series):
>> "vfio: pci: HACK! workaround thunderx pci_try_reset_bus crash"
>> Do not take this one for other platforms.
>>
>>
>> Eric Auger (15):
>>   iommu/iova: fix __alloc_and_insert_iova_range
>>   iommu: Introduce DOMAIN_ATTR_MSI_RESV
>>   iommu/dma: MSI doorbell alloc/free
>>   iommu/dma: Introduce iommu_calc_msi_resv
>>   iommu/arm-smmu: Implement domain_get_attr for DOMAIN_ATTR_MSI_RESV
>>   irqchip/gic-v2m: Register the MSI doorbell
>>   irqchip/gicv3-its: Register the MSI doorbell
>>   vfio: Introduce a vfio_dma type field
>>   vfio/type1: vfio_find_dma accepting a type argument
>>   vfio/type1: Implement recursive vfio_find_dma_from_node
>>   vfio/type1: Handle unmap/unpin and replay for VFIO_IOVA_RESERVED slots
>>   vfio: Allow reserved msi iova registration
>>   vfio/type1: Check doorbell safety
>>   iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP
>>   vfio/type1: Introduce MSI_RESV capability
>>
>> Robin Murphy (1):
>>   iommu/dma: Allow MSI-only cookies
>>
>>  drivers/iommu/Kconfig|   4 +-
>>  drivers/iommu/arm-smmu-v3.c  |  10 +-
>>  drivers/iommu/arm-smmu.c |  10 +-
>>  drivers/iommu/dma-iommu.c| 184 ++
>>  drivers/iommu/iova.c |   2 +-
>>  drivers/irqchip/irq-gic-v2m.c

Re: [PATCH 1/2] iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s

2016-10-17 Thread Robin Murphy 
Hi Lorenzo,

On 17/10/16 14:21, Lorenzo Pieralisi wrote:
> On Mon, Oct 17, 2016 at 12:06:20PM +0100, Robin Murphy wrote:
>> We now delay installing our per-bus iommu_ops until we know an SMMU has
>> successfully probed, as they don't serve much purpose beforehand, and
>> doing so also avoids fights between multiple IOMMU drivers in a single
>> kernel. However, the upshot of passing the return value of bus_set_iommu()
>> back from our probe function is that if there happens to be more than
>> one SMMUv3 device in a system, the second and subsequent probes will
>> wind up returning -EBUSY to the driver core and getting torn down again.
>>
>> There are essentially 3 cases in which bus_set_iommu() returns nonzero:
>> 1. The bus already has iommu_ops installed
>> 2. One of the add_device callbacks from the initial notifier failed
>> 3. Allocating or installing the notifier itself failed
>>
>> The first two are down to devices other than the SMMU in question, so
>> shouldn't abort an otherwise-successful SMMU probe, whilst the third is
>> indicative of the kind of catastrophic system failure which isn't going
>> to get much further anyway. Consequently, there is little harm in
>> ignoring the return value either way.
>>
>> CC: Lorenzo Pieralisi 
>> Signed-off-by: Robin Murphy 
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 11 ---
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 15c01c3cd540..74fbef384deb 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -2637,16 +2637,13 @@ static int arm_smmu_device_dt_probe(struct 
>> platform_device *pdev)
>>  of_iommu_set_ops(dev->of_node, _smmu_ops);
>>  #ifdef CONFIG_PCI
>>  pci_request_acs();
>> -ret = bus_set_iommu(_bus_type, _smmu_ops);
>> -if (ret)
>> -return ret;
>> +bus_set_iommu(_bus_type, _smmu_ops);
>>  #endif
>>  #ifdef CONFIG_ARM_AMBA
>> -ret = bus_set_iommu(_bustype, _smmu_ops);
>> -if (ret)
>> -return ret;
>> +bus_set_iommu(_bustype, _smmu_ops);
>>  #endif
>> -return bus_set_iommu(_bus_type, _smmu_ops);
>> +bus_set_iommu(_bus_type, _smmu_ops);
>> +return 0;
> 
> Nit: I do not see why you would not take the same approach as
> the ARM SMMUv1/v2, namely checking if ops are already set and
> skip the call if that's the case.

Well, I'd say it really goes the other way around - since the very first
thing bus_set_iommu() does is check if ops are present, and return if
so, and the v2 driver already doesn't care about that return value,
there's not really any need for it to duplicate the check either. I
didn't change it at the time to avoid cluttering the gigantic rework any
further, but I could spin a cleanup patch if you like.

> Anyway:
> 
> Acked-by: Lorenzo Pieralisi 

Thanks!

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

RE: [PATCH 1/2] iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s

2016-10-17 Thread Sricharan 
Hi,

>-Original Message-
>From: linux-arm-kernel [mailto:linux-arm-kernel-boun...@lists.infradead.org] 
>On Behalf Of Robin Murphy
>Sent: Monday, October 17, 2016 4:36 PM
>To: will.dea...@arm.com; j...@8bytes.org
>Cc: iommu@lists.linux-foundation.org; Lorenzo Pieralisi 
>; linux-arm-ker...@lists.infradead.org
>Subject: [PATCH 1/2] iommu/arm-smmu: Don't inadvertently reject multiple 
>SMMUv3s
>
>We now delay installing our per-bus iommu_ops until we know an SMMU has
>successfully probed, as they don't serve much purpose beforehand, and
>doing so also avoids fights between multiple IOMMU drivers in a single
>kernel. However, the upshot of passing the return value of bus_set_iommu()
>back from our probe function is that if there happens to be more than
>one SMMUv3 device in a system, the second and subsequent probes will
>wind up returning -EBUSY to the driver core and getting torn down again.
>
>There are essentially 3 cases in which bus_set_iommu() returns nonzero:
>1. The bus already has iommu_ops installed
>2. One of the add_device callbacks from the initial notifier failed
>3. Allocating or installing the notifier itself failed
>
>The first two are down to devices other than the SMMU in question, so
>shouldn't abort an otherwise-successful SMMU probe, whilst the third is
>indicative of the kind of catastrophic system failure which isn't going
>to get much further anyway. Consequently, there is little harm in
>ignoring the return value either way.
>
>CC: Lorenzo Pieralisi 
>Signed-off-by: Robin Murphy 
>---
> drivers/iommu/arm-smmu-v3.c | 11 ---
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>index 15c01c3cd540..74fbef384deb 100644
>--- a/drivers/iommu/arm-smmu-v3.c
>+++ b/drivers/iommu/arm-smmu-v3.c
>@@ -2637,16 +2637,13 @@ static int arm_smmu_device_dt_probe(struct 
>platform_device *pdev)
>   of_iommu_set_ops(dev->of_node, _smmu_ops);
> #ifdef CONFIG_PCI
>   pci_request_acs();
>-  ret = bus_set_iommu(_bus_type, _smmu_ops);
>-  if (ret)
>-  return ret;
>+  bus_set_iommu(_bus_type, _smmu_ops);
> #endif
> #ifdef CONFIG_ARM_AMBA
>-  ret = bus_set_iommu(_bustype, _smmu_ops);
>-  if (ret)
>-  return ret;
>+  bus_set_iommu(_bustype, _smmu_ops);
> #endif
>-  return bus_set_iommu(_bus_type, _smmu_ops);
>+  bus_set_iommu(_bus_type, _smmu_ops);
>+  return 0;
reviewed-by: sricha...@codeaurora.org

Regards,
 Sricharan
  

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

Re: [PATCH 1/2] iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s

2016-10-17 Thread Lorenzo Pieralisi 
On Mon, Oct 17, 2016 at 12:06:20PM +0100, Robin Murphy wrote:
> We now delay installing our per-bus iommu_ops until we know an SMMU has
> successfully probed, as they don't serve much purpose beforehand, and
> doing so also avoids fights between multiple IOMMU drivers in a single
> kernel. However, the upshot of passing the return value of bus_set_iommu()
> back from our probe function is that if there happens to be more than
> one SMMUv3 device in a system, the second and subsequent probes will
> wind up returning -EBUSY to the driver core and getting torn down again.
> 
> There are essentially 3 cases in which bus_set_iommu() returns nonzero:
> 1. The bus already has iommu_ops installed
> 2. One of the add_device callbacks from the initial notifier failed
> 3. Allocating or installing the notifier itself failed
> 
> The first two are down to devices other than the SMMU in question, so
> shouldn't abort an otherwise-successful SMMU probe, whilst the third is
> indicative of the kind of catastrophic system failure which isn't going
> to get much further anyway. Consequently, there is little harm in
> ignoring the return value either way.
> 
> CC: Lorenzo Pieralisi 
> Signed-off-by: Robin Murphy 
> ---
>  drivers/iommu/arm-smmu-v3.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 15c01c3cd540..74fbef384deb 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2637,16 +2637,13 @@ static int arm_smmu_device_dt_probe(struct 
> platform_device *pdev)
>   of_iommu_set_ops(dev->of_node, _smmu_ops);
>  #ifdef CONFIG_PCI
>   pci_request_acs();
> - ret = bus_set_iommu(_bus_type, _smmu_ops);
> - if (ret)
> - return ret;
> + bus_set_iommu(_bus_type, _smmu_ops);
>  #endif
>  #ifdef CONFIG_ARM_AMBA
> - ret = bus_set_iommu(_bustype, _smmu_ops);
> - if (ret)
> - return ret;
> + bus_set_iommu(_bustype, _smmu_ops);
>  #endif
> - return bus_set_iommu(_bus_type, _smmu_ops);
> + bus_set_iommu(_bus_type, _smmu_ops);
> + return 0;

Nit: I do not see why you would not take the same approach as
the ARM SMMUv1/v2, namely checking if ops are already set and
skip the call if that's the case.

Anyway:

Acked-by: Lorenzo Pieralisi 

>  }
>  
>  static int arm_smmu_device_remove(struct platform_device *pdev)
> -- 
> 1.9.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/2] arm64: Wire up iommu_dma_{map, unmap}_resource()

2016-10-17 Thread Robin Murphy 
With no coherency to worry about, just plug'em straight in.

CC: Catalin Marinas 
CC: Will Deacon 
Signed-off-by: Robin Murphy 
---
 arch/arm64/mm/dma-mapping.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 3f74d0d98de6..5cd0a383b14b 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -796,6 +796,8 @@ static void __iommu_unmap_sg_attrs(struct device *dev,
.sync_single_for_device = __iommu_sync_single_for_device,
.sync_sg_for_cpu = __iommu_sync_sg_for_cpu,
.sync_sg_for_device = __iommu_sync_sg_for_device,
+   .map_resource = iommu_dma_map_resource,
+   .unmap_resource = iommu_dma_unmap_resource,
.dma_supported = iommu_dma_supported,
.mapping_error = iommu_dma_mapping_error,
 };
-- 
1.9.1

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

[PATCH 1/2] iommu/dma: Implement dma_{map,unmap}_resource()

2016-10-17 Thread Robin Murphy 
With the new dma_{map,unmap}_resource() functions added to the DMA API
for the benefit of cases like slave DMA, add suitable implementations to
the arsenal of our generic layer. Since cache maintenance should not be
a concern, these can both be standalone versions without the need for
architecture-specific wrappers.

CC: Joerg Roedel 
Signed-off-by: Robin Murphy 
---

Since patch 2 has a build dependency on this one, they should probably
go together through either the arm64 tree or the iommu tree, but I can't
make up my mind which one seems more appropriate...

Robin.

 drivers/iommu/dma-iommu.c | 13 +
 include/linux/dma-iommu.h |  4 
 2 files changed, 17 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab8667e6f2..50acd71915db 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -624,6 +624,19 @@ void iommu_dma_unmap_sg(struct device *dev, struct 
scatterlist *sg, int nents,
__iommu_dma_unmap(iommu_get_domain_for_dev(dev), sg_dma_address(sg));
 }
 
+dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
+   size_t size, enum dma_data_direction dir, unsigned long attrs)
+{
+   return iommu_dma_map_page(dev, phys_to_page(phys), offset_in_page(phys),
+   size, dma_direction_to_prot(dir, false) | IOMMU_MMIO);
+}
+
+void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
+   size_t size, enum dma_data_direction dir, unsigned long attrs)
+{
+   __iommu_dma_unmap(iommu_get_domain_for_dev(dev), handle);
+}
+
 int iommu_dma_supported(struct device *dev, u64 mask)
 {
/*
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 32c589062bd9..7f7e9a7e3839 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -61,6 +61,10 @@ void iommu_dma_unmap_page(struct device *dev, dma_addr_t 
handle, size_t size,
enum dma_data_direction dir, unsigned long attrs);
 void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
enum dma_data_direction dir, unsigned long attrs);
+dma_addr_t iommu_dma_map_resource(struct device *dev, phys_addr_t phys,
+   size_t size, enum dma_data_direction dir, unsigned long attrs);
+void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
+   size_t size, enum dma_data_direction dir, unsigned long attrs);
 int iommu_dma_supported(struct device *dev, u64 mask);
 int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
 
-- 
1.9.1

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

[PATCH] iommu/iova: Extend cached node lookup condition

2016-10-17 Thread Robin Murphy 
When searching for a free IOVA range, we optimise the tree traversal
by starting from the cached32_node, instead of the last node, when
limit_pfn is equal to dma_32bit_pfn. However, if limit_pfn happens to
be smaller, then we'll go ahead and start from the top even though
dma_32bit_pfn is still a more suitable upper bound. Since this is
clearly a silly thing to do, adjust the lookup condition appropriately.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/iova.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index e23001bfcfee..080beca0197d 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -56,7 +56,7 @@ static unsigned long iova_rcache_get(struct iova_domain 
*iovad,
 static struct rb_node *
 __get_cached_rbnode(struct iova_domain *iovad, unsigned long *limit_pfn)
 {
-   if ((*limit_pfn != iovad->dma_32bit_pfn) ||
+   if ((*limit_pfn > iovad->dma_32bit_pfn) ||
(iovad->cached32_node == NULL))
return rb_last(>rbroot);
else {
-- 
1.9.1

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

[PATCH 2/2] iommu/mediatek: Convert M4Uv1 to iommu_fwspec

2016-10-17 Thread Robin Murphy 
Our per-device data consists of the M4U instance and firmware-provided
list of LARB IDs, which is a perfect fit for the generic iommu_fwspec
machinery. Use that directly instead of the custom archdata code - while
we can't rely on the of_xlate() mechanism to initialise things until the
32-bit ARM DMA code learns about groups and default domains, it still
results in a reasonable simplification overall.

CC: Honghui Zhang 
Signed-off-by: Robin Murphy 
---
 drivers/iommu/mtk_iommu.h|  6 ---
 drivers/iommu/mtk_iommu_v1.c | 95 +---
 2 files changed, 36 insertions(+), 65 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 3dab13b4a211..f59609f20270 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -34,12 +34,6 @@ struct mtk_iommu_suspend_reg {
u32 int_main_control;
 };
 
-struct mtk_iommu_client_priv {
-   struct list_headclient;
-   unsigned intmtk_m4u_id;
-   struct device   *m4udev;
-};
-
 struct mtk_iommu_domain;
 
 struct mtk_iommu_data {
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index b8aeb0768483..884c80cb795e 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -204,14 +204,14 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
 static void mtk_iommu_config(struct mtk_iommu_data *data,
 struct device *dev, bool enable)
 {
-   struct mtk_iommu_client_priv *head, *cur, *next;
struct mtk_smi_larb_iommu*larb_mmu;
unsigned int larbid, portid;
+   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+   int i;
 
-   head = dev->archdata.iommu;
-   list_for_each_entry_safe(cur, next, >client, client) {
-   larbid = mt2701_m4u_to_larb(cur->mtk_m4u_id);
-   portid = mt2701_m4u_to_port(cur->mtk_m4u_id);
+   for (i = 0; i < fwspec->num_ids; ++i) {
+   larbid = mt2701_m4u_to_larb(fwspec->ids[i]);
+   portid = mt2701_m4u_to_port(fwspec->ids[i]);
larb_mmu = >smi_imu.larb_imu[larbid];
 
dev_dbg(dev, "%s iommu port: %d\n",
@@ -271,14 +271,12 @@ static int mtk_iommu_attach_device(struct iommu_domain 
*domain,
   struct device *dev)
 {
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
-   struct mtk_iommu_client_priv *priv = dev->archdata.iommu;
-   struct mtk_iommu_data *data;
+   struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
int ret;
 
-   if (!priv)
+   if (!data)
return -ENODEV;
 
-   data = dev_get_drvdata(priv->m4udev);
if (!data->m4u_dom) {
data->m4u_dom = dom;
ret = mtk_iommu_domain_finalise(data);
@@ -295,13 +293,11 @@ static int mtk_iommu_attach_device(struct iommu_domain 
*domain,
 static void mtk_iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
 {
-   struct mtk_iommu_client_priv *priv = dev->archdata.iommu;
-   struct mtk_iommu_data *data;
+   struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
 
-   if (!priv)
+   if (!data)
return;
 
-   data = dev_get_drvdata(priv->m4udev);
mtk_iommu_config(data, dev, false);
 }
 
@@ -366,6 +362,8 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct 
iommu_domain *domain,
return pa;
 }
 
+static struct iommu_ops mtk_iommu_ops;
+
 /*
  * MTK generation one iommu HW only support one iommu domain, and all the 
client
  * sharing the same iova address space.
@@ -373,7 +371,7 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct 
iommu_domain *domain,
 static int mtk_iommu_create_mapping(struct device *dev,
struct of_phandle_args *args)
 {
-   struct mtk_iommu_client_priv *head, *priv, *next;
+   struct mtk_iommu_data *data;
struct platform_device *m4updev;
struct dma_iommu_mapping *mtk_mapping;
struct device *m4udev;
@@ -385,41 +383,37 @@ static int mtk_iommu_create_mapping(struct device *dev,
return -EINVAL;
}
 
-   if (!dev->archdata.iommu) {
+   if (!dev->iommu_fwspec) {
+   ret = iommu_fwspec_init(dev, >np->fwnode, _iommu_ops);
+   if (ret)
+   return ret;
+   } else if (dev->iommu_fwspec->ops != _iommu_ops) {
+   return -EINVAL;
+   }
+
+   if (!dev->iommu_fwspec->iommu_priv) {
/* Get the m4u device */
m4updev = of_find_device_by_node(args->np);
if (WARN_ON(!m4updev))
return -EINVAL;
 
-   head = kzalloc(sizeof(*head), GFP_KERNEL);
-   if (!head)
-   return

[PATCH 1/2] iommu/mediatek: Convert M4Uv2 to iommu_fwspec

2016-10-17 Thread Robin Murphy 
Our per-device data consists of the M4U instance and firmware-provided
list of LARB IDs, which is a perfect fit for the generic iommu_fwspec
machinery. Use that directly as a simpler alternative to the custom
archdata code.

CC: Yong Wu 
Signed-off-by: Robin Murphy 
---

These are fairly mechanical cleanups, so I'm pretty confident, but it
still bears mentioning that they're only compile-tested as I don't have
the relevant hardware.

Robin.

 drivers/iommu/mtk_iommu.c | 75 ---
 1 file changed, 18 insertions(+), 57 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index b12c12d74c33..13bb57995cd3 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -195,14 +195,14 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
 static void mtk_iommu_config(struct mtk_iommu_data *data,
 struct device *dev, bool enable)
 {
-   struct mtk_iommu_client_priv *head, *cur, *next;
struct mtk_smi_larb_iommu*larb_mmu;
unsigned int larbid, portid;
+   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+   int i;
 
-   head = dev->archdata.iommu;
-   list_for_each_entry_safe(cur, next, >client, client) {
-   larbid = MTK_M4U_TO_LARB(cur->mtk_m4u_id);
-   portid = MTK_M4U_TO_PORT(cur->mtk_m4u_id);
+   for (i = 0; i < fwspec->num_ids; ++i) {
+   larbid = MTK_M4U_TO_LARB(fwspec->ids[i]);
+   portid = MTK_M4U_TO_PORT(fwspec->ids[i]);
larb_mmu = >smi_imu.larb_imu[larbid];
 
dev_dbg(dev, "%s iommu port: %d\n",
@@ -282,14 +282,12 @@ static int mtk_iommu_attach_device(struct iommu_domain 
*domain,
   struct device *dev)
 {
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
-   struct mtk_iommu_client_priv *priv = dev->archdata.iommu;
-   struct mtk_iommu_data *data;
+   struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
int ret;
 
-   if (!priv)
+   if (!data)
return -ENODEV;
 
-   data = dev_get_drvdata(priv->m4udev);
if (!data->m4u_dom) {
data->m4u_dom = dom;
ret = mtk_iommu_domain_finalise(data);
@@ -310,13 +308,11 @@ static int mtk_iommu_attach_device(struct iommu_domain 
*domain,
 static void mtk_iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
 {
-   struct mtk_iommu_client_priv *priv = dev->archdata.iommu;
-   struct mtk_iommu_data *data;
+   struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
 
-   if (!priv)
+   if (!data)
return;
 
-   data = dev_get_drvdata(priv->m4udev);
mtk_iommu_config(data, dev, false);
 }
 
@@ -366,8 +362,8 @@ static int mtk_iommu_add_device(struct device *dev)
 {
struct iommu_group *group;
 
-   if (!dev->archdata.iommu) /* Not a iommu client device */
-   return -ENODEV;
+   if (!dev->iommu_fwspec || dev->iommu_fwspec->ops != _iommu_ops)
+   return -ENODEV; /* Not a iommu client device */
 
group = iommu_group_get_for_dev(dev);
if (IS_ERR(group))
@@ -379,33 +375,21 @@ static int mtk_iommu_add_device(struct device *dev)
 
 static void mtk_iommu_remove_device(struct device *dev)
 {
-   struct mtk_iommu_client_priv *head, *cur, *next;
-
-   head = dev->archdata.iommu;
-   if (!head)
+   if (!dev->iommu_fwspec || dev->iommu_fwspec->ops != _iommu_ops)
return;
 
-   list_for_each_entry_safe(cur, next, >client, client) {
-   list_del(>client);
-   kfree(cur);
-   }
-   kfree(head);
-   dev->archdata.iommu = NULL;
-
iommu_group_remove_device(dev);
+   iommu_fwspec_free(dev);
 }
 
 static struct iommu_group *mtk_iommu_device_group(struct device *dev)
 {
-   struct mtk_iommu_data *data;
-   struct mtk_iommu_client_priv *priv;
+   struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
 
-   priv = dev->archdata.iommu;
-   if (!priv)
+   if (!data)
return ERR_PTR(-ENODEV);
 
/* All the client devices are in the same m4u iommu-group */
-   data = dev_get_drvdata(priv->m4udev);
if (!data->m4u_group) {
data->m4u_group = iommu_group_alloc();
if (IS_ERR(data->m4u_group))
@@ -416,7 +400,6 @@ static struct iommu_group *mtk_iommu_device_group(struct 
device *dev)
 
 static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
 {
-   struct mtk_iommu_client_priv *head, *priv, *next;
struct platform_device *m4updev;
 
if (args->args_count != 1) {
@@ -425,38 +408,16 @@ static int mtk_iommu_of_xlate(struct device *dev, struct 
of_phandle_args *args)
return -EINVAL;
}
 
-

[PATCH 1/2] iommu: Favour per-instance IOMMU ops

2016-10-17 Thread Robin Murphy 
On systems with multiple IOMMUs, bus-level granularity can be too
coarse, particularly on the platform "bus", where the hardware may
actually warrant multiple different iommu_ops, which we currently have
no way at all to accommodate. As an initial step towards the necessary
flexibility, allow the device-focused API calls to use the ops provided
by the device-specific IOMMU instance data (when present) in preference
to the bus ops.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/iommu.c | 56 +--
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9a2f1960873b..eab883e6c5a9 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -37,10 +37,6 @@
 static struct kset *iommu_group_kset;
 static DEFINE_IDA(iommu_group_ida);
 
-struct iommu_callback_data {
-   const struct iommu_ops *ops;
-};
-
 struct iommu_group {
struct kobject kobj;
struct kobject *devices_kobj;
@@ -77,7 +73,7 @@ struct iommu_group_attribute iommu_group_attr_##_name =   
\
 #define to_iommu_group(_kobj)  \
container_of(_kobj, struct iommu_group, kobj)
 
-static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
+static struct iommu_domain *__iommu_domain_alloc(const struct iommu_ops *ops,
 unsigned type);
 static int __iommu_attach_device(struct iommu_domain *domain,
 struct device *dev);
@@ -801,6 +797,14 @@ struct iommu_group *pci_device_group(struct device *dev)
return group;
 }
 
+static const struct iommu_ops *dev_iommu_ops(struct device *dev)
+{
+   if (dev->iommu_fwspec && dev->iommu_fwspec->ops)
+   return dev->iommu_fwspec->ops;
+
+   return dev->bus->iommu_ops;
+}
+
 /**
  * iommu_group_get_for_dev - Find or create the IOMMU group for a device
  * @dev: target device
@@ -813,7 +817,7 @@ struct iommu_group *pci_device_group(struct device *dev)
  */
 struct iommu_group *iommu_group_get_for_dev(struct device *dev)
 {
-   const struct iommu_ops *ops = dev->bus->iommu_ops;
+   const struct iommu_ops *ops = dev_iommu_ops(dev);
struct iommu_group *group;
int ret;
 
@@ -834,7 +838,7 @@ struct iommu_group *iommu_group_get_for_dev(struct device 
*dev)
 * IOMMU driver.
 */
if (!group->default_domain) {
-   group->default_domain = __iommu_domain_alloc(dev->bus,
+   group->default_domain = __iommu_domain_alloc(ops,
 IOMMU_DOMAIN_DMA);
if (!group->domain)
group->domain = group->default_domain;
@@ -856,8 +860,7 @@ struct iommu_domain *iommu_group_default_domain(struct 
iommu_group *group)
 
 static int add_iommu_group(struct device *dev, void *data)
 {
-   struct iommu_callback_data *cb = data;
-   const struct iommu_ops *ops = cb->ops;
+   const struct iommu_ops *ops = dev_iommu_ops(dev);
int ret;
 
if (!ops->add_device)
@@ -880,8 +883,7 @@ static int add_iommu_group(struct device *dev, void *data)
 
 static int remove_iommu_group(struct device *dev, void *data)
 {
-   struct iommu_callback_data *cb = data;
-   const struct iommu_ops *ops = cb->ops;
+   const struct iommu_ops *ops = dev_iommu_ops(dev);
 
if (ops->remove_device && dev->iommu_group)
ops->remove_device(dev);
@@ -893,7 +895,7 @@ static int iommu_bus_notifier(struct notifier_block *nb,
  unsigned long action, void *data)
 {
struct device *dev = data;
-   const struct iommu_ops *ops = dev->bus->iommu_ops;
+   const struct iommu_ops *ops = dev_iommu_ops(dev);
struct iommu_group *group;
unsigned long group_action = 0;
 
@@ -946,9 +948,6 @@ static int iommu_bus_init(struct bus_type *bus, const 
struct iommu_ops *ops)
 {
int err;
struct notifier_block *nb;
-   struct iommu_callback_data cb = {
-   .ops = ops,
-   };
 
nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
if (!nb)
@@ -960,7 +959,7 @@ static int iommu_bus_init(struct bus_type *bus, const 
struct iommu_ops *ops)
if (err)
goto out_free;
 
-   err = bus_for_each_dev(bus, NULL, , add_iommu_group);
+   err = bus_for_each_dev(bus, NULL, NULL, add_iommu_group);
if (err)
goto out_err;
 
@@ -969,7 +968,7 @@ static int iommu_bus_init(struct bus_type *bus, const 
struct iommu_ops *ops)
 
 out_err:
/* Clean up */
-   bus_for_each_dev(bus, NULL, , remove_iommu_group);
+   bus_for_each_dev(bus, NULL, NULL, remove_iommu_group);
bus_unregister_notifier(bus, nb);
 
 out_free:
@@ -1047,29 +1046,29 @@ void iommu_set_fault_handler(struct iommu_domain 
*domain,
 }
 EXPORT_SYMBOL_GPL(iommu_set_fault_handler);

[PATCH 2/2] iommu/arm-smmu: Work around ARM DMA configuration

2016-10-17 Thread Robin Murphy 
The 32-bit ARM DMA configuration code predates the IOMMU core's default
domain functionality, and instead relies on allocating its own domains
and attaching any devices using the generic IOMMU binding to them.
Unfortunately, it does this relatively early on in the creation of the
device, before we've seen our add_device callback, which leads us to
attempt to operate on a half-configured master.

To avoid a crash, check for this situation on attach, but refuse to
play, as there's nothing we can do. This at least allows VFIO to keep
working for people who update their 32-bit DTs to the generic binding,
albeit with a few (innocuous) warnings from the DMA layer on boot.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/arm-smmu.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index c841eb7a1a74..3af7f8f62d0a 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1228,6 +1228,16 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
return -ENXIO;
}
 
+   /*
+* FIXME: The arch/arm DMA API code tries to attach devices to its own
+* domains between of_xlate() and add_device() - we have no way to cope
+* with that, so until ARM gets converted to rely on groups and default
+* domains, just say no (but more politely than by dereferencing NULL).
+* This should be at least a WARN_ON once that's sorted.
+*/
+   if (!fwspec->iommu_priv)
+   return -ENODEV;
+
smmu = fwspec_smmu(fwspec);
/* Ensure that the domain is finalised */
ret = arm_smmu_init_domain_context(domain, smmu);
-- 
1.9.1

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

[PATCH 1/2] iommu/arm-smmu: Don't inadvertently reject multiple SMMUv3s

2016-10-17 Thread Robin Murphy 
We now delay installing our per-bus iommu_ops until we know an SMMU has
successfully probed, as they don't serve much purpose beforehand, and
doing so also avoids fights between multiple IOMMU drivers in a single
kernel. However, the upshot of passing the return value of bus_set_iommu()
back from our probe function is that if there happens to be more than
one SMMUv3 device in a system, the second and subsequent probes will
wind up returning -EBUSY to the driver core and getting torn down again.

There are essentially 3 cases in which bus_set_iommu() returns nonzero:
1. The bus already has iommu_ops installed
2. One of the add_device callbacks from the initial notifier failed
3. Allocating or installing the notifier itself failed

The first two are down to devices other than the SMMU in question, so
shouldn't abort an otherwise-successful SMMU probe, whilst the third is
indicative of the kind of catastrophic system failure which isn't going
to get much further anyway. Consequently, there is little harm in
ignoring the return value either way.

CC: Lorenzo Pieralisi 
Signed-off-by: Robin Murphy 
---
 drivers/iommu/arm-smmu-v3.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 15c01c3cd540..74fbef384deb 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2637,16 +2637,13 @@ static int arm_smmu_device_dt_probe(struct 
platform_device *pdev)
of_iommu_set_ops(dev->of_node, _smmu_ops);
 #ifdef CONFIG_PCI
pci_request_acs();
-   ret = bus_set_iommu(_bus_type, _smmu_ops);
-   if (ret)
-   return ret;
+   bus_set_iommu(_bus_type, _smmu_ops);
 #endif
 #ifdef CONFIG_ARM_AMBA
-   ret = bus_set_iommu(_bustype, _smmu_ops);
-   if (ret)
-   return ret;
+   bus_set_iommu(_bustype, _smmu_ops);
 #endif
-   return bus_set_iommu(_bus_type, _smmu_ops);
+   bus_set_iommu(_bus_type, _smmu_ops);
+   return 0;
 }
 
 static int arm_smmu_device_remove(struct platform_device *pdev)
-- 
1.9.1

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

RE: [PATCH V3 0/8] IOMMU probe deferral support

2016-10-17 Thread Sricharan 
Resending, missed out on the link last time.

>-Original Message-
>From: linux-arm-msm-ow...@vger.kernel.org 
>[mailto:linux-arm-msm-ow...@vger.kernel.org] On Behalf Of Marek Szyprowski
>Sent: Monday, October 10, 2016 6:07 PM
>To: Sricharan R ; will.dea...@arm.com; 
>robin.mur...@arm.com; j...@8bytes.org; iommu@lists.linux-
>foundation.org; linux-arm-ker...@lists.infradead.org; 
>linux-arm-...@vger.kernel.org; laurent.pinch...@ideasonboard.com;
>tf...@chromium.org; srinivas.kandaga...@linaro.org
>Subject: Re: [PATCH V3 0/8] IOMMU probe deferral support
>
>Hi Sricharan,
>
>
>On 2016-10-04 19:03, Sricharan R wrote:
>> Initial post from Laurent Pinchart[1]. This is
>> series calls the dma ops configuration for the devices
>> at a generic place so that it works for all busses.
>> The dma_configure_ops for a device is now called during
>> the device_attach callback just before the probe of the
>> bus/driver is called. Similarly dma_deconfigure is called during
>> device/driver_detach path.
>>
>>
>> pci_bus_add_devices(platform/amba)(_device_create/driver_register)
>> | |
>> pci_bus_add_device (device_add/driver_register)
>> | |
>> device_attach   device_initial_probe
>> | |
>> __device_attach_driver__device_attach_driver
>> |
>> driver_probe_device
>> |
>> really_probe
>> |
>> dma_configure
>>
>>   Similarly on the device/driver_unregister path __device_release_driver
>> is
>>   called which inturn calls dma_deconfigure.
>>
>>   If the ACPI bus code follows the same, we can add acpi_dma_configure
>>   at the same place as of_dma_configure.
>>
>>   This series is based on the recently merged Generic DT bindings for
>>   PCI IOMMUs and ARM SMMU from Robin Murphy robin.mur...@arm.com [2]
>>
>>   This time tested this with platform and pci device for probe deferral
>>   and reprobe on arm64 based platform. There is an issue on the cleanup
>>   path for arm64 though, where there is WARN_ON if the dma_ops is reset
>> while
>>   device is attached to an domain in arch_teardown_dma_ops.
>>   But with iommu_groups created from the iommu driver, the device is
>> always
>>   attached to a domain/default_domain. So so the WARN has to be
>> removed/handled
>>   probably.
>
>Thanks for continuing work on this feature! Your can add my:
>
>Tested-by: Marek Szyprowski 
>

Hi Will,Robin,Joerg,
   
   I have tested the probe deferral for platform/pcie bus devices based on 
latest Generic DT bindings
series merged [1], for pci/arm-smmu.
   It will be good to know from you on whats the right way to take this 
forward ?

[1] http://www.spinics.net/lists/devicetree/msg142943.html

Regards,
 Sricharan

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

RE: [PATCH V3 0/8] IOMMU probe deferral support

2016-10-17 Thread Sricharan 
>-Original Message-
>From: Sricharan [mailto:sricha...@codeaurora.org]
>Sent: Wednesday, October 12, 2016 11:55 AM
>To: 'Marek Szyprowski' ; 'will.dea...@arm.com' 
>; 'robin.mur...@arm.com'
>; 'j...@8bytes.org' ; 
>'iommu@lists.linux-foundation.org' foundation.org>; 'linux-arm-ker...@lists.infradead.org' 
>; 'linux-arm-...@vger.kernel.org'
>; 'laurent.pinch...@ideasonboard.com' 
>;
>'tf...@chromium.org' ; 'srinivas.kandaga...@linaro.org' 
>
>Subject: RE: [PATCH V3 0/8] IOMMU probe deferral support
>
>Hi Marek,
>
>>Hi Sricharan,
>>
>>
>>On 2016-10-04 19:03, Sricharan R wrote:
>>> Initial post from Laurent Pinchart[1]. This is
>>> series calls the dma ops configuration for the devices
>>> at a generic place so that it works for all busses.
>>> The dma_configure_ops for a device is now called during
>>> the device_attach callback just before the probe of the
>>> bus/driver is called. Similarly dma_deconfigure is called during
>>> device/driver_detach path.
>>>
>>>
>>> pci_bus_add_devices(platform/amba)(_device_create/driver_register)
>>> | |
>>> pci_bus_add_device (device_add/driver_register)
>>> | |
>>> device_attach   device_initial_probe
>>> | |
>>> __device_attach_driver__device_attach_driver
>>> |
>>> driver_probe_device
>>> |
>>> really_probe
>>> |
>>> dma_configure
>>>
>>>   Similarly on the device/driver_unregister path __device_release_driver is
>>>   called which inturn calls dma_deconfigure.
>>>
>>>   If the ACPI bus code follows the same, we can add acpi_dma_configure
>>>   at the same place as of_dma_configure.
>>>
>>>   This series is based on the recently merged Generic DT bindings for
>>>   PCI IOMMUs and ARM SMMU from Robin Murphy robin.mur...@arm.com [2]
>>>
>>>   This time tested this with platform and pci device for probe deferral
>>>   and reprobe on arm64 based platform. There is an issue on the cleanup
>>>   path for arm64 though, where there is WARN_ON if the dma_ops is reset 
>>> while
>>>   device is attached to an domain in arch_teardown_dma_ops.
>>>   But with iommu_groups created from the iommu driver, the device is always
>>>   attached to a domain/default_domain. So so the WARN has to be 
>>> removed/handled
>>>   probably.
>>
>>Thanks for continuing work on this feature! Your can add my:
>>
>>Tested-by: Marek Szyprowski 
>>
Hi Will,Robin,Joerg,
   
   I have tested the probe deferral for platform/pcie bus devices based on 
latest Generic bindings
series merged [1], for pci/arm-smmu.
   It will good to know from you on whats the right way to take this 
forward ?

Regards,
 Sricharan

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