Re: [PATCH/RFC 3/4] iommu: dma: iommu iova domain reset

[Magnus Damm]

Hi Robin, Shimoda-san, everyone,

Thanks for your feedback!

On Thu, Jan 26, 2017 at 1:38 AM, Robin Murphy  wrote:
> On 25/01/17 12:54, Yoshihiro Shimoda wrote:
>> From: Magnus Damm 
>>
>> To add a workaround code for ipmmu-vmsa driver, this patch adds
>> a new geometry "force_reset_when_empty" not to reuse iova space.
>
> The domain geometry is absolutely not the appropriate place for that. If
> anything, it could possibly be a domain attribute, but it has nothing to
> do with describing the range of input addresses the hardware is able to
> translate.

I agree, this flag was added without too much consideration about
correct location.

>> When all pfns happen to get unmapped then ask the IOMMU driver to
>> flush the state followed by starting from an empty iova space.
>
> And what happens if all the PFNs are never unmapped? Many devices (USB
> being among them) use a coherent allocation for some kind of descriptor
> buffer which exists for the lifetime of the device, then use streaming
> mappings for data transfers - the net result of that is that the number
> of PFNs mapped will always be >=1, and eventually streaming mapping will
> fail because you've exhausted the address space. And if the device *is*
> a USB controller, at that point the thread will hang because the USB
> core's response to a DMA mapping failure happens to be "keep trying
> indefinitely".

You are absolutely right. My apologies for not providing more
information in the first place.

Like you mention, the workaround can not be made into something
general purpose, however for some restricted use case it might be
helpful. For instance, we have some MMC controllers that are able to
perform on-chip bus mastering but they lack scatter gather support.
>From the hardware design point of view the scatter gather feature was
decided to be put into the IPMMU hardware. Because of that we would
like to use the IPMMU hardware for this purpose, however with some ES
specific errata we need to handle unmapping in a special way.
Fortunately the MMC controllers are grouped together using the same
IPMMU and we are able to make sure that all the buffers are unmapped
now and then from a workaround in the MMC device driver. Not pretty
but not super intrusive.

> Essentially, however long this allocation workaround postpones it, one
> or other failure mode is unavoidable with certain devices unless you can
> do something drastic like periodically suspend and resume the entire
> system to reset everything.

Yeah, suspend/resume or unbind/bind might work.

