Re: [PATCH v3 1/1] iommu-api: Add map_sg/unmap_sg functions

2014-07-30 Thread Will Deacon 
On Tue, Jul 29, 2014 at 06:21:48PM +0100, Olav Haugan wrote:
 On 7/29/2014 2:25 AM, Will Deacon wrote:
  I agree that we can't handle IOMMUs that have a minimum page size larger
  than the CPU page size, but we should be able to handle the case where the
  maximum supported page size on the IOMMU is smaller than the CPU page size
  (e.g. 4k IOMMU with 64k pages on the CPU). I think that could trip a BUG_ON
  with your patch, although the alignment would be ok in iommu_map because
  page sizes are always a power-of-2. You also end up rounding the size to
  64k, which could lead to mapping more than you really need to.
 
 Which BUG_ON would I trip? If the supported IOMMU page size is less than
 the CPU supported page size then iommu_map will nicely take care of
 splitting up the mapping calls into sizes supported by the IOMMU (taken
 care of by iommu_pgsize()). However, I see you point regarding the
 PAGE_ALIGN of the offset+length that can cause overmapping which you
 don't really want. What is the alternative here? Just leave it and do
 not align at all? That is how iommu_map() currently works. It will
 return error if the iova|phys|size is not aligned to the minimum pgsize
 supported by the IOMMU. So I would not change the behavior if I just
 left it without trying to align.

Yeah, I think losing the align is probably the best bet for now.

 I will remove the BUG_ON for (iova  (~PAGE_MASK)).

Great, that's the BUG_ON I was referring to above.

  (The code in __map_sg_chunk in arch/arm/mm/dma-mapping.c does the same
  thing btw.)
  
  I have the same objection to that code :)
 
 I am hoping we can remove/simplify some of that code when we have the
 iommmu_map_sg API available

Looking forward to it!

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

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Will Deacon 
Hi all,

On Fri, Jul 04, 2014 at 04:29:17PM +0100, Thierry Reding wrote:
 From: Thierry Reding tred...@nvidia.com
 
 This commit introduces a generic device tree binding for IOMMU devices.
 Only a very minimal subset is described here, but it is enough to cover
 the requirements of both the Exynos System MMU and Tegra SMMU as
 discussed here:
 
 https://lkml.org/lkml/2014/4/27/346
 
 Signed-off-by: Thierry Reding tred...@nvidia.com
 ---
 Changes in v4:
 - clarify that disabling an IOMMU DT node may not disable translation
 - be more explicit that examples are only examples
 - add multi-ID master example
 
 Changes in v3:
 - use #iommu-cells instead of #address-cells/#size-cells
 - drop optional iommu-names property
 
 Changes in v2:
 - add notes about dma-ranges property (drop note from commit message)
 - document priorities of iommus property vs. dma-ranges property
 - drop #iommu-cells in favour of #address-cells and #size-cells
 - remove multiple-master device example
 
  Documentation/devicetree/bindings/iommu/iommu.txt | 172 
 ++
  1 file changed, 172 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt

I'm concerned that this patch hasn't been picked up for 3.17 (I can't see it
in -next). If we want to move the ARM SMMU driver over to this new binding,
we can't keep dragging our feet for much longer as I *really* don't plan to
support two bindings in parallel (one is complicated enough already).

Any chance we can see this merged, please?

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

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Thierry Reding 
On Wed, Jul 30, 2014 at 12:04:25PM +0100, Will Deacon wrote:
 Hi all,
 
 On Fri, Jul 04, 2014 at 04:29:17PM +0100, Thierry Reding wrote:
  From: Thierry Reding tred...@nvidia.com
  
  This commit introduces a generic device tree binding for IOMMU devices.
  Only a very minimal subset is described here, but it is enough to cover
  the requirements of both the Exynos System MMU and Tegra SMMU as
  discussed here:
  
  https://lkml.org/lkml/2014/4/27/346
  
  Signed-off-by: Thierry Reding tred...@nvidia.com
  ---
  Changes in v4:
  - clarify that disabling an IOMMU DT node may not disable translation
  - be more explicit that examples are only examples
  - add multi-ID master example
  
  Changes in v3:
  - use #iommu-cells instead of #address-cells/#size-cells
  - drop optional iommu-names property
  
  Changes in v2:
  - add notes about dma-ranges property (drop note from commit message)
  - document priorities of iommus property vs. dma-ranges property
  - drop #iommu-cells in favour of #address-cells and #size-cells
  - remove multiple-master device example
  
   Documentation/devicetree/bindings/iommu/iommu.txt | 172 
  ++
   1 file changed, 172 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt
 
 I'm concerned that this patch hasn't been picked up for 3.17 (I can't see it
 in -next). If we want to move the ARM SMMU driver over to this new binding,
 we can't keep dragging our feet for much longer as I *really* don't plan to
 support two bindings in parallel (one is complicated enough already).
 
 Any chance we can see this merged, please?

I think there weren't any comments left for me to address and I've
mostly been waiting for Joerg to pick it up.

Joerg, can you take this through the iommu tree for 3.17? Will acked
this, but perhaps you were waiting for an ACK from the device tree
bindings maintainers?

Will, perhaps you can get Pawel or Mark to look at this?

Arnd, I'm sure if we had your Acked-by that would go a long way too.

Thierry


pgp8Nj6TKtqxm.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Joerg Roedel 
On Wed, Jul 30, 2014 at 03:23:50PM +0200, Thierry Reding wrote:
 I think there weren't any comments left for me to address and I've
 mostly been waiting for Joerg to pick it up.
 
 Joerg, can you take this through the iommu tree for 3.17? Will acked
 this, but perhaps you were waiting for an ACK from the device tree
 bindings maintainers?
 
 Will, perhaps you can get Pawel or Mark to look at this?
 
 Arnd, I'm sure if we had your Acked-by that would go a long way too.

Yes, as Arnd requested this generic binding it would be good to have his
Acked-by before proceeding. Arnd?


Joerg

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

[PATCH 06/17] ACPICA: Tables: Update for DMAR table changes.

2014-07-30 Thread Lv Zheng 
From: Bob Moore robert.mo...@intel.com

Update table compiler and disassembler for new DMAR fields introduced
in Sept. 2013.
Note that Linux DMAR users need to be updated after applying this change.

[zetalog: changing drivers/iommu/dmar.c accordingly]

