Re: [PATCH 1/7] powerpc: Add interface to get msi region information

2013-09-24 Thread Bjorn Helgaas 
On Thu, Sep 19, 2013 at 12:59:17PM +0530, Bharat Bhushan wrote:
> This patch adds interface to get following information
>   - Number of MSI regions (which is number of MSI banks for powerpc).
>   - Get the region address range: Physical page which have the
>  address/addresses used for generating MSI interrupt
>  and size of the page.
> 
> These are required to create IOMMU (Freescale PAMU) mapping for
> devices which are directly assigned using VFIO.
> 
> Signed-off-by: Bharat Bhushan 
> ---
>  arch/powerpc/include/asm/machdep.h |8 +++
>  arch/powerpc/include/asm/pci.h |2 +
>  arch/powerpc/kernel/msi.c  |   18 
>  arch/powerpc/sysdev/fsl_msi.c  |   39 +--
>  arch/powerpc/sysdev/fsl_msi.h  |   11 -
>  drivers/pci/msi.c  |   26 
>  include/linux/msi.h|8 +++
>  include/linux/pci.h|   13 
>  8 files changed, 120 insertions(+), 5 deletions(-)
> 
> ...

> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index aca7578..6d85c15 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -30,6 +30,20 @@ static int pci_msi_enable = 1;
>  
>  /* Arch hooks */
>  
> +#ifndef arch_msi_get_region_count
> +int arch_msi_get_region_count(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#ifndef arch_msi_get_region
> +int arch_msi_get_region(int region_num, struct msi_region *region)
> +{
> + return 0;
> +}
> +#endif

This #define strategy is gone; see 4287d824 ("PCI: use weak functions for
MSI arch-specific functions").  Please use the weak function strategy
for your new MSI region functions.

> +
>  #ifndef arch_msi_check_device
>  int arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
>  {
> @@ -903,6 +917,18 @@ void pci_disable_msi(struct pci_dev *dev)
>  }
>  EXPORT_SYMBOL(pci_disable_msi);
>  
> +int msi_get_region_count(void)
> +{
> + return arch_msi_get_region_count();
> +}
> +EXPORT_SYMBOL(msi_get_region_count);
> +
> +int msi_get_region(int region_num, struct msi_region *region)
> +{
> + return arch_msi_get_region(region_num, region);
> +}
> +EXPORT_SYMBOL(msi_get_region);

Please split these interface additions, i.e., the drivers/pci/msi.c,
include/linux/msi.h, and include/linux/pci.h changes, into a separate
patch.

I don't know enough about VFIO to understand why these new interfaces
are needed.  Is this the first VFIO IOMMU driver?  I see
vfio_iommu_spapr_tce.c and vfio_iommu_type1.c but I don't know if
they're comparable to the Freescale PAMU.  Do other VFIO IOMMU
implementations support MSI?  If so, do they handle the problem of
mapping the MSI regions in a different way?

>  /**
>   * pci_msix_table_size - return the number of device's MSI-X table entries
>   * @dev: pointer to the pci_dev data structure of MSI-X device function
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index ee66f3a..ae32601 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -50,6 +50,12 @@ struct msi_desc {
>   struct kobject kobj;
>  };
>  
> +struct msi_region {
> + int region_num;
> + dma_addr_t addr;
> + size_t size;
> +};

This needs some sort of explanatory comment.

>  /*
>   * The arch hook for setup up msi irqs
>   */
> @@ -58,5 +64,7 @@ void arch_teardown_msi_irq(unsigned int irq);
>  int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
>  void arch_teardown_msi_irqs(struct pci_dev *dev);
>  int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
> +int arch_msi_get_region_count(void);
> +int arch_msi_get_region(int region_num, struct msi_region *region);
>  
>  #endif /* LINUX_MSI_H */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 186540d..2b26a59 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1126,6 +1126,7 @@ struct msix_entry {
>   u16 entry;  /* driver uses to specify entry, OS writes */
>  };
>  
> +struct msi_region;
>  
>  #ifndef CONFIG_PCI_MSI
>  static inline int pci_enable_msi_block(struct pci_dev *dev, unsigned int 
> nvec)
> @@ -1168,6 +1169,16 @@ static inline int pci_msi_enabled(void)
>  {
>   return 0;
>  }
> +
> +static inline int msi_get_region_count(void)
> +{
> + return 0;
> +}
> +
> +static inline int msi_get_region(int region_num, struct msi_region *region)
> +{
> + return 0;
> +}
>  #else
>  int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec);
>  int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec);
> @@ -1180,6 +1191,8 @@ void pci_disable_msix(struct pci_dev *dev);
>  void msi_remove_pci_irq_vectors(struct pci_dev *dev);
>  void pci_restore_msi_state(struct pci_dev *dev);
>  int pci_msi_enabled(void);
> +int msi_get_region_count(void);
> +int msi_get_region(int region_num, struct msi_region *region);
>  #endif
>  
>  #ifdef CONFIG_PCIEPORTBUS
> -- 
> 1.7.0.4
> 
> 
> --
> To unsubscribe from this list: send the line "uns

[PATCH v4 2/2] iommu: Change iommu driver to call io_page_fault trace event

2013-09-24 Thread Shuah Khan 
Change iommu driver call io_page_fault trace event. This iommu_error class
event can be enabled to trigger when an iommu error occurs. Trace information
includes driver name, device name, iova, and flags.

Testing:
Added trace calls to iommu_prepare_identity_map() for testing some of the
conditions that are hard to trigger. Here is the trace from the testing:

   swapper/0-1 [003]  2.003774: io_page_fault: IOMMU:pci 
:00:02.0 iova=0xcb80 flags=0x0002
   swapper/0-1 [003]  2.004098: io_page_fault: IOMMU:pci 
:00:1d.0 iova=0xcadc6000 flags=0x0002
   swapper/0-1 [003]  2.004115: io_page_fault: IOMMU:pci 
:00:1a.0 iova=0xcadc6000 flags=0x0002
   swapper/0-1 [003]  2.004129: io_page_fault: IOMMU:pci 
:00:1f.0 iova=0x flags=0x0002

Signed-off-by: Shuah Khan 
---
 include/linux/iommu.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7ea319e..a444c79 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define IOMMU_READ (1)
 #define IOMMU_WRITE(2)
@@ -227,6 +228,7 @@ static inline int report_iommu_fault(struct iommu_domain 
*domain,
ret = domain->handler(domain, dev, iova, flags,
domain->handler_token);
 
+   trace_io_page_fault(dev, iova, flags);
return ret;
 }
 
-- 
1.8.1.2

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