>> Signed-off-by: Magnus Damm 
>> Signed-off-by: Yoshihiro Shimoda 
>> ---
>>  drivers/iommu/dma-iommu.c | 42 +-
>>  drivers/iommu/iova.c  |  9 +
>>  include/linux/iommu.h |  2 ++
>>  include/linux/iova.h  |  1 +
>>  4 files changed, 49 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index a0b8c0f..d0fa0b1 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -42,6 +42,7 @@ struct iommu_dma_cookie {
>>   struct iova_domain  iovad;
>>   struct list_headmsi_page_list;
>>   spinlock_t  msi_lock;
>> + spinlock_t  reset_lock;
>
> So now we do get something in the cookie, but it's protecting a bunch of
> machinery that's accessible from a wider scope? That doesn't seem like a
> good design.

It's not. =)

There is room for improvement in the implementation for sure! We also
had other ideas of tracking mapped pages per device instead of per
domain. We have other requirements from the hardware to serialize some
MMC data handling, so grouping the MMC devices together into a single
IOMMU domain seemed light weight and simple for a first shot!

>>  };
>>
>>  static inline struct iova_domain *cookie_iovad(struct iommu_domain *domain)
>> @@ -74,6 +75,7 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>>
>>   spin_lock_init(>msi_lock);
>>   INIT_LIST_HEAD(>msi_page_list);
>> + spin_lock_init(>reset_lock);
>>   domain->iova_cookie = cookie;
>>   return 0;
>>  }
>> @@ -208,9 +210,11 @@ int dma_direction_to_prot(enum dma_data_direction dir, 
>> bool coherent)
>>  static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size,
>>   dma_addr_t dma_limit)
>>  {
>> + struct iommu_dma_cookie *cookie = domain->iova_cookie;
>>   struct iova_domain *iovad = cookie_iovad(domain);
>>   unsigned long shift = iova_shift(iovad);
>>   unsigned long length = iova_align(iovad, size) >> shift;
>> + unsigned long flags;
>>   struct iova *iova;
>>
>>   if (domain->geometry.force_aperture)
>> @@ -219,9 +223,19 @@ static struct iova *__alloc_iova(struct iommu_domain 
>> *domain, size_t size,
>>* Enforce size-alignment to be safe - there could perhaps be 

Re: [PATCH/RFC 1/4] iommu: dma: track mapped iova

[Magnus Damm]

Hi Robin, Shimoda-san, everyone,

On Thu, Jan 26, 2017 at 1:27 AM, Robin Murphy  wrote:
> On 25/01/17 12:53, Yoshihiro Shimoda wrote:
>> From: Magnus Damm 
>>
>> To track mapped iova for a workaround code in the future.
>>
>> Signed-off-by: Magnus Damm 
>> Signed-off-by: Yoshihiro Shimoda 
>> ---
>>  drivers/iommu/dma-iommu.c | 29 +++--
>>  include/linux/iommu.h |  2 ++
>
> So what's being added here is a counter of allocations within the
> iova_domain held by an iommu_dma_cookie? Then why is it all the way down
> in the iommu_domain and not in the cookie? That's needlessly invasive -
> it would be almost understandable (but still horrible) if you needed to
> refer to it directly from the IOMMU driver, but as far as I can see you
> don't.

You are very correct about all points. =)

As you noticed, the counter is kept per-domain. History is a bit
blurry but I think the reason for my abstraction-breaking is that i
decided to implement the simplest possible code to get the reset
callback to near the iova code and the domain level seemed suitable as
a first step. Moving it to a different level is of course no problem.

My main concern for this series is however if a subsystem-level
intrusive workaround like this ever could be beaten into acceptable
form for upstream merge.The code is pretty simple - the main portion
of the workaround is this counter that used to detect when the number
of mapped pages drops back to zero together with the callback. But
unless the code can be made pluggable it introduces overhead for all
users.

In your other email you asked what happens if the counter never drops
to zero. You are right that this workaround is not a general-purpose
fix suitable for all kinds of devices. Because of that the workaround
is designed to be enabled on only some of the IPMMU instances on the
system. It is also most likely an errata workaround for a particular
ES version, so we would most likely have to enable it with
soc_device_match().

I'll reply to patch 3/4 with some more details.

Thanks!

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

Re: [PATCH 0/2] Fix incorrect warning from dma-debug

[Geert Uytterhoeven]

Hi Robin,

On Wed, Jan 25, 2017 at 6:27 PM, Robin Murphy  wrote:
> On 25/01/17 16:23, Geert Uytterhoeven wrote:
>> On Mon, May 9, 2016 at 11:37 AM, Robin Murphy  wrote:
>>> On 08/05/16 11:59, Niklas Söderlund wrote:
 While using CONFIG_DMA_API_DEBUG i came across this warning which I
 think is a false positive. As shown dma_sync_single_for_device() are
 called from the dma_map_single() call path. This triggers the warning
 since the dma-debug code have not yet been made aware of the mapping.
>>>
>>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>>> current of_iommu_init() setup, the IOMMU is probed long before
>>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>>> some of the initial page table mappings and gets confused when we update
>>> them later.
>>
>> I think I've been seeing the same as Niklas since quite a while.
>> Finally I had a deeper look, and it looks like there is a bug somewhere,
>> causing the wrong IOMMU PTE to be synced.
>>
 I try to solve this by introducing __dma_sync_single_for_device() which
 do not call into the dma-debug code. I'm no expert and this might be a
 bad way of solving the problem but it allowed me to keep working.
>>>
>>> The simple fix should be to just call dma_debug_init() from a sufficiently
>>> earlier initcall level. The best would be to sort out a proper device
>>> dependency order to avoid the whole early-IOMMU-creation thing entirely.
>>
>> And so I did. After disabling the call to dma_debug_fs_init(), you can call
>> dma_debug_init() quite early. But the warning didn't go away:
>
> Yet the underlying reason has subtly changed!
>
>> ipmmu-vmsa e67b.mmu: DMA-API: device driver tries to sync DMA memory
>> it has not allocated [device address=0x00067bab2ff8] [size=8 
>> bytes]
>> [ cut here ]
>> WARNING: CPU: 0 PID: 174 at lib/dma-debug.c:1235 check_sync+0xcc/0x568
>> ...
>> [] check_sync+0xcc/0x568
>> [] debug_dma_sync_single_for_device+0x44/0x4c
>> [] __arm_lpae_set_pte.isra.3+0x6c/0x78
>> [] __arm_lpae_map+0x318/0x384
>> [] arm_lpae_map+0xb0/0xc4
>> [] ipmmu_map+0x48/0x58
>> [] iommu_map+0x120/0x1fc
>> [] __iommu_dma_map+0xb8/0xec
>> [] iommu_dma_map_page+0x50/0x58
>> [] __iommu_map_page+0x54/0x98
>>
>> So, who allocated that memory?
>>
>> During early kernel init (before fs_initcall(dma_debug_init)):
>>
>> arm-lpae io-pgtable: arm_lpae_alloc_pgtable:652: cfg->ias = 32
>
> Note that you have a 32-bit IAS...

Ah, so that's what it means...

>> data->pg_shift = 12 va_bits = 20
>> arm-lpae io-pgtable: arm_lpae_alloc_pgtable:657: data->bits_per_level = 9
>> data->levels = 3 pgd_bits = 2
>> ipmmu-vmsa e67b.mmu: __arm_lpae_alloc_pages:224
>> dma_map_single(0xffc63bab2000, 32) returned 0x00067bab2000
>>
>> Hence 0x67bab2000 is the PGD, which has only 4 entries (32 bytes).
>> Call stack:
>>
>> [] __arm_lpae_alloc_pages.isra.11+0x144/0x1e8
>> [] arm_64_lpae_alloc_pgtable_s1+0xdc/0x118
>> [] arm_32_lpae_alloc_pgtable_s1+0x44/0x68
>> [] alloc_io_pgtable_ops+0x4c/0x80
>> [] ipmmu_attach_device+0xd0/0x3b0
>>
>> When starting DMA from the device:
>>
>> iommu: map: iova 0xfff000 pa 0x00067a555000 size 0x1000 pgsize 
>> 4096
>
> ...then count the f's carefully.

Indeed, 40-bits.

>> arm-lpae io-pgtable: __arm_lpae_map:318: iova 0xfff000
>> phys 0x00067a555000 size 4096 lvl 1 ptep 0xffc63bab2000
>> arm-lpae io-pgtable: __arm_lpae_map:320: incr. ptep to 0xffc63bab2ff8
>> ipmmu-vmsa e67b.mmu: __arm_lpae_alloc_pages:224
>> dma_map_single(0xffc63a49, 4096) returned 0x00067a49
>> ipmmu-vmsa e67b.mmu: DMA-API: device driver tries to sync DMA memory
>> it has not allocated [device address=0x00067bab2ff8] [size=8 
>> bytes]
>>
>> __arm_lpae_map() added "ARM_LPAE_LVL_IDX(iova, lvl, data)" == 0xff8 to ptep
>> (the PGD base address), but the PGD has only 32 bytes, leading to the 
>> warning.
>>
>> Does my analysis make sense?
>> Do you have a clue?
>
> The initial false positive misleads from the fact that this is actually
> DMA-debug doing its job admirably. The bug lies in however you ended up
> trying to map a 40-bit IOVA in a 32-bit pagetable.

Right, I completely forgot that I was still running with a patch to
allow the DMAC to use 40-bit addresses:

dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40));