Cc: David Woodhouse dw...@infradead.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Bob Moore robert.mo...@intel.com
Signed-off-by: Lv Zheng lv.zh...@intel.com
---
 drivers/iommu/dmar.c  |   28 ++--
 include/acpi/actbl2.h |   14 +++---
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 9a4f05e..bbe33a8 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -84,7 +84,7 @@ void *dmar_alloc_dev_scope(void *start, void *end, int *cnt)
*cnt = 0;
while (start  end) {
scope = start;
-   if (scope-entry_type == ACPI_DMAR_SCOPE_TYPE_ACPI ||
+   if (scope-entry_type == ACPI_DMAR_SCOPE_TYPE_NAMESPACE ||
scope-entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT ||
scope-entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
(*cnt)++;
@@ -380,7 +380,7 @@ static int __init dmar_parse_one_andd(struct 
acpi_dmar_header *header)
struct acpi_dmar_andd *andd = (void *)header;
 
/* Check for NUL termination within the designated length */
-   if (strnlen(andd-object_name, header-length - 8) == header-length - 
8) {
+   if (strnlen(andd-device_name, header-length - 8) == header-length - 
8) {
WARN_TAINT(1, TAINT_FIRMWARE_WORKAROUND,
   Your BIOS is broken; ANDD object name is not 
NUL-terminated\n
   BIOS vendor: %s; Ver: %s; Product Version: %s\n,
@@ -390,7 +390,7 @@ static int __init dmar_parse_one_andd(struct 
acpi_dmar_header *header)
return -EINVAL;
}
pr_info(ANDD device: %x name: %s\n, andd-device_number,
-   andd-object_name);
+   andd-device_name);
 
return 0;
 }
@@ -448,17 +448,17 @@ dmar_table_print_dmar_entry(struct acpi_dmar_header 
*header)
(unsigned long long)rmrr-base_address,
(unsigned long long)rmrr-end_address);
break;
-   case ACPI_DMAR_TYPE_ATSR:
+   case ACPI_DMAR_TYPE_ROOT_ATS:
atsr = container_of(header, struct acpi_dmar_atsr, header);
pr_info(ATSR flags: %#x\n, atsr-flags);
break;
-   case ACPI_DMAR_HARDWARE_AFFINITY:
+   case ACPI_DMAR_TYPE_HARDWARE_AFFINITY:
rhsa = container_of(header, struct acpi_dmar_rhsa, header);
pr_info(RHSA base: %#016Lx proximity domain: %#x\n,
   (unsigned long long)rhsa-base_address,
   rhsa-proximity_domain);
break;
-   case ACPI_DMAR_TYPE_ANDD:
+   case ACPI_DMAR_TYPE_NAMESPACE:
/* We don't print this here because we need to sanity-check
   it first. So print it in dmar_parse_one_andd() instead. */
break;
@@ -539,15 +539,15 @@ parse_dmar_table(void)
case ACPI_DMAR_TYPE_RESERVED_MEMORY:
ret = dmar_parse_one_rmrr(entry_header);
break;
-   case ACPI_DMAR_TYPE_ATSR:
+   case ACPI_DMAR_TYPE_ROOT_ATS:
ret = dmar_parse_one_atsr(entry_header);
break;
-   case ACPI_DMAR_HARDWARE_AFFINITY:
+   case ACPI_DMAR_TYPE_HARDWARE_AFFINITY:
 #ifdef CONFIG_ACPI_NUMA
ret = dmar_parse_one_rhsa(entry_header);
 #endif
break;
-   case ACPI_DMAR_TYPE_ANDD:
+   case ACPI_DMAR_TYPE_NAMESPACE:
ret = dmar_parse_one_andd(entry_header);
break;
default:
@@ -631,7 +631,7 @@ static void __init dmar_acpi_insert_dev_scope(u8 
device_number,
for (scope = (void *)(drhd + 1);
 (unsigned long)scope  ((unsigned long)drhd) + 
drhd-header.length;
 scope = ((void *)scope) + scope-length) {
-   if (scope-entry_type != ACPI_DMAR_SCOPE_TYPE_ACPI)
+   if (scope-entry_type != ACPI_DMAR_SCOPE_TYPE_NAMESPACE)
continue;
if (scope-enumeration_id != device_number)
continue;
@@ -666,21 +666,21 @@ static int __init dmar_acpi_dev_scope_init(void)
for (andd = (void *)dmar_tbl + sizeof(struct acpi_table_dmar);
 ((unsigned long)andd)  ((unsigned long)dmar_tbl) + 
dmar_tbl-length;
 andd = ((void *)andd) + andd-header.length) {
-   if (andd-header.type == ACPI_DMAR_TYPE_ANDD) {
+   if (andd-header.type == ACPI_DMAR_TYPE_NAMESPACE) {

[PATCH 2/4] iommu/amd: Remove change_pte mmu_notifier call-back

2014-07-30 Thread Joerg Roedel 
From: Joerg Roedel jroe...@suse.de

All calls to this call-back are wrapped with
mmu_notifer_invalidate_range_start()/end(), making this
notifier pretty useless, so remove it.

Signed-off-by: Joerg Roedel jroe...@suse.de
Tested-by: Oded Gabbay oded.gab...@amd.com
---
 drivers/iommu/amd_iommu_v2.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 2b7de88..524fd67 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -406,14 +406,6 @@ static int mn_clear_flush_young(struct mmu_notifier *mn,
return 0;
 }
 
-static void mn_change_pte(struct mmu_notifier *mn,
- struct mm_struct *mm,
- unsigned long address,
- pte_t pte)
-{
-   __mn_flush_page(mn, address);
-}
-
 static void mn_invalidate_page(struct mmu_notifier *mn,
   struct mm_struct *mm,
   unsigned long address)
@@ -484,7 +476,6 @@ static void mn_release(struct mmu_notifier *mn, struct 
mm_struct *mm)
 static struct mmu_notifier_ops iommu_mn = {
.release= mn_release,
.clear_flush_young  = mn_clear_flush_young,
-   .change_pte = mn_change_pte,
.invalidate_page= mn_invalidate_page,
.invalidate_range_start = mn_invalidate_range_start,
.invalidate_range_end   = mn_invalidate_range_end,
-- 
1.9.1

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

[PATCH 3/4] iommu/amd: Fix device_state reference counting

2014-07-30 Thread Joerg Roedel 
From: Joerg Roedel jroe...@suse.de

The references to the device state are not dropped
everywhere. This might cause a dead-lock in
amd_iommu_free_device(). Fix it.

Signed-off-by: Joerg Roedel jroe...@suse.de
Tested-by: Oded Gabbay oded.gab...@amd.com
---
 drivers/iommu/amd_iommu_v2.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 524fd67..2de13be 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -368,6 +368,9 @@ static void free_pasid_states(struct device_state 
*dev_state)
 
put_pasid_state_wait(pasid_state); /* Reference taken in
  amd_iommu_pasid_bind */
+
+   /* Drop reference taken in amd_iommu_bind_pasid */
+   put_device_state(dev_state);
}
 
if (dev_state-pasid_levels == 2)
@@ -748,6 +751,10 @@ void amd_iommu_unbind_pasid(struct pci_dev *pdev, int 
pasid)
put_pasid_state_wait(pasid_state); /* Reference taken in
  amd_iommu_pasid_bind */
 out:
+   /* Drop reference taken in this function */
+   put_device_state(dev_state);
+
+   /* Drop reference taken in amd_iommu_bind_pasid */
put_device_state(dev_state);
 }
 EXPORT_SYMBOL(amd_iommu_unbind_pasid);
-- 
1.9.1

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

[PATCH 4/4] iommu/amd: Fix 2 typos in comments

2014-07-30 Thread Joerg Roedel 
From: Joerg Roedel jroe...@suse.de

amd_iommu_pasid_bind - amd_iommu_bind_pasid

Signed-off-by: Joerg Roedel jroe...@suse.de
---
 drivers/iommu/amd_iommu_v2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 2de13be..5f578e8 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -367,7 +367,7 @@ static void free_pasid_states(struct device_state 
*dev_state)
mmu_notifier_unregister(pasid_state-mn, pasid_state-mm);
 
put_pasid_state_wait(pasid_state); /* Reference taken in
- amd_iommu_pasid_bind */
+ amd_iommu_bind_pasid */
 
/* Drop reference taken in amd_iommu_bind_pasid */
put_device_state(dev_state);
@@ -749,7 +749,7 @@ void amd_iommu_unbind_pasid(struct pci_dev *pdev, int pasid)
mmu_notifier_unregister(pasid_state-mn, pasid_state-mm);
 
put_pasid_state_wait(pasid_state); /* Reference taken in
- amd_iommu_pasid_bind */
+ amd_iommu_bind_pasid */
 out:
/* Drop reference taken in this function */
put_device_state(dev_state);
-- 
1.9.1

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

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Will Deacon 
On Wed, Jul 30, 2014 at 02:23:50PM +0100, Thierry Reding wrote:
 On Wed, Jul 30, 2014 at 12:04:25PM +0100, Will Deacon wrote:
  On Fri, Jul 04, 2014 at 04:29:17PM +0100, Thierry Reding wrote:
   From: Thierry Reding tred...@nvidia.com
   
   This commit introduces a generic device tree binding for IOMMU devices.
   Only a very minimal subset is described here, but it is enough to cover
   the requirements of both the Exynos System MMU and Tegra SMMU as
   discussed here:
   
   https://lkml.org/lkml/2014/4/27/346
   
   Signed-off-by: Thierry Reding tred...@nvidia.com
   ---
   Changes in v4:
   - clarify that disabling an IOMMU DT node may not disable translation
   - be more explicit that examples are only examples
   - add multi-ID master example
   
   Changes in v3:
   - use #iommu-cells instead of #address-cells/#size-cells
   - drop optional iommu-names property
   
   Changes in v2:
   - add notes about dma-ranges property (drop note from commit message)
   - document priorities of iommus property vs. dma-ranges property
   - drop #iommu-cells in favour of #address-cells and #size-cells
   - remove multiple-master device example
   
Documentation/devicetree/bindings/iommu/iommu.txt | 172 
   ++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt
  
  I'm concerned that this patch hasn't been picked up for 3.17 (I can't see it
  in -next). If we want to move the ARM SMMU driver over to this new binding,
  we can't keep dragging our feet for much longer as I *really* don't plan to
  support two bindings in parallel (one is complicated enough already).
  
  Any chance we can see this merged, please?
 
 I think there weren't any comments left for me to address and I've
 mostly been waiting for Joerg to pick it up.
 
 Joerg, can you take this through the iommu tree for 3.17? Will acked
 this, but perhaps you were waiting for an ACK from the device tree
 bindings maintainers?

Rob, Mark: can one or both of you take a look at this please?

Cheers,

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

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Mark Rutland 
Hi Thierry,

This looks sane to me.

I just have a few questions below which are hopefully simple/stupid.

On Fri, Jul 04, 2014 at 04:29:17PM +0100, Thierry Reding wrote:
 From: Thierry Reding tred...@nvidia.com
 
 This commit introduces a generic device tree binding for IOMMU devices.
 Only a very minimal subset is described here, but it is enough to cover
 the requirements of both the Exynos System MMU and Tegra SMMU as
 discussed here:
 
 https://lkml.org/lkml/2014/4/27/346
 
 Signed-off-by: Thierry Reding tred...@nvidia.com
 ---
 Changes in v4:
 - clarify that disabling an IOMMU DT node may not disable translation
 - be more explicit that examples are only examples
 - add multi-ID master example
 
 Changes in v3:
 - use #iommu-cells instead of #address-cells/#size-cells
 - drop optional iommu-names property
 
 Changes in v2:
 - add notes about dma-ranges property (drop note from commit message)
 - document priorities of iommus property vs. dma-ranges property
 - drop #iommu-cells in favour of #address-cells and #size-cells
 - remove multiple-master device example
 
  Documentation/devicetree/bindings/iommu/iommu.txt | 172 
 ++
  1 file changed, 172 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt
 
 diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt 
 b/Documentation/devicetree/bindings/iommu/iommu.txt
 new file mode 100644
 index ..464a81eaaf61
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/iommu/iommu.txt
 @@ -0,0 +1,172 @@
 +This document describes the generic device tree binding for IOMMUs and their
 +master(s).
 +
 +
 +IOMMU device node:
 +==
 +
 +An IOMMU can provide the following services:
 +
 +* Remap address space to allow devices to access physical memory ranges that
 +  they otherwise wouldn't be capable of accessing.
 +
 +  Example: 32-bit DMA to 64-bit physical addresses
 +
 +* Implement scatter-gather at page level granularity so that the device does
 +  not have to.
 +
 +* Provide system protection against rogue DMA by forcing all accesses to go
 +  through the IOMMU and faulting when encountering accesses to unmapped
 +  address regions.
 +
 +* Provide address space isolation between multiple contexts.
 +
 +  Example: Virtualization
 +
 +Device nodes compatible with this binding represent hardware with some of the
 +above capabilities.
 +
 +IOMMUs can be single-master or multiple-master. Single-master IOMMU devices
 +typically have a fixed association to the master device, whereas multiple-
 +master IOMMU devices can translate accesses from more than one master.
 +
 +The device tree node of the IOMMU device's parent bus must contain a valid
 +dma-ranges property that describes how the physical address space of the
 +IOMMU maps to memory. An empty dma-ranges property means that there is a
 +1:1 mapping from IOMMU to memory.
 +
 +Required properties:
 +
 +- #iommu-cells: The number of cells in an IOMMU specifier needed to encode an
 +  address.
 +
 +The meaning of the IOMMU specifier is defined by the device tree binding of
 +the specific IOMMU. Below are a few examples of typical use-cases:
 +
 +- #iommu-cells = 0: Single master IOMMU devices are not configurable and
 +  therefore no additional information needs to be encoded in the specifier.
 +  This may also apply to multiple master IOMMU devices that do not allow the
 +  association of masters to be configured. Note that an IOMMU can by design
 +  be multi-master yet only expose a single master in a given configuration.
 +  In such cases the number of cells will usually be 1 as in the next case.
 +- #iommu-cells = 1: Multiple master IOMMU devices may need to be configured
 +  in order to enable translation for a given master. In such cases the single
 +  address cell corresponds to the master device's ID. In some cases more than
 +  one cell can be required to represent a single master ID.
 +- #iommu-cells = 4: Some IOMMU devices allow the DMA window for masters to
 +  be configured. The first cell of the address in this may contain the master
 +  device's ID for example, while the second cell could contain the start of
 +  the DMA window for the given device. The length of the DMA window is given
 +  by the third and fourth cells.
 +
 +Note that these are merely examples and real-world use-cases may use 
 different
 +definitions to represent their individual needs. Always refer to the specific
 +IOMMU binding for the exact meaning of the cells that make up the specifier.
 +
 +
 +IOMMU master node:
 +==
 +
 +Devices that access memory through an IOMMU are called masters. A device can
 +have multiple master interfaces (to one or more IOMMU devices).
 +
 +Required properties:
 +
 +- iommus: A list of phandle and IOMMU specifier pairs that describe the IOMMU
 +  master interfaces of the device. One entry in the list describes one master
 +  interface of the device.
 +

Re: [PATCH v3] iommu/arm-smmu: avoid calling request_irq in atomic context

2014-07-30 Thread Will Deacon 
Hey Mitch,

On Tue, Jul 29, 2014 at 07:11:15PM +0100, Mitchel Humpherys wrote:
 request_irq shouldn't be called from atomic context since it might
 sleep, but we're calling it with a spinlock held, resulting in:
 
 [9.172202] BUG: sleeping function called from invalid context at 
 kernel/mm/slub.c:926
 [9.182989] in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: 
 swapper/0
 [9.189762] CPU: 1 PID: 1 Comm: swapper/0 Tainted: GW
 3.10.40-gbc1b510b-38437-g55831d3bd9-dirty #97
 [9.199757] [c020c448] (unwind_backtrace+0x0/0x11c) from 
 [c02097d0] (show_stack+0x10/0x14)
 [9.208346] [c02097d0] (show_stack+0x10/0x14) from [c0301d74] 
 (kmem_cache_alloc_trace+0x3c/0x210)
 [9.217543] [c0301d74] (kmem_cache_alloc_trace+0x3c/0x210) from 
 [c0276a48] (request_threaded_irq+0x88/0x11c)
 [9.227702] [c0276a48] (request_threaded_irq+0x88/0x11c) from 
 [c0931ca4] (arm_smmu_attach_dev+0x188/0x858)
 [9.237686] [c0931ca4] (arm_smmu_attach_dev+0x188/0x858) from 
 [c0212cd8] (arm_iommu_attach_device+0x18/0xd0)
 [9.247837] [c0212cd8] (arm_iommu_attach_device+0x18/0xd0) from 
 [c093314c] (arm_smmu_test_probe+0x68/0xd4)
 [9.257823] [c093314c] (arm_smmu_test_probe+0x68/0xd4) from 
 [c05aadd0] (driver_probe_device+0x12c/0x330)
 [9.267629] [c05aadd0] (driver_probe_device+0x12c/0x330) from 
 [c05ab080] (__driver_attach+0x68/0x8c)
 [9.277090] [c05ab080] (__driver_attach+0x68/0x8c) from [c05a92d4] 
 (bus_for_each_dev+0x70/0x84)
 [9.286118] [c05a92d4] (bus_for_each_dev+0x70/0x84) from 
 [c05aa3b0] (bus_add_driver+0x100/0x244)
 [9.295233] [c05aa3b0] (bus_add_driver+0x100/0x244) from 
 [c05ab5d0] (driver_register+0x9c/0x124)
 [9.304347] [c05ab5d0] (driver_register+0x9c/0x124) from 
 [c0933088] (arm_smmu_test_init+0x14/0x38)
 [9.313635] [c0933088] (arm_smmu_test_init+0x14/0x38) from 
 [c0200618] (do_one_initcall+0xb8/0x160)
 [9.322926] [c0200618] (do_one_initcall+0xb8/0x160) from 
 [c1200b7c] (kernel_init_freeable+0x108/0x1cc)
 [9.332564] [c1200b7c] (kernel_init_freeable+0x108/0x1cc) from 
 [c0b924b0] (kernel_init+0xc/0xe4)
 [9.341675] [c0b924b0] (kernel_init+0xc/0xe4) from [c0205e38] 
 (ret_from_fork+0x14/0x3c)
 
 Fix this by moving the request_irq out of the critical section. This
 should be okay since smmu_domain-smmu is still being protected by the
 critical section. Also, we still don't program the Stream Match Register
 until after registering our interrupt handler so we shouldn't be missing
 any interrupts.
 
 Signed-off-by: Mitchel Humpherys mitch...@codeaurora.org
 ---
 Changelog:
 
   - v3: rework irq request code to avoid requesting the irq every
 time a master is added to the domain
   - v2: return error code from request_irq on failure
 ---
  drivers/iommu/arm-smmu.c | 73 
 +++-
  1 file changed, 41 insertions(+), 32 deletions(-)

I think this is correct, but we can do some cleanup now that you've moved
all the locking into the conditional. Messy diff below, which would be much
nicer sqaushed into your patch.

What do you reckon?

Will

---8

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 572f5579d38b..e33df1a676ec 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -868,10 +868,15 @@ static void arm_smmu_init_context_bank(struct 
arm_smmu_domain *smmu_domain)
 static int arm_smmu_init_domain_context(struct iommu_domain *domain,
struct arm_smmu_device *smmu)
 {
-   int ret, start;
+   int irq, start, ret = 0;
+   unsigned long flags;
struct arm_smmu_domain *smmu_domain = domain-priv;
struct arm_smmu_cfg *cfg = smmu_domain-cfg;
 
+   spin_lock_irqsave(smmu_domain-lock, flags);
+   if (smmu_domain-smmu)
+   goto out_unlock;
+
if (smmu-features  ARM_SMMU_FEAT_TRANS_NESTED) {
/*
 * We will likely want to change this if/when KVM gets
@@ -890,7 +895,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain 
*domain,
ret = __arm_smmu_alloc_bitmap(smmu-context_map, start,
  smmu-num_context_banks);
if (IS_ERR_VALUE(ret))
-   return ret;
+   goto out_unlock;
 
cfg-cbndx = ret;
if (smmu-version == 1) {
@@ -902,7 +907,22 @@ static int arm_smmu_init_domain_context(struct 
iommu_domain *domain,
 
ACCESS_ONCE(smmu_domain-smmu) = smmu;
arm_smmu_init_context_bank(smmu_domain);
+   spin_unlock_irqrestore(smmu_domain-lock, flags);
+
+   irq = smmu-irqs[smmu-num_global_irqs + cfg-irptndx];
+   ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
+ arm-smmu-context-fault, smmu_domain);
+   if (IS_ERR_VALUE(ret)) {
+   dev_err(smmu-dev, failed to request context IRQ %d (%u)\n,
+

Re: [PATCH v3] iommu/arm-smmu: avoid calling request_irq in atomic context

2014-07-30 Thread Mitchel Humpherys 
On Wed, Jul 30 2014 at 08:31:14 AM, Will Deacon will.dea...@arm.com wrote:
 Hey Mitch,

 On Tue, Jul 29, 2014 at 07:11:15PM +0100, Mitchel Humpherys wrote:
 request_irq shouldn't be called from atomic context since it might
 sleep, but we're calling it with a spinlock held, resulting in:
 
 [9.172202] BUG: sleeping function called from invalid context at 
 kernel/mm/slub.c:926
 [9.182989] in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: 
 swapper/0
 [9.189762] CPU: 1 PID: 1 Comm: swapper/0 Tainted: GW
 3.10.40-gbc1b510b-38437-g55831d3bd9-dirty #97
 [9.199757] [c020c448] (unwind_backtrace+0x0/0x11c) from 
 [c02097d0] (show_stack+0x10/0x14)
 [9.208346] [c02097d0] (show_stack+0x10/0x14) from [c0301d74] 
 (kmem_cache_alloc_trace+0x3c/0x210)
 [9.217543] [c0301d74] (kmem_cache_alloc_trace+0x3c/0x210) from 
 [c0276a48] (request_threaded_irq+0x88/0x11c)
 [9.227702] [c0276a48] (request_threaded_irq+0x88/0x11c) from 
 [c0931ca4] (arm_smmu_attach_dev+0x188/0x858)
 [9.237686] [c0931ca4] (arm_smmu_attach_dev+0x188/0x858) from 
 [c0212cd8] (arm_iommu_attach_device+0x18/0xd0)
 [9.247837] [c0212cd8] (arm_iommu_attach_device+0x18/0xd0) from 
 [c093314c] (arm_smmu_test_probe+0x68/0xd4)
 [9.257823] [c093314c] (arm_smmu_test_probe+0x68/0xd4) from 
 [c05aadd0] (driver_probe_device+0x12c/0x330)
 [9.267629] [c05aadd0] (driver_probe_device+0x12c/0x330) from 
 [c05ab080] (__driver_attach+0x68/0x8c)
 [9.277090] [c05ab080] (__driver_attach+0x68/0x8c) from 
 [c05a92d4] (bus_for_each_dev+0x70/0x84)
 [9.286118] [c05a92d4] (bus_for_each_dev+0x70/0x84) from 
 [c05aa3b0] (bus_add_driver+0x100/0x244)
 [9.295233] [c05aa3b0] (bus_add_driver+0x100/0x244) from 
 [c05ab5d0] (driver_register+0x9c/0x124)
 [9.304347] [c05ab5d0] (driver_register+0x9c/0x124) from 
 [c0933088] (arm_smmu_test_init+0x14/0x38)
 [9.313635] [c0933088] (arm_smmu_test_init+0x14/0x38) from 
 [c0200618] (do_one_initcall+0xb8/0x160)
 [9.322926] [c0200618] (do_one_initcall+0xb8/0x160) from 
 [c1200b7c] (kernel_init_freeable+0x108/0x1cc)
 [9.332564] [c1200b7c] (kernel_init_freeable+0x108/0x1cc) from 
 [c0b924b0] (kernel_init+0xc/0xe4)
 [9.341675] [c0b924b0] (kernel_init+0xc/0xe4) from [c0205e38] 
 (ret_from_fork+0x14/0x3c)
 
 Fix this by moving the request_irq out of the critical section. This
 should be okay since smmu_domain-smmu is still being protected by the
 critical section. Also, we still don't program the Stream Match Register
 until after registering our interrupt handler so we shouldn't be missing
 any interrupts.
 
 Signed-off-by: Mitchel Humpherys mitch...@codeaurora.org
 ---
 Changelog:
 
   - v3: rework irq request code to avoid requesting the irq every
 time a master is added to the domain
   - v2: return error code from request_irq on failure
 ---
  drivers/iommu/arm-smmu.c | 73 
 +++-
  1 file changed, 41 insertions(+), 32 deletions(-)

 I think this is correct, but we can do some cleanup now that you've moved
 all the locking into the conditional. Messy diff below, which would be much
 nicer sqaushed into your patch.

 What do you reckon?

Much cleaner, thanks. Just one question below.


 Will

 ---8

 diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
 index 572f5579d38b..e33df1a676ec 100644
 --- a/drivers/iommu/arm-smmu.c
 +++ b/drivers/iommu/arm-smmu.c
 @@ -868,10 +868,15 @@ static void arm_smmu_init_context_bank(struct 
 arm_smmu_domain *smmu_domain)
  static int arm_smmu_init_domain_context(struct iommu_domain *domain,
   struct arm_smmu_device *smmu)
  {
 - int ret, start;
 + int irq, start, ret = 0;
 + unsigned long flags;
   struct arm_smmu_domain *smmu_domain = domain-priv;
   struct arm_smmu_cfg *cfg = smmu_domain-cfg;
  
 + spin_lock_irqsave(smmu_domain-lock, flags);
 + if (smmu_domain-smmu)
 + goto out_unlock;
 +
   if (smmu-features  ARM_SMMU_FEAT_TRANS_NESTED) {
   /*
* We will likely want to change this if/when KVM gets
 @@ -890,7 +895,7 @@ static int arm_smmu_init_domain_context(struct 
 iommu_domain *domain,
   ret = __arm_smmu_alloc_bitmap(smmu-context_map, start,
 smmu-num_context_banks);
   if (IS_ERR_VALUE(ret))
 - return ret;
 + goto out_unlock;
  
   cfg-cbndx = ret;
   if (smmu-version == 1) {
 @@ -902,7 +907,22 @@ static int arm_smmu_init_domain_context(struct 
 iommu_domain *domain,
  
   ACCESS_ONCE(smmu_domain-smmu) = smmu;
   arm_smmu_init_context_bank(smmu_domain);
 + spin_unlock_irqrestore(smmu_domain-lock, flags);
 +
 + irq = smmu-irqs[smmu-num_global_irqs + cfg-irptndx];
 + ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
 +   arm-smmu-context-fault, smmu_domain);
 +

Re: [PATCH v3] iommu/arm-smmu: avoid calling request_irq in atomic context

2014-07-30 Thread Will Deacon 
On Wed, Jul 30, 2014 at 05:51:48PM +0100, Mitchel Humpherys wrote:
 On Wed, Jul 30 2014 at 08:31:14 AM, Will Deacon will.dea...@arm.com wrote:
  On Tue, Jul 29, 2014 at 07:11:15PM +0100, Mitchel Humpherys wrote:
  Changelog:
  
- v3: rework irq request code to avoid requesting the irq every
  time a master is added to the domain
- v2: return error code from request_irq on failure
  ---
   drivers/iommu/arm-smmu.c | 73 
  +++-
   1 file changed, 41 insertions(+), 32 deletions(-)
 
  I think this is correct, but we can do some cleanup now that you've moved
  all the locking into the conditional. Messy diff below, which would be much
  nicer sqaushed into your patch.
 
  What do you reckon?
 
 Much cleaner, thanks. Just one question below.

[...]

  @@ -902,7 +907,22 @@ static int arm_smmu_init_domain_context(struct 
  iommu_domain *domain,
   
  ACCESS_ONCE(smmu_domain-smmu) = smmu;
  arm_smmu_init_context_bank(smmu_domain);
  +   spin_unlock_irqrestore(smmu_domain-lock, flags);
  +
  +   irq = smmu-irqs[smmu-num_global_irqs + cfg-irptndx];
  +   ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
  + arm-smmu-context-fault, smmu_domain);
  +   if (IS_ERR_VALUE(ret)) {
  +   dev_err(smmu-dev, failed to request context IRQ %d (%u)\n,
  +   cfg-irptndx, irq);
  +   cfg-irptndx = INVALID_IRPTNDX;
 
 We want to return ret here due to the request_irq failure, right?

Actually, no, and it's a subtle change introduced by this fix. Before, we
would partially tear-down the domain here by freeing the cbndx but actually,
that's racy as hell. The moment we drop the lock another device can attach
successfully to the domain we've set, so we can't safely tear things down at
that point. The best bet is to continue with the warning -- you end up with
a domain without a context interrupt, but that's not fatal to the driver.

If we returned an error, I can't think of a safe way to reset the domain.

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

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Olof Johansson 
Hi,

On Wed, Jul 30, 2014 at 8:26 AM, Mark Rutland mark.rutl...@arm.com wrote:
 Hi Thierry,

 This looks sane to me.

 I just have a few questions below which are hopefully simple/stupid.

 On Fri, Jul 04, 2014 at 04:29:17PM +0100, Thierry Reding wrote:
 From: Thierry Reding tred...@nvidia.com

 This commit introduces a generic device tree binding for IOMMU devices.
 Only a very minimal subset is described here, but it is enough to cover
 the requirements of both the Exynos System MMU and Tegra SMMU as
 discussed here:

 https://lkml.org/lkml/2014/4/27/346

 Signed-off-by: Thierry Reding tred...@nvidia.com
 ---
 Changes in v4:
 - clarify that disabling an IOMMU DT node may not disable translation
 - be more explicit that examples are only examples
 - add multi-ID master example

 Changes in v3:
 - use #iommu-cells instead of #address-cells/#size-cells
 - drop optional iommu-names property

 Changes in v2:
 - add notes about dma-ranges property (drop note from commit message)
 - document priorities of iommus property vs. dma-ranges property
 - drop #iommu-cells in favour of #address-cells and #size-cells
 - remove multiple-master device example

  Documentation/devicetree/bindings/iommu/iommu.txt | 172 
 ++
  1 file changed, 172 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt

 diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt 
 b/Documentation/devicetree/bindings/iommu/iommu.txt
 new file mode 100644
 index ..464a81eaaf61
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/iommu/iommu.txt
 @@ -0,0 +1,172 @@
 +This document describes the generic device tree binding for IOMMUs and their
 +master(s).
 +
 +
 +IOMMU device node:
 +==
 +
 +An IOMMU can provide the following services:
 +
 +* Remap address space to allow devices to access physical memory ranges that
 +  they otherwise wouldn't be capable of accessing.
 +
 +  Example: 32-bit DMA to 64-bit physical addresses
 +
 +* Implement scatter-gather at page level granularity so that the device does
 +  not have to.
 +
 +* Provide system protection against rogue DMA by forcing all accesses to 
 go
 +  through the IOMMU and faulting when encountering accesses to unmapped
 +  address regions.
 +
 +* Provide address space isolation between multiple contexts.
 +
 +  Example: Virtualization
 +
 +Device nodes compatible with this binding represent hardware with some of 
 the
 +above capabilities.
 +
 +IOMMUs can be single-master or multiple-master. Single-master IOMMU devices
 +typically have a fixed association to the master device, whereas multiple-
 +master IOMMU devices can translate accesses from more than one master.
 +
 +The device tree node of the IOMMU device's parent bus must contain a valid
 +dma-ranges property that describes how the physical address space of the
 +IOMMU maps to memory. An empty dma-ranges property means that there is a
 +1:1 mapping from IOMMU to memory.
 +
 +Required properties:
 +
 +- #iommu-cells: The number of cells in an IOMMU specifier needed to encode 
 an
 +  address.
 +
 +The meaning of the IOMMU specifier is defined by the device tree binding of
 +the specific IOMMU. Below are a few examples of typical use-cases:
 +
 +- #iommu-cells = 0: Single master IOMMU devices are not configurable and
 +  therefore no additional information needs to be encoded in the specifier.
 +  This may also apply to multiple master IOMMU devices that do not allow the
 +  association of masters to be configured. Note that an IOMMU can by design
 +  be multi-master yet only expose a single master in a given configuration.
 +  In such cases the number of cells will usually be 1 as in the next case.
 +- #iommu-cells = 1: Multiple master IOMMU devices may need to be 
 configured
 +  in order to enable translation for a given master. In such cases the 
 single
 +  address cell corresponds to the master device's ID. In some cases more 
 than
 +  one cell can be required to represent a single master ID.
 +- #iommu-cells = 4: Some IOMMU devices allow the DMA window for masters to
 +  be configured. The first cell of the address in this may contain the 
 master
 +  device's ID for example, while the second cell could contain the start of
 +  the DMA window for the given device. The length of the DMA window is given
 +  by the third and fourth cells.
 +
 +Note that these are merely examples and real-world use-cases may use 
 different
 +definitions to represent their individual needs. Always refer to the 
 specific
 +IOMMU binding for the exact meaning of the cells that make up the specifier.
 +
 +
 +IOMMU master node:
 +==
 +
 +Devices that access memory through an IOMMU are called masters. A device can
 +have multiple master interfaces (to one or more IOMMU devices).
 +
 +Required properties:
 +
 +- iommus: A list of phandle and IOMMU specifier pairs that describe the 
 IOMMU
 +  master interfaces

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Olof Johansson 
On Wed, Jul 30, 2014 at 6:33 AM, Joerg Roedel j...@8bytes.org wrote:
 On Wed, Jul 30, 2014 at 03:23:50PM +0200, Thierry Reding wrote:
 I think there weren't any comments left for me to address and I've
 mostly been waiting for Joerg to pick it up.

 Joerg, can you take this through the iommu tree for 3.17? Will acked
 this, but perhaps you were waiting for an ACK from the device tree
 bindings maintainers?

 Will, perhaps you can get Pawel or Mark to look at this?

 Arnd, I'm sure if we had your Acked-by that would go a long way too.

 Yes, as Arnd requested this generic binding it would be good to have his
 Acked-by before proceeding. Arnd?

Arnd is on vacation now, unfortunately. I've read up on the history
and the current proposal from Thierry looks sane to me. As discussed,
there might be a need for some common helpers to walk the tree and
figure out things.

There's also the possibility that the IOMMU at init time actually
modifies and configures the stream IDs for the clients, but that again
is an implementation detail and not something that affects the binding
per se.

I'm not giving my Ack now since I want to make sure there are no
disagreements on my separate reply from a minute ago, but I expect
you'll have it once we've had that round trip of comments. :)


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

[PATCH v4] iommu/arm-smmu: avoid calling request_irq in atomic context

2014-07-30 Thread Mitchel Humpherys 
request_irq shouldn't be called from atomic context since it might
sleep, but we're calling it with a spinlock held, resulting in:

[9.172202] BUG: sleeping function called from invalid context at 
kernel/mm/slub.c:926
[9.182989] in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0
[9.189762] CPU: 1 PID: 1 Comm: swapper/0 Tainted: GW
3.10.40-gbc1b510b-38437-g55831d3bd9-dirty #97
[9.199757] [c020c448] (unwind_backtrace+0x0/0x11c) from [c02097d0] 
(show_stack+0x10/0x14)
[9.208346] [c02097d0] (show_stack+0x10/0x14) from [c0301d74] 
(kmem_cache_alloc_trace+0x3c/0x210)
[9.217543] [c0301d74] (kmem_cache_alloc_trace+0x3c/0x210) from 
[c0276a48] (request_threaded_irq+0x88/0x11c)
[9.227702] [c0276a48] (request_threaded_irq+0x88/0x11c) from 
[c0931ca4] (arm_smmu_attach_dev+0x188/0x858)
[9.237686] [c0931ca4] (arm_smmu_attach_dev+0x188/0x858) from 
[c0212cd8] (arm_iommu_attach_device+0x18/0xd0)
[9.247837] [c0212cd8] (arm_iommu_attach_device+0x18/0xd0) from 
[c093314c] (arm_smmu_test_probe+0x68/0xd4)
[9.257823] [c093314c] (arm_smmu_test_probe+0x68/0xd4) from 
[c05aadd0] (driver_probe_device+0x12c/0x330)
[9.267629] [c05aadd0] (driver_probe_device+0x12c/0x330) from 
[c05ab080] (__driver_attach+0x68/0x8c)
[9.277090] [c05ab080] (__driver_attach+0x68/0x8c) from [c05a92d4] 
(bus_for_each_dev+0x70/0x84)
[9.286118] [c05a92d4] (bus_for_each_dev+0x70/0x84) from [c05aa3b0] 
(bus_add_driver+0x100/0x244)
[9.295233] [c05aa3b0] (bus_add_driver+0x100/0x244) from [c05ab5d0] 
(driver_register+0x9c/0x124)
[9.304347] [c05ab5d0] (driver_register+0x9c/0x124) from [c0933088] 
(arm_smmu_test_init+0x14/0x38)
[9.313635] [c0933088] (arm_smmu_test_init+0x14/0x38) from 
[c0200618] (do_one_initcall+0xb8/0x160)
[9.322926] [c0200618] (do_one_initcall+0xb8/0x160) from [c1200b7c] 
(kernel_init_freeable+0x108/0x1cc)
[9.332564] [c1200b7c] (kernel_init_freeable+0x108/0x1cc) from 
[c0b924b0] (kernel_init+0xc/0xe4)
[9.341675] [c0b924b0] (kernel_init+0xc/0xe4) from [c0205e38] 
(ret_from_fork+0x14/0x3c)

Fix this by moving the request_irq out of the critical section. This
should be okay since smmu_domain-smmu is still being protected by the
critical section. Also, we still don't program the Stream Match Register
until after registering our interrupt handler so we shouldn't be missing
any interrupts.

Signed-off-by: Mitchel Humpherys mitch...@codeaurora.org
---
Changelog:

  - v4: some cleanup suggested by Will
  - v3: rework irq request code to avoid requesting the irq every
time a master is added to the domain
  - v2: return error code from request_irq on failure
---
 drivers/iommu/arm-smmu.c | 49 +---
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f3f66416e2..e33df1a676 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -868,10 +868,15 @@ static void arm_smmu_init_context_bank(struct 
arm_smmu_domain *smmu_domain)
 static int arm_smmu_init_domain_context(struct iommu_domain *domain,
struct arm_smmu_device *smmu)
 {
-   int irq, ret, start;
+   int irq, start, ret = 0;
+   unsigned long flags;
struct arm_smmu_domain *smmu_domain = domain-priv;
struct arm_smmu_cfg *cfg = smmu_domain-cfg;
 
+   spin_lock_irqsave(smmu_domain-lock, flags);
+   if (smmu_domain-smmu)
+   goto out_unlock;
+
if (smmu-features  ARM_SMMU_FEAT_TRANS_NESTED) {
/*
 * We will likely want to change this if/when KVM gets
@@ -890,7 +895,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain 
*domain,
ret = __arm_smmu_alloc_bitmap(smmu-context_map, start,
  smmu-num_context_banks);
if (IS_ERR_VALUE(ret))
-   return ret;
+   goto out_unlock;
 
cfg-cbndx = ret;
if (smmu-version == 1) {
@@ -900,22 +905,23 @@ static int arm_smmu_init_domain_context(struct 
iommu_domain *domain,
cfg-irptndx = cfg-cbndx;
}
 
+   ACCESS_ONCE(smmu_domain-smmu) = smmu;
+   arm_smmu_init_context_bank(smmu_domain);
+   spin_unlock_irqrestore(smmu_domain-lock, flags);
+
irq = smmu-irqs[smmu-num_global_irqs + cfg-irptndx];
ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
- arm-smmu-context-fault, domain);
+ arm-smmu-context-fault, smmu_domain);
if (IS_ERR_VALUE(ret)) {
dev_err(smmu-dev, failed to request context IRQ %d (%u)\n,
cfg-irptndx, irq);
cfg-irptndx = INVALID_IRPTNDX;
-   goto out_free_context;
}
 
-   smmu_domain-smmu = smmu;
-   arm_smmu_init_context_bank(smmu_domain);

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Rob Herring 
On Wed, Jul 30, 2014 at 9:30 AM, Will Deacon will.dea...@arm.com wrote:
 On Wed, Jul 30, 2014 at 02:23:50PM +0100, Thierry Reding wrote:
 On Wed, Jul 30, 2014 at 12:04:25PM +0100, Will Deacon wrote:
  On Fri, Jul 04, 2014 at 04:29:17PM +0100, Thierry Reding wrote:
   From: Thierry Reding tred...@nvidia.com
  
   This commit introduces a generic device tree binding for IOMMU devices.
   Only a very minimal subset is described here, but it is enough to cover
   the requirements of both the Exynos System MMU and Tegra SMMU as
   discussed here:
  
   https://lkml.org/lkml/2014/4/27/346
  
   Signed-off-by: Thierry Reding tred...@nvidia.com
   ---
   Changes in v4:
   - clarify that disabling an IOMMU DT node may not disable translation
   - be more explicit that examples are only examples
   - add multi-ID master example
  
   Changes in v3:
   - use #iommu-cells instead of #address-cells/#size-cells
   - drop optional iommu-names property
  
   Changes in v2:
   - add notes about dma-ranges property (drop note from commit message)
   - document priorities of iommus property vs. dma-ranges property
   - drop #iommu-cells in favour of #address-cells and #size-cells
   - remove multiple-master device example
  
Documentation/devicetree/bindings/iommu/iommu.txt | 172 
   ++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/iommu.txt
 
  I'm concerned that this patch hasn't been picked up for 3.17 (I can't see 
  it
  in -next). If we want to move the ARM SMMU driver over to this new binding,
  we can't keep dragging our feet for much longer as I *really* don't plan to
  support two bindings in parallel (one is complicated enough already).
 
  Any chance we can see this merged, please?

 I think there weren't any comments left for me to address and I've
 mostly been waiting for Joerg to pick it up.

 Joerg, can you take this through the iommu tree for 3.17? Will acked
 this, but perhaps you were waiting for an ACK from the device tree
 bindings maintainers?

 Rob, Mark: can one or both of you take a look at this please?

I've been quiet on this round, but I think prior input I've had has
been addressed. If we believe this will work for ARM SMMU and MSM
IOMMU and some of the crazy chaining scenarios, then I'm fine with the
binding.

Acked-by: Rob Herring r...@kernel.org

Rob

P.S. Thankfully, there are no Calxeda systems with the SMMU enabled,
so a binding change should not cause much pain.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Mark Rutland 
[...]

  +Multiple-master IOMMU:
  +--
  +
  + iommu {
  + /* the specifier represents the ID of the master */
  + #iommu-cells = 1;
  + };
  +
  + master@1 {
  + /* device has master ID 42 in the IOMMU */
  + iommus = /iommu 42;
  + };
  +
  + master@2 {
  + /* device has master IDs 23 and 24 in the IOMMU */
  + iommus = /iommu 23, /iommu 24;
  + };
 
  In future I suspect master will need to be able to identify which master
  IDs correspond to which of their master ports (where each port might
  have an arbitrary number of master IDs).
 
  While we don't need that for the first run, it would be nice to have
  that looked into so master bindings don't come up with arbitrarily
  different ways of doing that.
 
 iommu-names would be the logical extension to handle that, just like
 we do with other resources, right?

Possibly. If the master has multiple IDs assigned to transactions from a
single master port then it depends on how the master wants to group
those for the sake of the binding. If it's per-port then you'd need the
same name multiple times:

iommus = iommu 0, iommu 4, iommu 17, iommu 25;
iommu-names = video, video, dram, dram;

This is really specific to a given master, so we can table that until
the first master appears which needs to distinguish between IDs.

  +
  +Multiple-master IOMMU with configurable DMA window:
  +---
  +
  + / {
  + #address-cells = 1;
  + #size-cells = 1;
  +
  + iommu {
  + /* master ID, address and length of DMA window */
  + #iommu-cells = 4;
  + };
  +
  + master {
  + /* master ID 42, 4 GiB DMA window starting at 0 */
  + iommus = /iommu  42  0  0x1 0x0;
 
  Is this that window is from the POV of the master, i.e. the master can
  address 0x0 to 0x when generating transactions, and these get
  translated somehow?
 
  Or is this the physical addresses to allocate to the master?
 
 It needs to be clarified in the documentation, but as far as I know it
 is the DMA address space that is used.

Ok. So that's pre-translation, from the POV of the master?

If we don't have that knowledge about the master already (e.g. based on
the compatible string), surely we always need that information in a
given iommu-specifier format? Otherwise certain iommus won't be able to
handle masters with limited addressing only due to limitations of their
binding.

 It is somewhat confusing to have size-cells = 1 and then use 2 cells
 for size in the iommu property. It's legal and expected, but having
 size-cells in the example adds a little confusion.
 
 Either way, I'm OK with fixing the above with an incremental patch,
 assuming there is no disagreements on what's said above.

I like the general idea.

Given my concerns are to do with implementation details I'm happy to
have this go through and fix it up as the first implementations of the
binding take shape.

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

Re: [PATCH v4] devicetree: Add generic IOMMU device tree bindings

2014-07-30 Thread Arnd Bergmann 
On Wednesday 30 July 2014, Thierry Reding wrote:
se?
 
 I think there weren't any comments left for me to address and I've
 mostly been waiting for Joerg to pick it up.
 
 Joerg, can you take this through the iommu tree for 3.17? Will acked
 this, but perhaps you were waiting for an ACK from the device tree
 bindings maintainers?
 
 Will, perhaps you can get Pawel or Mark to look at this?
 
 Arnd, I'm sure if we had your Acked-by that would go a long way too.


Sorry for missing this before my vacation.

Reviewed-by: Arnd Bergmann a...@arndb.de

Olof, please merge it into arm-soc so we can finally build on top
of this!

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