[PATCH v4 1/2] iommu: Add iommu_error class event to iommu trace

2013-09-24 Thread Shuah Khan 
iommu_error class event can be enabled to trigger when an iommu
error occurs. This trace event is intended to be called to report the
error information. Trace information includes driver name, device name,
iova, and flags.

iommu_error:io_page_fault

Signed-off-by: Shuah Khan 
---
 drivers/iommu/iommu-traces.c |  3 +++
 include/trace/events/iommu.h | 33 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/iommu/iommu-traces.c b/drivers/iommu/iommu-traces.c
index a2af60f..71ac5fa 100644
--- a/drivers/iommu/iommu-traces.c
+++ b/drivers/iommu/iommu-traces.c
@@ -22,3 +22,6 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(detach_device_from_domain);
 /* iommu_map_unmap */
 EXPORT_TRACEPOINT_SYMBOL_GPL(map);
 EXPORT_TRACEPOINT_SYMBOL_GPL(unmap);
+
+/* iommu_error */
+EXPORT_TRACEPOINT_SYMBOL_GPL(io_page_fault);
diff --git a/include/trace/events/iommu.h b/include/trace/events/iommu.h
index 86bcc5a..a8f5c32 100644
--- a/include/trace/events/iommu.h
+++ b/include/trace/events/iommu.h
@@ -123,6 +123,39 @@ DEFINE_EVENT_PRINT(iommu_map_unmap, unmap,
__entry->iova, __entry->size
)
 );
+
+DECLARE_EVENT_CLASS(iommu_error,
+
+   TP_PROTO(struct device *dev, unsigned long iova, int flags),
+
+   TP_ARGS(dev, iova, flags),
+
+   TP_STRUCT__entry(
+   __string(device, dev_name(dev))
+   __string(driver, dev_driver_string(dev))
+   __field(u64, iova)
+   __field(int, flags)
+   ),
+
+   TP_fast_assign(
+   __assign_str(device, dev_name(dev));
+   __assign_str(driver, dev_driver_string(dev));
+   __entry->iova = iova;
+   __entry->flags = flags;
+   ),
+
+   TP_printk("IOMMU:%s %s iova=0x%016llx flags=0x%04x",
+   __get_str(driver), __get_str(device),
+   __entry->iova, __entry->flags
+   )
+);
+
+DEFINE_EVENT(iommu_error, io_page_fault,
+
+   TP_PROTO(struct device *dev, unsigned long iova, int flags),
+
+   TP_ARGS(dev, iova, flags)
+);
 #endif /* _TRACE_IOMMU_H */
 
 /* This part must be outside protection */
-- 
1.8.1.2

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

[PATCH v4 0/2] iommu: Add iommu_error class event to iommu trace

2013-09-24 Thread Shuah Khan 
This patch set adds iommu_error class event to iommu trace. iommu_error class
event can be enabled to trigger when an iommu error occurs. This trace event
is intended to be called to report the error information. Trace information
includes driver name, device name, iova, and flags.

iommu_error:io_page_fault

This patch set depends on the previous patch set that added iommu tracing
feature. Reference:

http://www.kernelhub.org/?msg=313155&p=2
http://www.kernelhub.org/?msg=313156&p=2

Shuah Khan (2):
  iommu: Add iommu_error class event to iommu trace
  iommu: Change iommu driver to call io_page_fault trace event

 drivers/iommu/iommu-traces.c |  3 +++
 include/linux/iommu.h|  2 ++
 include/trace/events/iommu.h | 33 +
 3 files changed, 38 insertions(+)

-- 
1.8.1.2

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

Re: [PATCH 7/7] iommu/arm-smmu: Clear global and context bank fault status and syndrome registers

2013-09-24 Thread Andreas Herrmann 
On Tue, Sep 24, 2013 at 11:42:52AM -0400, Will Deacon wrote:
> On Tue, Sep 24, 2013 at 04:07:01PM +0100, Andreas Herrmann wrote:
> > Signed-off-by: Andreas Herrmann 
> > ---
> >  drivers/iommu/arm-smmu.c |9 +
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index 251564e..a499146 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -645,6 +645,10 @@ static void arm_smmu_init_context_bank(struct 
> > arm_smmu_domain *smmu_domain)
> > stage1 = root_cfg->cbar != CBAR_TYPE_S2_TRANS;
> > cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, root_cfg->cbndx);
> >  
> > +   /* clear fsr */
> > +   writel_relaxed(0x, cb_base + ARM_SMMU_CB_FSR);
> > +   writel_relaxed(0, cb_base + ARM_SMMU_CB_FSYNR0);
> > +
> > /* CBAR */
> > reg = root_cfg->cbar;
> > if (smmu->version == 1)
> > @@ -1570,6 +1574,11 @@ static void arm_smmu_device_reset(struct 
> > arm_smmu_device *smmu)
> > int i = 0;
> > u32 scr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sCR0);
> >  
> > +   /* clear global FSRs */
> > +   writel(0x, gr0_base + ARM_SMMU_GR0_sGFSR);
> > +   writel(0, gr0_base + ARM_SMMU_GR0_sGFSYNR0);
> > +   writel(0, gr0_base + ARM_SMMU_GR0_sGFSYNR1);
> 
> Why do you need this?

According to the spec the status and syndrome registers have
unknown/unpredictable reset values. So better set known values before
we start to use these registers (ie. handle faults where we read
them). No?


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

Re: [PATCH 2/7] iommu/arm-smmu: Calculate SMMU_CB_BASE from smmu register values