which works fine without IOMMU, but fails with, as the IPMMU-VMSA driver
doesn't support that yet.

Thanks a lot, and sorry for the noise...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, 

Re: [PATCH V7 01/11] iommu/of: Refactor of_iommu_configure() for error handling

[Tomasz Nowicki]

Hi Robin,

On 25.01.2017 18:35, Robin Murphy wrote:

Hi Tomasz,

On 25/01/17 17:17, Tomasz Nowicki wrote:

Hi Sricharan,

On 23.01.2017 17:18, Sricharan R wrote:

From: Robin Murphy 

In preparation for some upcoming cleverness, rework the control flow in
of_iommu_configure() to minimise duplication and improve the propogation
of errors. It's also as good a time as any to switch over from the
now-just-a-compatibility-wrapper of_iommu_get_ops() to using the generic
IOMMU instance interface directly.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/of_iommu.c | 83
+++-
 1 file changed, 53 insertions(+), 30 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 0f57ddc..ee49081 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const
char *prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);

+static const struct iommu_ops
+*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec)
+{
+const struct iommu_ops *ops;
+struct fwnode_handle *fwnode = _spec->np->fwnode;
+int err;
+
+ops = iommu_get_instance(fwnode);
+if (!ops || !ops->of_xlate)
+return NULL;
+
+err = iommu_fwspec_init(dev, _spec->np->fwnode, ops);
+if (err)
+return ERR_PTR(err);
+
+err = ops->of_xlate(dev, iommu_spec);
+if (err)
+return ERR_PTR(err);
+
+return ops;
+}
+
 static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
 {
 struct of_phandle_args *iommu_spec = data;
@@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev,
u16 alias, void *data)
 }

 static const struct iommu_ops
-*of_pci_iommu_configure(struct pci_dev *pdev, struct device_node
*bridge_np)
+*of_pci_iommu_init(struct pci_dev *pdev, struct device_node *bridge_np)
 {
 const struct iommu_ops *ops;
 struct of_phandle_args iommu_spec;
+int err;

 /*
  * Start by tracing the RID alias down the PCI topology as
@@ -123,56 +146,56 @@ static int __get_pci_rid(struct pci_dev *pdev,
u16 alias, void *data)
  * bus into the system beyond, and which IOMMU it ends up at.
  */
 iommu_spec.np = NULL;
-if (of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
-   "iommu-map-mask", _spec.np, iommu_spec.args))
-return NULL;
+err = of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
+ "iommu-map-mask", _spec.np,
+ iommu_spec.args);
+if (err)
+return ERR_PTR(err);

-ops = of_iommu_get_ops(iommu_spec.np);
-if (!ops || !ops->of_xlate ||
-iommu_fwspec_init(>dev, _spec.np->fwnode, ops) ||
-ops->of_xlate(>dev, _spec))
-ops = NULL;
+ops = of_iommu_xlate(>dev, _spec);

 of_node_put(iommu_spec.np);
 return ops;
 }

-const struct iommu_ops *of_iommu_configure(struct device *dev,
-   struct device_node *master_np)
+static const struct iommu_ops
+*of_platform_iommu_init(struct device *dev, struct device_node *np)
 {
 struct of_phandle_args iommu_spec;
-struct device_node *np;
 const struct iommu_ops *ops = NULL;
 int idx = 0;

-if (dev_is_pci(dev))
-return of_pci_iommu_configure(to_pci_dev(dev), master_np);
-
 /*
  * We don't currently walk up the tree looking for a parent IOMMU.
  * See the `Notes:' section of
  * Documentation/devicetree/bindings/iommu/iommu.txt
  */
-while (!of_parse_phandle_with_args(master_np, "iommus",
-   "#iommu-cells", idx,
-   _spec)) {
-np = iommu_spec.np;
-ops = of_iommu_get_ops(np);
-
-if (!ops || !ops->of_xlate ||
-iommu_fwspec_init(dev, >fwnode, ops) ||
-ops->of_xlate(dev, _spec))
-goto err_put_node;
-
-of_node_put(np);
+while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells",
+   idx, _spec)) {
+ops = of_iommu_xlate(dev, _spec);
+of_node_put(iommu_spec.np);
 idx++;
+if (IS_ERR_OR_NULL(ops))
+break;
 }

 return ops;
+}
+
+const struct iommu_ops *of_iommu_configure(struct device *dev,
+   struct device_node *master_np)
+{
+const struct iommu_ops *ops;
+
+if (!master_np)
+return NULL;
+
+if (dev_is_pci(dev))
+ops = of_pci_iommu_init(to_pci_dev(dev), master_np);

I gave the whole patch set a try on ThunderX. really_probe() is failing
on dma_configure()->of_pci_iommu_init() for each PCI device.

When you say "failing", do you mean cleanly, or with a crash? I've
managed to hit __of_match_node() dereferencing NULL from
of_iommu_xlate() in a horribly complicated chain of events, which I'm
trying to figure out now, and I wonder if the two might be related.



Cleanly, of_pci_iommu_init()->of_pci_map_rid() can't 

Re: [PATCH V7 01/11] iommu/of: Refactor of_iommu_configure() for error handling

[Robin Murphy]

Hi Tomasz,

On 25/01/17 17:17, Tomasz Nowicki wrote:
> Hi Sricharan,
> 
> On 23.01.2017 17:18, Sricharan R wrote:
>> From: Robin Murphy 
>>
>> In preparation for some upcoming cleverness, rework the control flow in
>> of_iommu_configure() to minimise duplication and improve the propogation
>> of errors. It's also as good a time as any to switch over from the
>> now-just-a-compatibility-wrapper of_iommu_get_ops() to using the generic
>> IOMMU instance interface directly.
>>
>> Signed-off-by: Robin Murphy 
>> ---
>>  drivers/iommu/of_iommu.c | 83
>> +++-
>>  1 file changed, 53 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index 0f57ddc..ee49081 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const
>> char *prefix, int index,
>>  }
>>  EXPORT_SYMBOL_GPL(of_get_dma_window);
>>
>> +static const struct iommu_ops
>> +*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec)
>> +{
>> +const struct iommu_ops *ops;
>> +struct fwnode_handle *fwnode = _spec->np->fwnode;
>> +int err;
>> +
>> +ops = iommu_get_instance(fwnode);
>> +if (!ops || !ops->of_xlate)
>> +return NULL;
>> +
>> +err = iommu_fwspec_init(dev, _spec->np->fwnode, ops);
>> +if (err)
>> +return ERR_PTR(err);
>> +
>> +err = ops->of_xlate(dev, iommu_spec);
>> +if (err)
>> +return ERR_PTR(err);
>> +
>> +return ops;
>> +}
>> +
>>  static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
>>  {
>>  struct of_phandle_args *iommu_spec = data;
>> @@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev,
>> u16 alias, void *data)
>>  }
>>
>>  static const struct iommu_ops
>> -*of_pci_iommu_configure(struct pci_dev *pdev, struct device_node
>> *bridge_np)
>> +*of_pci_iommu_init(struct pci_dev *pdev, struct device_node *bridge_np)
>>  {
>>  const struct iommu_ops *ops;
>>  struct of_phandle_args iommu_spec;
>> +int err;
>>
>>  /*
>>   * Start by tracing the RID alias down the PCI topology as
>> @@ -123,56 +146,56 @@ static int __get_pci_rid(struct pci_dev *pdev,
>> u16 alias, void *data)
>>   * bus into the system beyond, and which IOMMU it ends up at.
>>   */
>>  iommu_spec.np = NULL;
>> -if (of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
>> -   "iommu-map-mask", _spec.np, iommu_spec.args))
>> -return NULL;
>> +err = of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
>> + "iommu-map-mask", _spec.np,
>> + iommu_spec.args);
>> +if (err)
>> +return ERR_PTR(err);
>>
>> -ops = of_iommu_get_ops(iommu_spec.np);
>> -if (!ops || !ops->of_xlate ||
>> -iommu_fwspec_init(>dev, _spec.np->fwnode, ops) ||
>> -ops->of_xlate(>dev, _spec))
>> -ops = NULL;
>> +ops = of_iommu_xlate(>dev, _spec);
>>
>>  of_node_put(iommu_spec.np);
>>  return ops;
>>  }
>>
>> -const struct iommu_ops *of_iommu_configure(struct device *dev,
>> -   struct device_node *master_np)
>> +static const struct iommu_ops
>> +*of_platform_iommu_init(struct device *dev, struct device_node *np)
>>  {
>>  struct of_phandle_args iommu_spec;
>> -struct device_node *np;
>>  const struct iommu_ops *ops = NULL;
>>  int idx = 0;
>>
>> -if (dev_is_pci(dev))
>> -return of_pci_iommu_configure(to_pci_dev(dev), master_np);
>> -
>>  /*
>>   * We don't currently walk up the tree looking for a parent IOMMU.
>>   * See the `Notes:' section of
>>   * Documentation/devicetree/bindings/iommu/iommu.txt
>>   */
>> -while (!of_parse_phandle_with_args(master_np, "iommus",
>> -   "#iommu-cells", idx,
>> -   _spec)) {
>> -np = iommu_spec.np;
>> -ops = of_iommu_get_ops(np);
>> -
>> -if (!ops || !ops->of_xlate ||
>> -iommu_fwspec_init(dev, >fwnode, ops) ||
>> -ops->of_xlate(dev, _spec))
>> -goto err_put_node;
>> -
>> -of_node_put(np);
>> +while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells",
>> +   idx, _spec)) {
>> +ops = of_iommu_xlate(dev, _spec);
>> +of_node_put(iommu_spec.np);
>>  idx++;
>> +if (IS_ERR_OR_NULL(ops))
>> +break;
>>  }
>>
>>  return ops;
>> +}
>> +
>> +const struct iommu_ops *of_iommu_configure(struct device *dev,
>> +   struct device_node *master_np)
>> +{
>> +const struct iommu_ops *ops;
>> +
>> +if (!master_np)
>> +return NULL;
>> +
>> +if (dev_is_pci(dev))
>> +ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
> 
> I gave the whole patch set a try on ThunderX. really_probe() is failing
> on 

Re: [PATCH V7 10/11] iommu/arm-smmu: Clean up early-probing workarounds

[Tomasz Nowicki]

On 23.01.2017 17:18, Sricharan R wrote:

From: Robin Murphy 

Now that the appropriate ordering is enforced via profe-deferral of
masters in core code, rip it all out and bask in the simplicity.

Acked-by: Will Deacon 
Signed-off-by: Robin Murphy 
[Sricharan: Rebased on top of ACPI IORT SMMU series]
Signed-off-by: Sricharan R 


[...]


-
-#ifdef CONFIG_ACPI
-static int __init acpi_smmu_v3_init(struct acpi_table_header *table)
-{
-   if (iort_node_match(ACPI_IORT_NODE_SMMU_V3))
-   return arm_smmu_init();


Since we clean the code we can remove iort_node_match() from iort.c. 
There is no user for it any more. Unless Lorenzo has some plans for it.


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

Re: [PATCH V7 01/11] iommu/of: Refactor of_iommu_configure() for error handling

[Tomasz Nowicki]

Hi Sricharan,

On 23.01.2017 17:18, Sricharan R wrote:

From: Robin Murphy 

In preparation for some upcoming cleverness, rework the control flow in
of_iommu_configure() to minimise duplication and improve the propogation
of errors. It's also as good a time as any to switch over from the
now-just-a-compatibility-wrapper of_iommu_get_ops() to using the generic
IOMMU instance interface directly.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/of_iommu.c | 83 +++-
 1 file changed, 53 insertions(+), 30 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 0f57ddc..ee49081 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const char 
*prefix, int index,
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);

+static const struct iommu_ops
+*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec)
+{
+   const struct iommu_ops *ops;
+   struct fwnode_handle *fwnode = _spec->np->fwnode;
+   int err;
+
+   ops = iommu_get_instance(fwnode);
+   if (!ops || !ops->of_xlate)
+   return NULL;
+
+   err = iommu_fwspec_init(dev, _spec->np->fwnode, ops);
+   if (err)
+   return ERR_PTR(err);
+
+   err = ops->of_xlate(dev, iommu_spec);
+   if (err)
+   return ERR_PTR(err);
+
+   return ops;
+}
+
 static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
 {
struct of_phandle_args *iommu_spec = data;
@@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, 
void *data)
 }

 static const struct iommu_ops