2013-09-24 Thread Andreas Herrmann 
On Tue, Sep 24, 2013 at 11:34:57AM -0400, Will Deacon wrote:
> On Tue, Sep 24, 2013 at 04:06:56PM +0100, Andreas Herrmann wrote:
> > Currently it is derived from smmu resource size. If the resource size
> > is wrongly specified (e.g. too large) this leads to a miscalculation
> > and can cause undefined behaviour when context bank registers are
> > modified.
> > 
> > Signed-off-by: Andreas Herrmann 
> > ---
> >  drivers/iommu/arm-smmu.c |7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index 97b764b..f5a856e 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -207,7 +207,7 @@
> >  #define CBA2R_RW64_64BIT   (1 << 0)
> >  
> >  /* Translation context bank */
> > -#define ARM_SMMU_CB_BASE(smmu) ((smmu)->base + ((smmu)->size 
> > >> 1))
> > +#define ARM_SMMU_CB_BASE(smmu) ((smmu)->cb_base)
> >  #define ARM_SMMU_CB(smmu, n)   ((n) * (smmu)->pagesize)
> >  
> >  #define ARM_SMMU_CB_SCTLR  0x0
> > @@ -339,6 +339,7 @@ struct arm_smmu_device {
> > struct device_node  *parent_of_node;
> >  
> > void __iomem*base;
> > +   void __iomem*cb_base;
> > unsigned long   size;
> > unsigned long   pagesize;
> >  
> > @@ -1701,7 +1702,9 @@ static int arm_smmu_device_cfg_probe(struct 
> > arm_smmu_device *smmu)
> >  
> > /* Check that we ioremapped enough */
> > size = 1 << (((id >> ID1_NUMPAGENDXB_SHIFT) & ID1_NUMPAGENDXB_MASK) + 
> > 1);
> > -   size *= (smmu->pagesize << 1);
> > +   size *= smmu->pagesize;
> > +   smmu->cb_base = smmu->base + size;
> > +   size *= 2;
> > if (smmu->size < size)
> > dev_warn(smmu->dev,
> >  "device is 0x%lx bytes but only mapped 0x%lx!\n",
> 
> Hmm, this is a tricky one. We know that we have an inconsistency (i.e. the
> DT and the hardware don't agree on the size of the device) but we warn and
> attempt to continue with the value from the DT. I don't think that trusting
> the hardware is the right thing to do in this case, since it's not possible
> to change so we should let the DT act as an override.

> In other words: if the device tree is wrong, go fix it.

Yes, I've found this issue with a wrong DT. With the original code
there was some weirdness when setting certain context bank
registers. (Identifying the root cause was not straight forward.)

I think it's somehow odd not to trust the hardware values in the first
place and to add (right from the beginning) a quirk for potential
implementation bugs. Are there already implementations that use wrong
register values that are required to determine the partitioning of the
SMMU address space?

If there is a mismatch it's hard to say which value is the correct
one. I think there are three options:
(1) just print a warning about the mismatch
(2) print a warning + override based on DT
(3) print a warning + override based on DT + have an option to switch
off the override

So, what's your choice?


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

Re: [PATCH 3/7] ARM: dma-mapping: Always pass proper prot flags to iommu_map()

2013-09-24 Thread Andreas Herrmann 
On Tue, Sep 24, 2013 at 11:36:25AM -0400, Will Deacon wrote:
> On Tue, Sep 24, 2013 at 04:06:57PM +0100, Andreas Herrmann wrote:
> > ... otherwise it is impossible for the low level iommu driver to
> > figure out which pte flags should be used.
> > 
> > In __map_sg_chunk we can derive the flags from dma_data_direction.
> > 
> > In __iommu_create_mapping we should treat the memory like
> > DMA_BIDIRECTIONAL and pass both IOMMU_READ and IOMMU_WRITE to
> > iommu_map.
> > __iommu_create_mapping is used during dma_alloc_coherent (via
> > arm_iommu_alloc_attrs).  AFAIK dma_alloc_coherent is responsible for
> > allocation _and_ mapping.  I think this implies that access to the
> > mapped pages should be allowed.
> > 
> > Cc: Marek Szyprowski 
> > Signed-off-by: Andreas Herrmann 
> > ---
> >  arch/arm/mm/dma-mapping.c |   20 ++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index f5e1a84..95abed8 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -1232,7 +1232,8 @@ __iommu_create_mapping(struct device *dev, struct 
> > page **pages, size_t size)
> > break;
> >  
> > len = (j - i) << PAGE_SHIFT;
> > -   ret = iommu_map(mapping->domain, iova, phys, len, 0);
> > +   ret = iommu_map(mapping->domain, iova, phys, len,
> > +   IOMMU_READ|IOMMU_WRITE);
> > if (ret < 0)
> > goto fail;
> > iova += len;
> > @@ -1444,6 +1445,7 @@ static int __map_sg_chunk(struct device *dev, struct 
> > scatterlist *sg,
> > int ret = 0;
> > unsigned int count;
> > struct scatterlist *s;
> > +   int prot;
> >  
> > size = PAGE_ALIGN(size);
> > *handle = DMA_ERROR_CODE;
> > @@ -1460,7 +1462,21 @@ static int __map_sg_chunk(struct device *dev, struct 
> > scatterlist *sg,
> > !dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
> > __dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, 
> > dir);
> >  
> > -   ret = iommu_map(mapping->domain, iova, phys, len, 0);
> > +   switch (dir) {
> > +   case DMA_BIDIRECTIONAL:
> > +   prot = IOMMU_READ | IOMMU_WRITE;
> > +   break;
> > +   case DMA_TO_DEVICE:
> > +   prot = IOMMU_READ;
> > +   break;
> > +   case DMA_FROM_DEVICE:
> > +   prot = IOMMU_WRITE;
> > +   break;
> > +   default:
> > +   prot = 0;
> > +   }
> > +
> > +   ret = iommu_map(mapping->domain, iova, phys, len, prot);
> 
> I already added this code to arm_coherent_iommu_map_page but forgot to
> update the rest of the time. Could you shift the switch into a helper and
> call that instead of inlining it explicitly?

Yes, will do so.


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

Re: [PATCH 7/7] iommu/arm-smmu: Clear global and context bank fault status and syndrome registers

2013-09-24 Thread Will Deacon 
On Tue, Sep 24, 2013 at 04:07:01PM +0100, Andreas Herrmann wrote:
> Signed-off-by: Andreas Herrmann 
> ---
>  drivers/iommu/arm-smmu.c |9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 251564e..a499146 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -645,6 +645,10 @@ static void arm_smmu_init_context_bank(struct 
> arm_smmu_domain *smmu_domain)
>   stage1 = root_cfg->cbar != CBAR_TYPE_S2_TRANS;
>   cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, root_cfg->cbndx);
>  
> + /* clear fsr */
> + writel_relaxed(0x, cb_base + ARM_SMMU_CB_FSR);
> + writel_relaxed(0, cb_base + ARM_SMMU_CB_FSYNR0);
> +
>   /* CBAR */
>   reg = root_cfg->cbar;
>   if (smmu->version == 1)
> @@ -1570,6 +1574,11 @@ static void arm_smmu_device_reset(struct 
> arm_smmu_device *smmu)
>   int i = 0;
>   u32 scr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sCR0);
>  
> + /* clear global FSRs */
> + writel(0x, gr0_base + ARM_SMMU_GR0_sGFSR);
> + writel(0, gr0_base + ARM_SMMU_GR0_sGFSYNR0);
> + writel(0, gr0_base + ARM_SMMU_GR0_sGFSYNR1);

Why do you need this?

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

Re: [PATCH 6/7] iommu/arm-smmu: Add module parameter arm-smmu=off|force_isolation

2013-09-24 Thread Will Deacon 
On Tue, Sep 24, 2013 at 04:07:00PM +0100, Andreas Herrmann wrote:
>   arm-smmu=   arm-smmu driver option
> 
>   off Disable arm-smmu driver (ie. ignore available SMMUs)
> 
>   force_isolation
>   Try to attach each master device on every SMMU to a
>   separate iommu_domain.
> 
> Default is that driver detects SMMUs but no translation is configured
> (transactions just bypass translation process).
> 
> Signed-off-by: Andreas Herrmann 
> ---
>  drivers/iommu/arm-smmu.c |   26 ++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 3eb2259..251564e 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -399,6 +399,9 @@ struct arm_smmu_domain {
>  static DEFINE_SPINLOCK(arm_smmu_devices_lock);
>  static LIST_HEAD(arm_smmu_devices);
>  
> +static bool arm_smmu_disabled;
> +static bool arm_smmu_force_isolation;
> +
>  static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu,
>   struct device_node *dev_node)
>  {
> @@ -1837,6 +1840,9 @@ static int arm_smmu_device_dt_probe(struct 
> platform_device *pdev)
>   struct of_phandle_args masterspec;
>   int num_irqs, i, err;
>  
> + if (arm_smmu_disabled)
> + return -ENODEV;
> +
>   smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
>   if (!smmu) {
>   dev_err(dev, "failed to allocate arm_smmu_device\n");
> @@ -2022,6 +2028,23 @@ static struct platform_driver arm_smmu_driver = {
>   .remove = arm_smmu_device_remove,
>  };
>  
> +static int __init arm_smmu_parse_options(char *str)
> +{
> + if (*str) {
> + str++;
> + if (!strncmp(str, "off", 3))
> + arm_smmu_disabled = true;
> + else if(!strncmp(str, "force_isolation", 15))
> + arm_smmu_force_isolation = true;
> + else {
> + pr_warn("arm_smmu: invalid parameter (\"%s\")\n", str);
> + return 0;
> + }
> + }
> + return 1;
> +}
> +__setup("arm-smmu", arm_smmu_parse_options);

If this is going to be a common function for IOMMUs, let's instead move the
command-line parsing out into the generic IOMMU layer, then have an optional
callback into the low-level IOMMU driver for enabling/disabling it.

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

Re: [PATCH 3/7] ARM: dma-mapping: Always pass proper prot flags to iommu_map()

2013-09-24 Thread Will Deacon 
On Tue, Sep 24, 2013 at 04:06:57PM +0100, Andreas Herrmann wrote:
> ... otherwise it is impossible for the low level iommu driver to
> figure out which pte flags should be used.
> 
> In __map_sg_chunk we can derive the flags from dma_data_direction.
> 
> In __iommu_create_mapping we should treat the memory like
> DMA_BIDIRECTIONAL and pass both IOMMU_READ and IOMMU_WRITE to
> iommu_map.
> __iommu_create_mapping is used during dma_alloc_coherent (via
> arm_iommu_alloc_attrs).  AFAIK dma_alloc_coherent is responsible for
> allocation _and_ mapping.  I think this implies that access to the
> mapped pages should be allowed.
> 
> Cc: Marek Szyprowski 
> Signed-off-by: Andreas Herrmann 
> ---
>  arch/arm/mm/dma-mapping.c |   20 ++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index f5e1a84..95abed8 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1232,7 +1232,8 @@ __iommu_create_mapping(struct device *dev, struct page 
> **pages, size_t size)
>   break;
>  
>   len = (j - i) << PAGE_SHIFT;
> - ret = iommu_map(mapping->domain, iova, phys, len, 0);
> + ret = iommu_map(mapping->domain, iova, phys, len,
> + IOMMU_READ|IOMMU_WRITE);
>   if (ret < 0)
>   goto fail;
>   iova += len;
> @@ -1444,6 +1445,7 @@ static int __map_sg_chunk(struct device *dev, struct 
> scatterlist *sg,
>   int ret = 0;
>   unsigned int count;
>   struct scatterlist *s;
> + int prot;
>  
>   size = PAGE_ALIGN(size);
>   *handle = DMA_ERROR_CODE;
> @@ -1460,7 +1462,21 @@ static int __map_sg_chunk(struct device *dev, struct 
> scatterlist *sg,
>   !dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
>   __dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, 
> dir);
>  
> - ret = iommu_map(mapping->domain, iova, phys, len, 0);
> + switch (dir) {
> + case DMA_BIDIRECTIONAL:
> + prot = IOMMU_READ | IOMMU_WRITE;
> + break;
> + case DMA_TO_DEVICE:
> + prot = IOMMU_READ;
> + break;
> + case DMA_FROM_DEVICE:
> + prot = IOMMU_WRITE;
> + break;
> + default:
> + prot = 0;
> + }
> +
> + ret = iommu_map(mapping->domain, iova, phys, len, prot);

I already added this code to arm_coherent_iommu_map_page but forgot to
update the rest of the time. Could you shift the switch into a helper and
call that instead of inlining it explicitly?

Cheers,

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

Re: [PATCH 4/7] iommu/arm-smmu: Check for num_context_irqs > 0 to avoid divide by zero exception

2013-09-24 Thread Will Deacon 
On Tue, Sep 24, 2013 at 04:06:58PM +0100, Andreas Herrmann wrote:
> With the right (or wrong;-) definition of v1 SMMU node in DTB it is
> possible to trigger a division by zero in arm_smmu_init_domain_context
> (if number of context irqs is 0):
> 
>if (smmu->version == 1) {
>root_cfg->irptndx = atomic_inc_return(&smmu->irptndx);
>  → root_cfg->irptndx %= smmu->num_context_irqs;
>} else {
> 
> Avoid this by checking for num_context_irqs > 0 before trying to
> assign interrupts to contexts.
> 
> Signed-off-by: Andreas Herrmann 
> ---
>  drivers/iommu/arm-smmu.c |   31 +--
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f5a856e..0dfd255 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -828,21 +828,24 @@ static int arm_smmu_init_domain_context(struct 
> iommu_domain *domain,
>   return ret;
>  
>   root_cfg->cbndx = ret;
> - if (smmu->version == 1) {
> - root_cfg->irptndx = atomic_inc_return(&smmu->irptndx);
> - root_cfg->irptndx %= smmu->num_context_irqs;
> - } else {
> - root_cfg->irptndx = root_cfg->cbndx;
> - }
>  
> - irq = smmu->irqs[smmu->num_global_irqs + root_cfg->irptndx];
> - ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
> -   "arm-smmu-context-fault", domain);
> - if (IS_ERR_VALUE(ret)) {
> - dev_err(smmu->dev, "failed to request context IRQ %d (%u)\n",
> - root_cfg->irptndx, irq);
> - root_cfg->irptndx = -1;
> - goto out_free_context;
> + if (smmu->num_context_irqs) {

Can we move this check to probe time, to avoid re-evaluating it every time
we initialise a new domain?

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

Re: [PATCH 2/7] iommu/arm-smmu: Calculate SMMU_CB_BASE from smmu register values

2013-09-24 Thread Will Deacon 
On Tue, Sep 24, 2013 at 04:06:56PM +0100, Andreas Herrmann wrote:
> Currently it is derived from smmu resource size. If the resource size
> is wrongly specified (e.g. too large) this leads to a miscalculation
> and can cause undefined behaviour when context bank registers are
> modified.
> 
> Signed-off-by: Andreas Herrmann 
> ---
>  drivers/iommu/arm-smmu.c |7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 97b764b..f5a856e 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -207,7 +207,7 @@
>  #define CBA2R_RW64_64BIT (1 << 0)
>  
>  /* Translation context bank */
> -#define ARM_SMMU_CB_BASE(smmu)   ((smmu)->base + ((smmu)->size 
> >> 1))
> +#define ARM_SMMU_CB_BASE(smmu)   ((smmu)->cb_base)
>  #define ARM_SMMU_CB(smmu, n) ((n) * (smmu)->pagesize)
>  
>  #define ARM_SMMU_CB_SCTLR0x0
> @@ -339,6 +339,7 @@ struct arm_smmu_device {
>   struct device_node  *parent_of_node;
>  
>   void __iomem*base;
> + void __iomem*cb_base;
>   unsigned long   size;
>   unsigned long   pagesize;
>  
> @@ -1701,7 +1702,9 @@ static int arm_smmu_device_cfg_probe(struct 
> arm_smmu_device *smmu)
>  
>   /* Check that we ioremapped enough */
>   size = 1 << (((id >> ID1_NUMPAGENDXB_SHIFT) & ID1_NUMPAGENDXB_MASK) + 
> 1);
> - size *= (smmu->pagesize << 1);
> + size *= smmu->pagesize;
> + smmu->cb_base = smmu->base + size;
> + size *= 2;
>   if (smmu->size < size)
>   dev_warn(smmu->dev,
>"device is 0x%lx bytes but only mapped 0x%lx!\n",

Hmm, this is a tricky one. We know that we have an inconsistency (i.e. the
DT and the hardware don't agree on the size of the device) but we warn and
attempt to continue with the value from the DT. I don't think that trusting
the hardware is the right thing to do in this case, since it's not possible
to change so we should let the DT act as an override.

In other words: if the device tree is wrong, go fix it.

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

Re: [PATCH 0/7]: iommu/arm-smmu: Misc fixes/adaptions

2013-09-24 Thread Will Deacon 
On Tue, Sep 24, 2013 at 04:06:54PM +0100, Andreas Herrmann wrote:
> Hi,

Hi Andreas,

> Following patches fix misc issues, that I've seen when using arm-smmu
> driver with MMU-400.

Thanks for the patches! I'll respond to each one in turn.

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

Re: [PATCH 1/7] iommu/arm-smmu: Switch to arch_initcall for driver registration

2013-09-24 Thread Andreas Herrmann 
iommu/arm-smmu: Remove bogus semicolon from if conditions

Those prevented proper registration of arm_smmu_ops.

Signed-off-by: Andreas Herrmann 
---
 drivers/iommu/arm-smmu.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Will,

It seems that I've managed to exclude the very first patch of the
series. Here it is.


Regards,

Andreas

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f417e89..be56846 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1966,10 +1966,10 @@ static int __init arm_smmu_init(void)
return ret;
 
/* Oh, for a proper bus abstraction */
-   if (!iommu_present(&platform_bus_type));
+   if (!iommu_present(&platform_bus_type))
bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
 
-   if (!iommu_present(&amba_bustype));
+   if (!iommu_present(&amba_bustype))
bus_set_iommu(&amba_bustype, &arm_smmu_ops);
 
return 0;
-- 
1.7.9.5

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

Re: [PATCH 1/7] iommu/arm-smmu: Switch to arch_initcall for driver registration

2013-09-24 Thread Will Deacon 
On Tue, Sep 24, 2013 at 04:14:30PM +0100, Andreas Herrmann wrote:
> iommu/arm-smmu: Remove bogus semicolon from if conditions
> 
> Those prevented proper registration of arm_smmu_ops.
> 
> Signed-off-by: Andreas Herrmann 
> ---
>  drivers/iommu/arm-smmu.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Will,
> 
> It seems that I've managed to exclude the very first patch of the
> series. Here it is.

No worries; I already have a fix for this and Joerg pulled it today.

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

[PATCH 0/7]: iommu/arm-smmu: Misc fixes/adaptions

2013-09-24 Thread Andreas Herrmann 
Hi,

Following patches fix misc issues, that I've seen when using arm-smmu
driver with MMU-400.

I also suggest to add an option to automatically create domains for
each master of detected SMMUs. (Similar to what is available for other
iommu driver(s).)

Comments welcome.


Regards,

Andreas

PS: Patches are against v3.12-rc1-336-gd8524ae (and yes I probably
need to rebase against Joerg's or your smmu tree but I wanted to
sent this out to get feedback as soon as possible.)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/7] iommu/arm-smmu: Calculate SMMU_CB_BASE from smmu register values

2013-09-24 Thread Andreas Herrmann 
Currently it is derived from smmu resource size. If the resource size
is wrongly specified (e.g. too large) this leads to a miscalculation
and can cause undefined behaviour when context bank registers are
modified.

Signed-off-by: Andreas Herrmann 
---
 drivers/iommu/arm-smmu.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 97b764b..f5a856e 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -207,7 +207,7 @@
 #define CBA2R_RW64_64BIT   (1 << 0)
 
 /* Translation context bank */
-#define ARM_SMMU_CB_BASE(smmu) ((smmu)->base + ((smmu)->size >> 1))
+#define ARM_SMMU_CB_BASE(smmu) ((smmu)->cb_base)
 #define ARM_SMMU_CB(smmu, n)   ((n) * (smmu)->pagesize)
 
 #define ARM_SMMU_CB_SCTLR  0x0
@@ -339,6 +339,7 @@ struct arm_smmu_device {
struct device_node  *parent_of_node;
 
void __iomem*base;
+   void __iomem*cb_base;
unsigned long   size;
unsigned long   pagesize;
 
@@ -1701,7 +1702,9 @@ static int arm_smmu_device_cfg_probe(struct 
arm_smmu_device *smmu)
 
/* Check that we ioremapped enough */
size = 1 << (((id >> ID1_NUMPAGENDXB_SHIFT) & ID1_NUMPAGENDXB_MASK) + 
1);
-   size *= (smmu->pagesize << 1);
+   size *= smmu->pagesize;
+   smmu->cb_base = smmu->base + size;
+   size *= 2;
if (smmu->size < size)
dev_warn(smmu->dev,
 "device is 0x%lx bytes but only mapped 0x%lx!\n",
-- 
1.7.9.5

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

[PATCH 1/7] iommu/arm-smmu: Switch to arch_initcall for driver registration

2013-09-24 Thread Andreas Herrmann 
This should ensure that arm-smmu is initialized before other drivers
start handling devices that propably need smmu support.

Also remove module_exit function as we most likely never want to
unload this driver.

Signed-off-by: Andreas Herrmann 
---
 drivers/iommu/arm-smmu.c |8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index be56846..97b764b 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1975,13 +1975,7 @@ static int __init arm_smmu_init(void)
return 0;
 }
 
-static void __exit arm_smmu_exit(void)
-{
-   return platform_driver_unregister(&arm_smmu_driver);
-}
-
-module_init(arm_smmu_init);
-module_exit(arm_smmu_exit);
+arch_initcall(arm_smmu_init);
 
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
 MODULE_AUTHOR("Will Deacon ");
-- 
1.7.9.5

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

[PATCH 6/7] iommu/arm-smmu: Add module parameter arm-smmu=off|force_isolation

2013-09-24 Thread Andreas Herrmann 
  arm-smmu= arm-smmu driver option

off Disable arm-smmu driver (ie. ignore available SMMUs)

force_isolation
Try to attach each master device on every SMMU to a
separate iommu_domain.

Default is that driver detects SMMUs but no translation is configured
(transactions just bypass translation process).

Signed-off-by: Andreas Herrmann 
---
 drivers/iommu/arm-smmu.c |   26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 3eb2259..251564e 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -399,6 +399,9 @@ struct arm_smmu_domain {
 static DEFINE_SPINLOCK(arm_smmu_devices_lock);
 static LIST_HEAD(arm_smmu_devices);
 
+static bool arm_smmu_disabled;
+static bool arm_smmu_force_isolation;
+
 static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu,
struct device_node *dev_node)
 {
@@ -1837,6 +1840,9 @@ static int arm_smmu_device_dt_probe(struct 
platform_device *pdev)
struct of_phandle_args masterspec;
int num_irqs, i, err;
 
+   if (arm_smmu_disabled)
+   return -ENODEV;
+
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
if (!smmu) {
dev_err(dev, "failed to allocate arm_smmu_device\n");
@@ -2022,6 +2028,23 @@ static struct platform_driver arm_smmu_driver = {
.remove = arm_smmu_device_remove,
 };
 
+static int __init arm_smmu_parse_options(char *str)
+{
+   if (*str) {
+   str++;
+   if (!strncmp(str, "off", 3))
+   arm_smmu_disabled = true;
+   else if(!strncmp(str, "force_isolation", 15))
+   arm_smmu_force_isolation = true;
+   else {
+   pr_warn("arm_smmu: invalid parameter (\"%s\")\n", str);
+   return 0;
+   }
+   }
+   return 1;
+}
+__setup("arm-smmu", arm_smmu_parse_options);
+
 static int __init arm_smmu_init(void)
 {
int ret;
@@ -2037,6 +2060,9 @@ static int __init arm_smmu_init(void)
if (!iommu_present(&amba_bustype))
bus_set_iommu(&amba_bustype, &arm_smmu_ops);
 
+   if (arm_smmu_force_isolation)
+   arm_smmu_isolate_devices();
+
return 0;
 }
 
-- 
1.7.9.5

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

[PATCH 3/7] ARM: dma-mapping: Always pass proper prot flags to iommu_map()

2013-09-24 Thread Andreas Herrmann 
... otherwise it is impossible for the low level iommu driver to
figure out which pte flags should be used.

In __map_sg_chunk we can derive the flags from dma_data_direction.

In __iommu_create_mapping we should treat the memory like
DMA_BIDIRECTIONAL and pass both IOMMU_READ and IOMMU_WRITE to
iommu_map.
__iommu_create_mapping is used during dma_alloc_coherent (via
arm_iommu_alloc_attrs).  AFAIK dma_alloc_coherent is responsible for
allocation _and_ mapping.  I think this implies that access to the
mapped pages should be allowed.

Cc: Marek Szyprowski 
Signed-off-by: Andreas Herrmann 
---
 arch/arm/mm/dma-mapping.c |   20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index f5e1a84..95abed8 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1232,7 +1232,8 @@ __iommu_create_mapping(struct device *dev, struct page 
**pages, size_t size)
break;
 
len = (j - i) << PAGE_SHIFT;
-   ret = iommu_map(mapping->domain, iova, phys, len, 0);
+   ret = iommu_map(mapping->domain, iova, phys, len,
+   IOMMU_READ|IOMMU_WRITE);
if (ret < 0)
goto fail;
iova += len;
@@ -1444,6 +1445,7 @@ static int __map_sg_chunk(struct device *dev, struct 
scatterlist *sg,
int ret = 0;
unsigned int count;
struct scatterlist *s;
+   int prot;
 
size = PAGE_ALIGN(size);
*handle = DMA_ERROR_CODE;
@@ -1460,7 +1462,21 @@ static int __map_sg_chunk(struct device *dev, struct 
scatterlist *sg,
!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, 
dir);
 
-   ret = iommu_map(mapping->domain, iova, phys, len, 0);
+   switch (dir) {
+   case DMA_BIDIRECTIONAL:
+   prot = IOMMU_READ | IOMMU_WRITE;
+   break;
+   case DMA_TO_DEVICE:
+   prot = IOMMU_READ;
+   break;
+   case DMA_FROM_DEVICE:
+   prot = IOMMU_WRITE;
+   break;
+   default:
+   prot = 0;
+   }
+
+   ret = iommu_map(mapping->domain, iova, phys, len, prot);
if (ret < 0)
goto fail;
count += len >> PAGE_SHIFT;
-- 
1.7.9.5

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

[PATCH 5/7] iommu/arm-smmu: Add function that isolates all masters for all SMMUs

2013-09-24 Thread Andreas Herrmann 
Each device is put into its own protection domain (if possible).
For configuration with one or just a view masters per SMMU that
is easy to achieve.

In case of many devices per SMMU (e.g. MMU-500 with it's distributed
translation support) isolation of each device might not be possible --
depending on number of available SMR groups and/or context banks.

Derive dma_base and size from (coherent_)dma_mask of device

Signed-off-by: Andreas Herrmann 
---
 drivers/iommu/arm-smmu.c |   59 ++
 1 file changed, 59 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 0dfd255..3eb2259 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -46,6 +46,7 @@
 #include 
 
 #include 
+#include 
 
 /* Maximum number of stream IDs assigned to a single device */
 #define MAX_MASTER_STREAMIDS   8
@@ -1768,6 +1769,64 @@ static int arm_smmu_device_cfg_probe(struct 
arm_smmu_device *smmu)
return 0;
 }
 
+extern struct platform_device *of_find_device_by_node(struct device_node *np);
+
+static int arm_smmu_isolate_devices(void)
+{
+   struct dma_iommu_mapping *mapping;
+   struct arm_smmu_device *smmu;
+   struct rb_node *rbn;
+   struct arm_smmu_master *master;
+   struct platform_device *pdev;
+   struct device *dev;
+   void __iomem *gr0_base;
+   u32 cr0;
+   int ret = 0;
+   size_t size;
+
+   list_for_each_entry(smmu, &arm_smmu_devices, list) {
+   rbn = rb_first(&smmu->masters);
+   while (rbn) {
+   master = container_of(rbn, struct arm_smmu_master, 
node);
+   pdev = of_find_device_by_node(master->of_node);
+   if (!pdev)
+   break;
+   dev = &pdev->dev;
+
+   size = (size_t) dev->coherent_dma_mask;
+   size = size ? : (unsigned long) dev->dma_mask;
+   if (!size) {
+   dev_warn(dev, "(coherent_)dma_mask not set\n");
+   continue;
+   }
+
+   mapping = arm_iommu_create_mapping(&platform_bus_type,
+   0, size, 0);
+   if (IS_ERR(mapping)) {
+   ret = PTR_ERR(mapping);
+   dev_info(dev, "arm_iommu_create_mapping 
failed\n");
+   goto out;
+   }
+
+   ret = arm_iommu_attach_device(dev, mapping);
+   if (ret < 0) {
+   dev_info(dev, "arm_iommu_attach_device 
failed\n");
+   arm_iommu_release_mapping(mapping);
+   }
+   rbn = rb_next(rbn);
+   }
+
+   gr0_base = ARM_SMMU_GR0(smmu);
+   cr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sCR0);
+   cr0 |= sCR0_USFCFG;
+   writel(cr0, gr0_base + ARM_SMMU_GR0_sCR0);
+   }
+
+out:
+   return ret;
+}
+
+
 static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 {
struct resource *res;
-- 
1.7.9.5

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

[PATCH 4/7] iommu/arm-smmu: Check for num_context_irqs > 0 to avoid divide by zero exception

2013-09-24 Thread Andreas Herrmann 
With the right (or wrong;-) definition of v1 SMMU node in DTB it is
possible to trigger a division by zero in arm_smmu_init_domain_context
(if number of context irqs is 0):

   if (smmu->version == 1) {
   root_cfg->irptndx = atomic_inc_return(&smmu->irptndx);
 → root_cfg->irptndx %= smmu->num_context_irqs;
   } else {

Avoid this by checking for num_context_irqs > 0 before trying to
assign interrupts to contexts.

Signed-off-by: Andreas Herrmann 
---
 drivers/iommu/arm-smmu.c |   31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f5a856e..0dfd255 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -828,21 +828,24 @@ static int arm_smmu_init_domain_context(struct 
iommu_domain *domain,
return ret;
 
root_cfg->cbndx = ret;
-   if (smmu->version == 1) {
-   root_cfg->irptndx = atomic_inc_return(&smmu->irptndx);
-   root_cfg->irptndx %= smmu->num_context_irqs;
-   } else {
-   root_cfg->irptndx = root_cfg->cbndx;
-   }
 
-   irq = smmu->irqs[smmu->num_global_irqs + root_cfg->irptndx];
-   ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
- "arm-smmu-context-fault", domain);
-   if (IS_ERR_VALUE(ret)) {
-   dev_err(smmu->dev, "failed to request context IRQ %d (%u)\n",
-   root_cfg->irptndx, irq);
-   root_cfg->irptndx = -1;
-   goto out_free_context;
+   if (smmu->num_context_irqs) {
+   if (smmu->version == 1) {
+   root_cfg->irptndx = atomic_inc_return(&smmu->irptndx);
+   root_cfg->irptndx %= smmu->num_context_irqs;
+   } else {
+   root_cfg->irptndx = root_cfg->cbndx;
+   }
+
+   irq = smmu->irqs[smmu->num_global_irqs + root_cfg->irptndx];
+   ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
+   "arm-smmu-context-fault", domain);
+   if (IS_ERR_VALUE(ret)) {
+   dev_err(smmu->dev, "failed to request context IRQ %d 
(%u)\n",
+   root_cfg->irptndx, irq);
+   root_cfg->irptndx = -1;
+   goto out_free_context;
+   }
}
 
root_cfg->smmu = smmu;
-- 
1.7.9.5

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

[PATCH 7/7] iommu/arm-smmu: Clear global and context bank fault status and syndrome registers

2013-09-24 Thread Andreas Herrmann 
Signed-off-by: Andreas Herrmann 
---
 drivers/iommu/arm-smmu.c |9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 251564e..a499146 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -645,6 +645,10 @@ static void arm_smmu_init_context_bank(struct 
arm_smmu_domain *smmu_domain)
stage1 = root_cfg->cbar != CBAR_TYPE_S2_TRANS;
cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, root_cfg->cbndx);
 
+   /* clear fsr */
+   writel_relaxed(0x, cb_base + ARM_SMMU_CB_FSR);
+   writel_relaxed(0, cb_base + ARM_SMMU_CB_FSYNR0);
+
/* CBAR */
reg = root_cfg->cbar;
if (smmu->version == 1)
@@ -1570,6 +1574,11 @@ static void arm_smmu_device_reset(struct arm_smmu_device 
*smmu)
int i = 0;
u32 scr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sCR0);
 
+   /* clear global FSRs */
+   writel(0x, gr0_base + ARM_SMMU_GR0_sGFSR);
+   writel(0, gr0_base + ARM_SMMU_GR0_sGFSYNR0);
+   writel(0, gr0_base + ARM_SMMU_GR0_sGFSYNR1);
+
/* Mark all SMRn as invalid and all S2CRn as bypass */
for (i = 0; i < smmu->num_mapping_groups; ++i) {
writel_relaxed(~SMR_VALID, gr0_base + ARM_SMMU_GR0_SMR(i));
-- 
1.7.9.5

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

Re: [PATCH v2] intel-iommu: Quiesce devices before disabling IOMMU

2013-09-24 Thread David Woodhouse 
On Tue, 2013-09-24 at 15:16 +0200, Joerg Roedel wrote:
> 
> I am not convinced that this is the right approach. If a device wasn't
> translated by VT-d in the old kernel doesn't mean it will not be
> translated in the new kernel. How about unconditionally resetting all
> PCI busses and/or functions here before IOMMU initialization proceeds?

Forget the IOMMU.

If a device is doing DMA in the old kernel, and *continues* doing DMA
under the new kernel, surely something ought to shut it down?

It's the generic kexec or machine_shutdown code which should be doing
this, not IOMMU code. *Especially* not Intel-specific IOMMU code. It
does not live here.

If anything, the IOMMU gives you a way to *survive* this kind of thing
and means that you might get away without quiescing devices when
otherwise you would have died.

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2] intel-iommu: Quiesce devices before disabling IOMMU

2013-09-24 Thread Joerg Roedel 
On Wed, Sep 18, 2013 at 03:09:01PM +0900, Takao Indoh wrote:
> + /*
> +  *  In the case of kdump, ioremap is needed because root-entry table
> +  *  exists in first kernel's memory area which is not mapped in second
> +  *  kernel
> +  */
> + root = (struct root_entry *)ioremap(addr, PAGE_SIZE);
> + if (!root)
> + return;
> +
> + for (bus = 0; bus < ROOT_ENTRY_NR; bus++) {
> + if (!root_present(&root[bus]))
> + continue;
> +
> + context = (struct context_entry *)ioremap(
> + root[bus].val & VTD_PAGE_MASK, PAGE_SIZE);
> + if (!context)
> + continue;
> +
> + for (devfn = 0; devfn < CONTEXT_ENTRY_NR; devfn++) {
> + if (!context_present(&context[devfn]))
> + continue;
> +
> + dev = pci_get_domain_bus_and_slot(segment, bus, devfn);
> + if (!dev)
> + continue;
> +
> + if (!pci_reset_bus(dev->bus)) /* go to next bus */
> + break;
> + else /* Try per-function reset */
> + pci_reset_function(dev);
> +
> + }
> + iounmap(context);
> + }
> + iounmap(root);

I am not convinced that this is the right approach. If a device wasn't
translated by VT-d in the old kernel doesn't mean it will not be
translated in the new kernel. How about unconditionally resetting all
PCI busses and/or functions here before IOMMU initialization proceeds?


Joerg


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

Re: [PATCH 1/2] iommu: Add iommu_fault class event to iommu trace

2013-09-24 Thread Shuah Khan 

On 09/24/2013 04:48 AM, Joerg Roedel wrote:

On Fri, Aug 16, 2013 at 11:20:16AM -0600, Shuah Khan wrote:

+DEFINE_EVENT(iommu_fault, report_fault,
+
+   TP_PROTO(struct device *dev, unsigned long iova, int flags),
+
+   TP_ARGS(dev, iova, flags)
+);


I am not entirely happy with the naming. It is better to name the class
iommu_error and the page-fault event io_page_fault. This makes it more
clear what kind of fault it is.


Joerg




Joerg,

Yes, iommu_error and io_page_fault sounds better. I will rename and send 
the updated patch.


-- Shuah

--
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah...@samsung.com | (970) 672-0658
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/1] x86/iommu: correct ICS register offset

2013-09-24 Thread Joerg Roedel 
On Tue, Sep 17, 2013 at 04:38:29PM +0800, ZhenHua wrote:
> Hi Guys,
> Though  DMAR_ICS_REG is not used yet, I think this patch is
> necessary. So please take a look at it.

You are right, my Spec says the same. It doesn't matter much since the
register seems to be unused in the VT-d driver. I applied the patch to
iommu/fixes anyway.


Joerg


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

Re: [PATCH 1/2] iommu: Add iommu_fault class event to iommu trace

2013-09-24 Thread Joerg Roedel 
On Fri, Aug 16, 2013 at 11:20:16AM -0600, Shuah Khan wrote:
> +DEFINE_EVENT(iommu_fault, report_fault,
> +
> + TP_PROTO(struct device *dev, unsigned long iova, int flags),
> +
> + TP_ARGS(dev, iova, flags)
> +);

I am not entirely happy with the naming. It is better to name the class
iommu_error and the page-fault event io_page_fault. This makes it more
clear what kind of fault it is.


Joerg


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

Re: [PATCH 0/7] iommu: Add event tracing feature to iommu

2013-09-24 Thread Joerg Roedel 
On Thu, Aug 15, 2013 at 11:59:22AM -0600, Shuah Khan wrote:
> Shuah Khan (7):
>   iommu: Add event tracing feature to iommu
>   iommu: Change iommu driver to call add_device_to_group trace event
>   iommu: Change iommu driver to call remove_device_to_group trace event
>   iommu: Change iommu driver to call attach_device_to_domain trace
> event
>   iommu: Change iommu driver to call detach_device_to_domain trace
> event
>   iommu: Change iommu driver to call map trace event
>   iommu: Change iommu driver to call unmap trace event

Applied to tracing branch, thanks Shuah.


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

Re: [PATCH] MAINTAINERS: add overall IOMMU section

2013-09-24 Thread Joerg Roedel 
On Fri, Sep 13, 2013 at 01:10:27PM -0600, Stephen Warren wrote:
> I believe that Joerg Roedel is at least the path through which
> drivers/iommu changes should be merged. Add a MAINTAINERS entry to
> make this clear, so that he's Cd'd on all relevant patches. This is
> relevant for non-AMD/Intel IOMMUs, where get_maintainers.pl doesn't
> currently remind anyone to Cc Joerg on patches.

Applied to iommu/fixes, thanks Stephen.

I always hesitated a bit to do that, but if it makes me Cc'ed on all
iommu patches from now on it is probably ok ;)


Joerg


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

Re: [GIT PULL] iommu/arm-smmu: fixes for 3.12

2013-09-24 Thread Joerg Roedel 
On Tue, Sep 17, 2013 at 02:19:49PM +0100, Will Deacon wrote:
> The following changes since commit 272b98c6455f00884f0350f775c5342358ebb73f:
> 
>   Linux 3.12-rc1 (2013-09-16 16:17:51 -0400)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git 
> for-joerg/arm-smmu/fixes

Pulled, thanks Will.


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