-*of_pci_iommu_configure(struct pci_dev *pdev, struct device_node *bridge_np)
+*of_pci_iommu_init(struct pci_dev *pdev, struct device_node *bridge_np)
 {
const struct iommu_ops *ops;
struct of_phandle_args iommu_spec;
+   int err;

/*
 * Start by tracing the RID alias down the PCI topology as
@@ -123,56 +146,56 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, 
void *data)
 * bus into the system beyond, and which IOMMU it ends up at.
 */
iommu_spec.np = NULL;
-   if (of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
-  "iommu-map-mask", _spec.np, iommu_spec.args))
-   return NULL;
+   err = of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
+"iommu-map-mask", _spec.np,
+iommu_spec.args);
+   if (err)
+   return ERR_PTR(err);

-   ops = of_iommu_get_ops(iommu_spec.np);
-   if (!ops || !ops->of_xlate ||
-   iommu_fwspec_init(>dev, _spec.np->fwnode, ops) ||
-   ops->of_xlate(>dev, _spec))
-   ops = NULL;
+   ops = of_iommu_xlate(>dev, _spec);

of_node_put(iommu_spec.np);
return ops;
 }

-const struct iommu_ops *of_iommu_configure(struct device *dev,
-  struct device_node *master_np)
+static const struct iommu_ops
+*of_platform_iommu_init(struct device *dev, struct device_node *np)
 {
struct of_phandle_args iommu_spec;
-   struct device_node *np;
const struct iommu_ops *ops = NULL;
int idx = 0;

-   if (dev_is_pci(dev))
-   return of_pci_iommu_configure(to_pci_dev(dev), master_np);
-
/*
 * We don't currently walk up the tree looking for a parent IOMMU.
 * See the `Notes:' section of
 * Documentation/devicetree/bindings/iommu/iommu.txt
 */
-   while (!of_parse_phandle_with_args(master_np, "iommus",
-  "#iommu-cells", idx,
-  _spec)) {
-   np = iommu_spec.np;
-   ops = of_iommu_get_ops(np);
-
-   if (!ops || !ops->of_xlate ||
-   iommu_fwspec_init(dev, >fwnode, ops) ||
-   ops->of_xlate(dev, _spec))
-   goto err_put_node;
-
-   of_node_put(np);
+   while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells",
+  idx, _spec)) {
+   ops = of_iommu_xlate(dev, _spec);
+   of_node_put(iommu_spec.np);
idx++;
+   if (IS_ERR_OR_NULL(ops))
+   break;
}

return ops;
+}
+
+const struct iommu_ops *of_iommu_configure(struct device *dev,
+  struct device_node *master_np)
+{
+   const struct iommu_ops *ops;
+
+   if (!master_np)
+   return NULL;
+
+   if (dev_is_pci(dev))
+   ops = of_pci_iommu_init(to_pci_dev(dev), master_np);


I gave the whole patch set a try on ThunderX. really_probe() is failing 
on dma_configure()->of_pci_iommu_init() for each PCI device. 

Re: [PATCH/RFC 3/4] iommu: dma: iommu iova domain reset

[Robin Murphy]

On 25/01/17 12:54, Yoshihiro Shimoda wrote:
> From: Magnus Damm 
> 
> To add a workaround code for ipmmu-vmsa driver, this patch adds
> a new geometry "force_reset_when_empty" not to reuse iova space.

The domain geometry is absolutely not the appropriate place for that. If
anything, it could possibly be a domain attribute, but it has nothing to
do with describing the range of input addresses the hardware is able to
translate.

> When all pfns happen to get unmapped then ask the IOMMU driver to
> flush the state followed by starting from an empty iova space.

And what happens if all the PFNs are never unmapped? Many devices (USB
being among them) use a coherent allocation for some kind of descriptor
buffer which exists for the lifetime of the device, then use streaming
mappings for data transfers - the net result of that is that the number
of PFNs mapped will always be >=1, and eventually streaming mapping will
fail because you've exhausted the address space. And if the device *is*
a USB controller, at that point the thread will hang because the USB
core's response to a DMA mapping failure happens to be "keep trying
indefinitely".

Essentially, however long this allocation workaround postpones it, one
or other failure mode is unavoidable with certain devices unless you can
do something drastic like periodically suspend and resume the entire
system to reset everything.

> Signed-off-by: Magnus Damm 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/iommu/dma-iommu.c | 42 +-
>  drivers/iommu/iova.c  |  9 +
>  include/linux/iommu.h |  2 ++
>  include/linux/iova.h  |  1 +
>  4 files changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index a0b8c0f..d0fa0b1 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -42,6 +42,7 @@ struct iommu_dma_cookie {
>   struct iova_domain  iovad;
>   struct list_headmsi_page_list;
>   spinlock_t  msi_lock;
> + spinlock_t  reset_lock;

So now we do get something in the cookie, but it's protecting a bunch of
machinery that's accessible from a wider scope? That doesn't seem like a
good design.

>  };
>  
>  static inline struct iova_domain *cookie_iovad(struct iommu_domain *domain)
> @@ -74,6 +75,7 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>  
>   spin_lock_init(>msi_lock);
>   INIT_LIST_HEAD(>msi_page_list);
> + spin_lock_init(>reset_lock);
>   domain->iova_cookie = cookie;
>   return 0;
>  }
> @@ -208,9 +210,11 @@ int dma_direction_to_prot(enum dma_data_direction dir, 
> bool coherent)
>  static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size,
>   dma_addr_t dma_limit)
>  {
> + struct iommu_dma_cookie *cookie = domain->iova_cookie;
>   struct iova_domain *iovad = cookie_iovad(domain);
>   unsigned long shift = iova_shift(iovad);
>   unsigned long length = iova_align(iovad, size) >> shift;
> + unsigned long flags;
>   struct iova *iova;
>  
>   if (domain->geometry.force_aperture)
> @@ -219,9 +223,19 @@ static struct iova *__alloc_iova(struct iommu_domain 
> *domain, size_t size,
>* Enforce size-alignment to be safe - there could perhaps be an
>* attribute to control this per-device, or at least per-domain...
>*/
> - iova = alloc_iova(iovad, length, dma_limit >> shift, true);
> - if (iova)
> - atomic_add(iova_size(iova), >iova_pfns_mapped);
> + if (domain->geometry.force_reset_when_empty) {
> + spin_lock_irqsave(>reset_lock, flags);

Is this locking definitely safe against the potential concurrent callers
of __free_iova() on the same domain who won't touch reset_lock? (It may
well be, it's just not clear)

> +
> + iova = alloc_iova(iovad, length, dma_limit >> shift, true);
> + if (iova)
> + atomic_add(iova_size(iova), >iova_pfns_mapped);
> +
> + spin_unlock_irqrestore(>reset_lock, flags);
> + } else {
> + iova = alloc_iova(iovad, length, dma_limit >> shift, true);
> + if (iova)
> + atomic_add(iova_size(iova), >iova_pfns_mapped);

Why would we even need to keep track of this on non-broken IOMMUS?

> + }
>  
>   return iova;
>  }
> @@ -229,10 +243,28 @@ static struct iova *__alloc_iova(struct iommu_domain 
> *domain, size_t size,
>  void
>  __free_iova_domain(struct iommu_domain *domain, struct iova *iova)
>  {
> + struct iommu_dma_cookie *cookie = domain->iova_cookie;
>   struct iova_domain *iovad = cookie_iovad(domain);
> + unsigned long flags;
>  
> - atomic_sub(iova_size(iova), >iova_pfns_mapped);
> - __free_iova(iovad, iova);
> + /* In case force_reset_when_empty is set, do not reuse iova space
> +

Re: [PATCH/RFC 1/4] iommu: dma: track mapped iova

[Robin Murphy]

On 25/01/17 12:53, Yoshihiro Shimoda wrote:
> From: Magnus Damm 
> 
> To track mapped iova for a workaround code in the future.
> 
> Signed-off-by: Magnus Damm 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/iommu/dma-iommu.c | 29 +++--
>  include/linux/iommu.h |  2 ++

So what's being added here is a counter of allocations within the
iova_domain held by an iommu_dma_cookie? Then why is it all the way down
in the iommu_domain and not in the cookie? That's needlessly invasive -
it would be almost understandable (but still horrible) if you needed to
refer to it directly from the IOMMU driver, but as far as I can see you
don't.

Robin.

>  2 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 2db0d64..a0b8c0f 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -19,6 +19,7 @@
>   * along with this program.  If not, see .
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -147,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, 
> dma_addr_t base,
>   order = __ffs(domain->pgsize_bitmap);
>   base_pfn = max_t(unsigned long, 1, base >> order);
>   end_pfn = (base + size - 1) >> order;
> + atomic_set(>iova_pfns_mapped, 0);
>  
>   /* Check the domain allows at least some access to the device... */
>   if (domain->geometry.force_aperture) {
> @@ -209,6 +211,7 @@ static struct iova *__alloc_iova(struct iommu_domain 
> *domain, size_t size,
>   struct iova_domain *iovad = cookie_iovad(domain);
>   unsigned long shift = iova_shift(iovad);
>   unsigned long length = iova_align(iovad, size) >> shift;
> + struct iova *iova;
>  
>   if (domain->geometry.force_aperture)
>   dma_limit = min(dma_limit, domain->geometry.aperture_end);
> @@ -216,9 +219,23 @@ static struct iova *__alloc_iova(struct iommu_domain 
> *domain, size_t size,
>* Enforce size-alignment to be safe - there could perhaps be an
>* attribute to control this per-device, or at least per-domain...
>*/
> - return alloc_iova(iovad, length, dma_limit >> shift, true);
> + iova = alloc_iova(iovad, length, dma_limit >> shift, true);
> + if (iova)
> + atomic_add(iova_size(iova), >iova_pfns_mapped);
> +
> + return iova;
>  }
>  
> +void
> +__free_iova_domain(struct iommu_domain *domain, struct iova *iova)
> +{
> + struct iova_domain *iovad = cookie_iovad(domain);
> +
> + atomic_sub(iova_size(iova), >iova_pfns_mapped);
> + __free_iova(iovad, iova);
> +}
> +
> +
>  /* The IOVA allocator knows what we mapped, so just unmap whatever that was 
> */
>  static void __iommu_dma_unmap(struct iommu_domain *domain, dma_addr_t 
> dma_addr)
>  {
> @@ -235,7 +252,7 @@ static void __iommu_dma_unmap(struct iommu_domain 
> *domain, dma_addr_t dma_addr)
>   size -= iommu_unmap(domain, pfn << shift, size);
>   /* ...and if we can't, then something is horribly, horribly wrong */
>   WARN_ON(size > 0);
> - __free_iova(iovad, iova);
> + __free_iova_domain(domain, iova);
>  }
>  
>  static void __iommu_dma_free_pages(struct page **pages, int count)
> @@ -401,7 +418,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t 
> size, gfp_t gfp,
>  out_free_sg:
>   sg_free_table();
>  out_free_iova:
> - __free_iova(iovad, iova);
> + __free_iova_domain(domain, iova);
>  out_free_pages:
>   __iommu_dma_free_pages(pages, count);
>   return NULL;
> @@ -447,7 +464,7 @@ static dma_addr_t __iommu_dma_map(struct device *dev, 
> phys_addr_t phys,
>  
>   dma_addr = iova_dma_addr(iovad, iova);
>   if (iommu_map(domain, dma_addr, phys - iova_off, len, prot)) {
> - __free_iova(iovad, iova);
> + __free_iova_domain(domain, iova);
>   return DMA_ERROR_CODE;
>   }
>   return dma_addr + iova_off;
> @@ -613,7 +630,7 @@ int iommu_dma_map_sg(struct device *dev, struct 
> scatterlist *sg,
>   return __finalise_sg(dev, sg, nents, dma_addr);
>  
>  out_free_iova:
> - __free_iova(iovad, iova);
> + __free_iova_domain(domain, iova);
>  out_restore_sg:
>   __invalidate_sg(sg, nents);
>   return 0;
> @@ -689,7 +706,7 @@ static struct iommu_dma_msi_page 
> *iommu_dma_get_msi_page(struct device *dev,
>   return msi_page;
>  
>  out_free_iova:
> - __free_iova(iovad, iova);
> + __free_iova_domain(domain, iova);
>  out_free_page:
>   kfree(msi_page);
>   return NULL;
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0ff5111..91d0159 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -19,6 +19,7 @@
>  #ifndef __LINUX_IOMMU_H
>  #define __LINUX_IOMMU_H
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -84,6 +85,7 @@ struct iommu_domain {
>   

[PATCH/RFC 3/4] iommu: dma: iommu iova domain reset

[Yoshihiro Shimoda]

From: Magnus Damm 

To add a workaround code for ipmmu-vmsa driver, this patch adds
a new geometry "force_reset_when_empty" not to reuse iova space.
When all pfns happen to get unmapped then ask the IOMMU driver to
flush the state followed by starting from an empty iova space.

Signed-off-by: Magnus Damm 
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/iommu/dma-iommu.c | 42 +-
 drivers/iommu/iova.c  |  9 +
 include/linux/iommu.h |  2 ++
 include/linux/iova.h  |  1 +
 4 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index a0b8c0f..d0fa0b1 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -42,6 +42,7 @@ struct iommu_dma_cookie {
struct iova_domain  iovad;
struct list_headmsi_page_list;
spinlock_t  msi_lock;
+   spinlock_t  reset_lock;
 };
 
 static inline struct iova_domain *cookie_iovad(struct iommu_domain *domain)
@@ -74,6 +75,7 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
 
spin_lock_init(>msi_lock);
INIT_LIST_HEAD(>msi_page_list);
+   spin_lock_init(>reset_lock);
domain->iova_cookie = cookie;
return 0;
 }
@@ -208,9 +210,11 @@ int dma_direction_to_prot(enum dma_data_direction dir, 
bool coherent)
 static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size,
dma_addr_t dma_limit)
 {
+   struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iova_domain *iovad = cookie_iovad(domain);
unsigned long shift = iova_shift(iovad);
unsigned long length = iova_align(iovad, size) >> shift;
+   unsigned long flags;
struct iova *iova;
 
if (domain->geometry.force_aperture)
@@ -219,9 +223,19 @@ static struct iova *__alloc_iova(struct iommu_domain 
*domain, size_t size,
 * Enforce size-alignment to be safe - there could perhaps be an
 * attribute to control this per-device, or at least per-domain...
 */
-   iova = alloc_iova(iovad, length, dma_limit >> shift, true);
-   if (iova)
-   atomic_add(iova_size(iova), >iova_pfns_mapped);
+   if (domain->geometry.force_reset_when_empty) {
+   spin_lock_irqsave(>reset_lock, flags);
+
+   iova = alloc_iova(iovad, length, dma_limit >> shift, true);
+   if (iova)
+   atomic_add(iova_size(iova), >iova_pfns_mapped);
+
+   spin_unlock_irqrestore(>reset_lock, flags);
+   } else {
+   iova = alloc_iova(iovad, length, dma_limit >> shift, true);
+   if (iova)
+   atomic_add(iova_size(iova), >iova_pfns_mapped);
+   }
 
return iova;
 }
@@ -229,10 +243,28 @@ static struct iova *__alloc_iova(struct iommu_domain 
*domain, size_t size,
 void
 __free_iova_domain(struct iommu_domain *domain, struct iova *iova)
 {
+   struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iova_domain *iovad = cookie_iovad(domain);
+   unsigned long flags;
 
-   atomic_sub(iova_size(iova), >iova_pfns_mapped);
-   __free_iova(iovad, iova);
+   /* In case force_reset_when_empty is set, do not reuse iova space
+* but instead simply keep on expanding seemingly forever.
+* When all pfns happen to get unmapped then ask the IOMMU driver to
+* flush the state followed by starting from an empty iova space.
+*/
+   if (domain->geometry.force_reset_when_empty) {
+   spin_lock_irqsave(>reset_lock, flags);
+   if (atomic_sub_return(iova_size(iova),
+ >iova_pfns_mapped) == 0) {
+   reset_iova_domain(iovad);
+   if (domain->ops->domain_reset)
+   domain->ops->domain_reset(domain);
+   }
+   spin_unlock_irqrestore(>reset_lock, flags);
+   } else {
+   atomic_sub(iova_size(iova), >iova_pfns_mapped);
+   __free_iova(iovad, iova);
+   }
 }
 
 
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 866ad65..50aaa46 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -464,6 +464,7 @@ void put_iova_domain(struct iova_domain *iovad)
while (node) {
struct iova *iova = rb_entry(node, struct iova, node);
 
+   __cached_rbnode_delete_update(iovad, iova);
rb_erase(node, >rbroot);
free_iova_mem(iova);
node = rb_first(>rbroot);
@@ -472,6 +473,14 @@ void put_iova_domain(struct iova_domain *iovad)
 }
 EXPORT_SYMBOL_GPL(put_iova_domain);
 
+void
+reset_iova_domain(struct iova_domain *iovad)
+{
+   put_iova_domain(iovad);
+   init_iova_rcaches(iovad);
+}

[PATCH/RFC 2/4] iommu: iova: use __alloc_percpu_gfp() with GFP_NOWAIT in init_iova_rcaches()

[Yoshihiro Shimoda]

In the future, the init_iova_rcaches will be called in atomic.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/iommu/iova.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index b7268a1..866ad65 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -723,7 +723,9 @@ static void init_iova_rcaches(struct iova_domain *iovad)
rcache = >rcaches[i];
spin_lock_init(>lock);
rcache->depot_size = 0;
-   rcache->cpu_rcaches = __alloc_percpu(sizeof(*cpu_rcache), 
cache_line_size());
+   rcache->cpu_rcaches = __alloc_percpu_gfp(sizeof(*cpu_rcache),
+cache_line_size(),
+GFP_NOWAIT);
if (WARN_ON(!rcache->cpu_rcaches))
continue;
for_each_possible_cpu(cpu) {
-- 
1.9.1

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

[PATCH/RFC 4/4] iommu: ipmmu-vmsa: enable force_reset_when_empty

[Yoshihiro Shimoda]

The IPMMU of R-Car Gen3 will mistake an address translation if
IMCTR.FLUSH is set while some related devices that on the same doamin
are running. To avoid this, this patch uses the force_reset_when_empty
feature.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/iommu/ipmmu-vmsa.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 11550ac..4b62969 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -302,7 +302,8 @@ static void ipmmu_tlb_flush_all(void *cookie)
 {
struct ipmmu_vmsa_domain *domain = cookie;
 
-   ipmmu_tlb_invalidate(domain);
+   if (!domain->io_domain.geometry.force_reset_when_empty)
+   ipmmu_tlb_invalidate(domain);
 }
 
 static void ipmmu_tlb_add_flush(unsigned long iova, size_t size,
@@ -555,6 +556,13 @@ static void ipmmu_domain_free(struct iommu_domain 
*io_domain)
kfree(domain);
 }
 
+static void ipmmu_domain_reset(struct iommu_domain *io_domain)
+{
+   struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
+
+   ipmmu_tlb_invalidate(domain);
+}
+
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
   struct device *dev)
 {
@@ -832,6 +840,7 @@ static struct iommu_domain *ipmmu_domain_alloc(unsigned 
type)
 static const struct iommu_ops ipmmu_ops = {
.domain_alloc = ipmmu_domain_alloc,
.domain_free = ipmmu_domain_free,
+   .domain_reset = ipmmu_domain_reset,
.attach_dev = ipmmu_attach_device,
.detach_dev = ipmmu_detach_device,
.map = ipmmu_map,
@@ -858,8 +867,10 @@ static struct iommu_domain 
*ipmmu_domain_alloc_dma(unsigned type)
 
case IOMMU_DOMAIN_DMA:
io_domain = __ipmmu_domain_alloc(type);
-   if (io_domain)
+   if (io_domain) {
iommu_get_dma_cookie(io_domain);
+   io_domain->geometry.force_reset_when_empty = true;
+   }
break;
}
 
@@ -927,6 +938,7 @@ static int ipmmu_of_xlate_dma(struct device *dev,
 static const struct iommu_ops ipmmu_ops = {
.domain_alloc = ipmmu_domain_alloc_dma,
.domain_free = ipmmu_domain_free_dma,
+   .domain_reset = ipmmu_domain_reset,
.attach_dev = ipmmu_attach_device,
.detach_dev = ipmmu_detach_device,
.map = ipmmu_map,
-- 
1.9.1

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

Re: [PATCH/RFC 1/2] arm64: mm: Silently allow devices lacking IOMMU group

[Magnus Damm]

Hi Robin,

On Mon, Jan 23, 2017 at 9:34 PM, Robin Murphy  wrote:
> Hi Magnus,
>
> On 23/01/17 12:12, Magnus Damm wrote:
>> From: Magnus Damm 
>>
>> Consider failure of iommu_get_domain_for_dev() as non-critical and
>> get rid of the warning printout. This allows IOMMU properties to be
>> included in the DTB even though the kernel is configured with
>> CONFIG_IOMMU_API=n or in case a particular IOMMU driver refuses to
>> enable IOMMU support for a certain slave device and returns error
>> from the ->add_device() callback.
>>
>> This is only a cosmetic change that removes console warning printouts.
>
> The warning is there for a reason - at this point, we *expected* the
> device to be using an IOMMU for DMA, so a failure is significant. Rather
> than masking genuine failures in other cases because your case
> deliberately breaks that expectation, simply change the expectation -
> i.e. rather than letting of_xlate() succeed then failing add_device()
> later, reject the of_xlate() call up-front such that the DMA layer never
> gets told about the IOMMU in the first place.

Thanks for pointing me in the right direction! I will try to handle
this in xlate() instead.

Cheers,

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

Re: [PATCH v8 9/9] perf/amd/iommu: Enable support for multiple IOMMUs

[Borislav Petkov]

On Wed, Jan 25, 2017 at 10:46:53AM +0100, Peter Zijlstra wrote:
> Which is absolutely insane.

Right,

IMO, the simplest thing to do for your purposes is to embed a struct
amd_iommu pointer into struct perf_amd_iommu at init time so that you
don't have to do all that crazy dance in the PMU functions and iterate
over the iommus in get_amd_iommu() each time.

Which would then simplify all your other functions. For example:

int amd_iommu_pc_get_reg(unsigned int idx, u8 bank, u8 cntr, u8 fxn, u64 *value)

should be

int amd_iommu_pc_get_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, u8 fxn, u64 
*value)

and you can save yourself a lot of glue code and get rid of that
get_amd_iommu() thing.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu