Re: [PATCH] iommu/vt-d: Assign old irt entries a common valid vector in kdump kernel

2016-03-02 Thread Xunlei Pang
On 03/02/2016 at 10:58 PM, Joerg Roedel wrote:
> On Wed, Mar 02, 2016 at 06:02:28PM +0800, Xunlei Pang wrote:
>> Currently, the kernel copies the old irt entries during iommu
>> initialization for kdump, so old vectors in the first kernel are
>> used but having no related kernel irq handlers set explicitly,
>> this can lead to some problems after lapics are enabled:
>>  - When some in-flight dma finished and triggered an interrupt,
>>the kernel will throw a warning message in do_IRQ() like "No
>>irq handler", because handle_irq() will return false with no
>>irq_desc handlers. This may confuse users.
>>  - When the in-flight dma interrupt arrives, and if there happens
>>to be an irq with the same vector allocated in kdump kernel,
>>it will invoke the existing ISR registered in kdump kernel as
>>if one valid interrupt in the kdump kernel happens. This might
>>cause some wrong software logic, for example if the ISR always
>>wakes up a process.
> Hmm, the current situation with misdirected irq messages in the kdump
> kernel is not different from a situation without any iommu at all,
> right?

Right, non-iommu in-flight DMA after crash also suffers from this.
I think both of them should be fixed if possible.

> And the goal of preserving the old mappings is to get as close as
> possible to the situation without iommu. This seems to carry the VT-d
> driver away from that.

Without iommu, it's not so easy to fix due to the MSI registers
located in different pci devices. But vt-d introduces a mechanism
to redirect both MSI/MSI-X and I/O APIC to a common IR table,
so we can handle that much easily with the help of the IR table.

On kdump side, present-day servers with vt-d enabled are
becoming increasingly common-place, if this does happen in
real world(usually it will), that would be hard to dig it out, so I
think it would be better if we can fix it.

Also CC kexec list

Regards,
Xunlei

>
>
>   Joerg
>

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


[PATCH] arm64/dma-mapping: Add DMA_ATTR_ALLOC_SINGLE_PAGES support

2016-03-02 Thread Yong Wu
Sometimes it is not worth for the iommu allocating big chunks.
Here we enable DMA_ATTR_ALLOC_SINGLE_PAGES which could help avoid to
allocate big chunks while iommu allocating buffer.

More information about this attribute, please check Doug's commit[1].

[1]: https://lkml.org/lkml/2016/1/11/720

Cc: Robin Murphy 
Suggested-by: Douglas Anderson 
Signed-off-by: Yong Wu 
---

Our video drivers may soon use this.

 arch/arm64/mm/dma-mapping.c |  4 ++--
 drivers/iommu/dma-iommu.c   | 14 ++
 include/linux/dma-iommu.h   |  4 ++--
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 331c4ca..3225e3ca 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -562,8 +562,8 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t 
size,
struct page **pages;
pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
 
-   pages = iommu_dma_alloc(dev, iosize, gfp, ioprot, handle,
-   flush_page);
+   pages = iommu_dma_alloc(dev, iosize, gfp, ioprot, attrs,
+   handle, flush_page);
if (!pages)
return NULL;
 
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 72d6182..3569cb6 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -190,7 +190,8 @@ static void __iommu_dma_free_pages(struct page **pages, int 
count)
kvfree(pages);
 }
 
-static struct page **__iommu_dma_alloc_pages(unsigned int count, gfp_t gfp)
+static struct page **__iommu_dma_alloc_pages(unsigned int count, gfp_t gfp,
+struct dma_attrs *attrs)
 {
struct page **pages;
unsigned int i = 0, array_size = count * sizeof(*pages);
@@ -203,6 +204,10 @@ static struct page **__iommu_dma_alloc_pages(unsigned int 
count, gfp_t gfp)
if (!pages)
return NULL;
 
+   /* Go straight to 4K chunks if caller says it's OK. */
+   if (dma_get_attr(DMA_ATTR_ALLOC_SINGLE_PAGES, attrs))
+   order = 0;
+
/* IOMMU can map any pages, so himem can also be used here */
gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
 
@@ -268,6 +273,7 @@ void iommu_dma_free(struct device *dev, struct page 
**pages, size_t size,
  * @size: Size of buffer in bytes
  * @gfp: Allocation flags
  * @prot: IOMMU mapping flags
+ * @attrs: DMA attributes flags
  * @handle: Out argument for allocated DMA handle
  * @flush_page: Arch callback which must ensure PAGE_SIZE bytes from the
  * given VA/PA are visible to the given non-coherent device.
@@ -278,8 +284,8 @@ void iommu_dma_free(struct device *dev, struct page 
**pages, size_t size,
  * Return: Array of struct page pointers describing the buffer,
  *or NULL on failure.
  */
-struct page **iommu_dma_alloc(struct device *dev, size_t size,
-   gfp_t gfp, int prot, dma_addr_t *handle,
+struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
+   int prot, struct dma_attrs *attrs, dma_addr_t *handle,
void (*flush_page)(struct device *, const void *, phys_addr_t))
 {
struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
@@ -292,7 +298,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t 
size,
 
*handle = DMA_ERROR_CODE;
 
-   pages = __iommu_dma_alloc_pages(count, gfp);
+   pages = __iommu_dma_alloc_pages(count, gfp, attrs);
if (!pages)
return NULL;
 
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index fc48103..08d9603 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -38,8 +38,8 @@ int dma_direction_to_prot(enum dma_data_direction dir, bool 
coherent);
  * These implement the bulk of the relevant DMA mapping callbacks, but require
  * the arch code to take care of attributes and cache maintenance
  */
-struct page **iommu_dma_alloc(struct device *dev, size_t size,
-   gfp_t gfp, int prot, dma_addr_t *handle,
+struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
+   int prot, struct dma_attrs *attrs, dma_addr_t *handle,
void (*flush_page)(struct device *, const void *, phys_addr_t));
 void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
dma_addr_t *handle);
-- 
1.8.1.1.dirty

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


Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704

2016-03-02 Thread Tirumalesh Chalamarla



On 03/02/2016 05:10 AM, Robin Murphy wrote:

On 24/02/16 21:13, Tirumalesh Chalamarla wrote:

Due to Errata#27704 CN88xx SMMUv2,supports  only shared ASID and VMID
namespaces; specifically within a given node SMMU0 and SMMU1 share,
as does SMMU2 and SMMU3.

This patch address these issuee by supplying asid and vmid
while calculating ASID and VMID for Thunder SMMUv2.

NOTE: resending with commit message fix.

changes from V2:
- removed *_base from DT, and replaced with compatible string

changes from V1:
- rebased on top of 16 bit VMID patch
- removed redundent options from DT
- insted of transform, DT now supplies starting ASID/VMID

Signed-off-by: Tirumalesh Chalamarla 
Signed-off-by: Akula Geethasowjanya

---
  .../devicetree/bindings/iommu/arm,smmu.txt |  1 +
  drivers/iommu/arm-smmu.c   | 48
+-


I guess Documentation/arm64/silicon-errata.txt wants updating too?


YES, will update it.

  2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 7180745..19fe6f2 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -16,6 +16,7 @@ conditions.
  "arm,mmu-400"
  "arm,mmu-401"
  "arm,mmu-500"
+"cavium,smmu-v2"


That seems a rather broad name for a compatible - will there definitely
never be another Cavium implementation of SMMUv2? Even then, if there is
future respin of CN88xx silicon where the SMMU needs some other
workaround for some other issue, will that be discoverable from
SMMU_IDR7 or similar? Even an absurdly over-specific compatible such as
"cavium,thunderx-cn88xx-pass2-smmu" would be preferable to something
which later turns out to be insufficient to uniquely identify a specific
implementation.



i don't think there is a SMMUv2 from Cavium again, there will be SMMUv3 
of course.


there will be other flavors like 86xx etc, which has same implementation 
as 88xx so will have same errata. i think this name is more appropriate.



Otherwise, modulo Will's comments, this looks a lot more reasonable than
before, thanks.

Robin.


depending on the particular implementation and/or the
version of the architecture implemented.
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 247a469..c704f88 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -326,6 +326,12 @@ struct arm_smmu_device {

  struct list_headlist;
  struct rb_rootmasters;
+/*
+ *The following fields are specific to Cavium, Thunder
+ */
+u32cavium_smmu_id;
+u32cavium_id_base;
+
  };

  struct arm_smmu_cfg {
@@ -335,8 +341,8 @@ struct arm_smmu_cfg {
  };
  #define INVALID_IRPTNDX0xff

-#define ARM_SMMU_CB_ASID(cfg)((cfg)->cbndx)
-#define ARM_SMMU_CB_VMID(cfg)((cfg)->cbndx + 1)
+#define ARM_SMMU_CB_ASID(smmu, cfg) ((u16)(smmu)->cavium_id_base +
(cfg)->cbndx)
+#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base +
(cfg)->cbndx + 1)

  enum arm_smmu_domain_stage {
  ARM_SMMU_DOMAIN_S1 = 0,
@@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
  const char *prop;
  };

+static int cavium_smmu_count;
+
  static struct arm_smmu_option_prop arm_smmu_options[] = {
  { ARM_SMMU_OPT_SECURE_CFG_ACCESS,
"calxeda,smmu-secure-config-access" },
  { 0, NULL},
@@ -575,11 +583,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)

  if (stage1) {
  base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
-writel_relaxed(ARM_SMMU_CB_ASID(cfg),
+writel_relaxed(ARM_SMMU_CB_ASID(smmu, cfg),
 base + ARM_SMMU_CB_S1_TLBIASID);
  } else {
  base = ARM_SMMU_GR0(smmu);
-writel_relaxed(ARM_SMMU_CB_VMID(cfg),
+writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg),
 base + ARM_SMMU_GR0_TLBIVMID);
  }

@@ -601,7 +609,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned
long iova, size_t size,

  if (!IS_ENABLED(CONFIG_64BIT) || smmu->version ==
ARM_SMMU_V1) {
  iova &= ~12UL;
-iova |= ARM_SMMU_CB_ASID(cfg);
+iova |= ARM_SMMU_CB_ASID(smmu, cfg);
  do {
  writel_relaxed(iova, reg);
  iova += granule;
@@ -609,7 +617,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned
long iova, size_t size,
  #ifdef CONFIG_64BIT
  } else {
  iova >>= 12;
-iova |= (u64)ARM_SMMU_CB_ASID(cfg) << 48;
+iova |= (u64)ARM_SMMU_CB_ASID(smmu, cfg) << 48;
  do {
  writeq_relaxed(iova, reg);
  

Re: [PATCH] iommu/ipmmu-vmsa: Add r8a7795 DT binding

2016-03-02 Thread Magnus Damm
On Wed, Mar 2, 2016 at 11:55 PM, Joerg Roedel  wrote:
> On Mon, Feb 29, 2016 at 11:33:09PM +0900, Magnus Damm wrote:
>> From: Magnus Damm 
>>
>> Update the IPMMU DT binding documentation to include the r8a7795 compat
>> string as well as the "renesas,ipmmu-main" property that on r8a7795 will
>> be used to describe the topology and the relationship between the various
>> cache IPMMU instances and the main IPMMU.
>>
>> Signed-off-by: Magnus Damm 
>> ---
>>
>>  Written against linux-next tag next-20160229
>>
>>  Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |   15 
>> --
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> Through which tree should this go?

Unless anyone objects may I suggest going through your tree please!

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


Re: [PATCH] iommu/vt-d: Assign old irt entries a common valid vector in kdump kernel

2016-03-02 Thread Joerg Roedel
On Wed, Mar 02, 2016 at 06:02:28PM +0800, Xunlei Pang wrote:
> Currently, the kernel copies the old irt entries during iommu
> initialization for kdump, so old vectors in the first kernel are
> used but having no related kernel irq handlers set explicitly,
> this can lead to some problems after lapics are enabled:
>  - When some in-flight dma finished and triggered an interrupt,
>the kernel will throw a warning message in do_IRQ() like "No
>irq handler", because handle_irq() will return false with no
>irq_desc handlers. This may confuse users.
>  - When the in-flight dma interrupt arrives, and if there happens
>to be an irq with the same vector allocated in kdump kernel,
>it will invoke the existing ISR registered in kdump kernel as
>if one valid interrupt in the kdump kernel happens. This might
>cause some wrong software logic, for example if the ISR always
>wakes up a process.

Hmm, the current situation with misdirected irq messages in the kdump
kernel is not different from a situation without any iommu at all,
right?

And the goal of preserving the old mappings is to get as close as
possible to the situation without iommu. This seems to carry the VT-d
driver away from that.


Joerg

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


Re: [PATCH] iommu/ipmmu-vmsa: Add r8a7795 DT binding

2016-03-02 Thread Joerg Roedel
On Mon, Feb 29, 2016 at 11:33:09PM +0900, Magnus Damm wrote:
> From: Magnus Damm 
> 
> Update the IPMMU DT binding documentation to include the r8a7795 compat
> string as well as the "renesas,ipmmu-main" property that on r8a7795 will
> be used to describe the topology and the relationship between the various
> cache IPMMU instances and the main IPMMU.
> 
> Signed-off-by: Magnus Damm 
> ---
> 
>  Written against linux-next tag next-20160229
> 
>  Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |   15 
> --
>  1 file changed, 13 insertions(+), 2 deletions(-)

Through which tree should this go?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [patch] iommu/exynos: checking for IS_ERR() instead of NULL

2016-03-02 Thread Joerg Roedel
On Wed, Mar 02, 2016 at 01:10:47PM +0300, Dan Carpenter wrote:
> of_platform_device_create() returns NULL on error, it never returns
> error pointers.
> 
> Fixes: 8ed55c812fa8 ('iommu/exynos: Init from dt-specific callback instead of 
> initcall')
> Signed-off-by: Dan Carpenter 

Applied, thanks.

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


Re: [patch 2/2] iommu/mediatek: checking for IS_ERR() instead of NULL

2016-03-02 Thread Joerg Roedel
On Wed, Mar 02, 2016 at 01:10:27PM +0300, Dan Carpenter wrote:
> of_platform_device_create() returns NULL on error, it never returns
> error pointers.
> 
> Fixes: 0df4fabe208d ('iommu/mediatek: Add mt8173 IOMMU driver')
> Signed-off-by: Dan Carpenter 

Applied, thanks.

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


Re: Bug: Freeing dma regions

2016-03-02 Thread David Kiarie
On Wed, Mar 2, 2016 at 4:45 PM, Joerg Roedel  wrote:
> On Wed, Mar 02, 2016 at 03:58:18PM +0300, David Kiarie wrote:
>> On Wed, Mar 2, 2016 at 3:35 PM, Joerg Roedel  wrote:
>> > On Wed, Mar 02, 2016 at 11:40:04AM +0300, David Kiarie wrote:
>> >> What effect is setting the value next_bit to last invalidated index
>> >> supposed to have ?
>> >
>> > The idea is to safe the IOTLB flush by not re-using the address-range
>> > until the allocator wraps around to 0 again. We only allocate address
>> > ranges between next_bit and end-of-range. This way we don't need a flush
>> > after every unmap operation.
>>
>> Okay, I'll look at this issue again.
>>
>> Last unrelated question. If I have 'iommu=pt' IOMMU doesn't seem to
>> allocate any addresses and doesn't populate the 'root page table'
>> until I try to pass-through a device. Why is that set like this ?
>
> The pt stands for passthrough. In this mode the IOMMU is enabled, but
> configured in way as if it is disabled. This means that all devices get
> access to the physical address space and no remapping through the
> DMA-API is done at all. Often this is used for performance reasons.
>
> This only changes when a device is is assigned to a guest. In that case
> it will only see the guest-physical address space via a page-table.

Thanks for the insight!

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


Re: [patch 1/2] iommu/mediatek: signedness bug in probe function

2016-03-02 Thread Joerg Roedel
On Wed, Mar 02, 2016 at 01:10:06PM +0300, Dan Carpenter wrote:
> "larb_nr" needs to be signed for the error handling to work.  "i" can
> be int as well.
> 
> Fixes: 0df4fabe208d ('iommu/mediatek: Add mt8173 IOMMU driver')
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 721ffdb..1a4022c 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -578,7 +578,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   struct resource *res;
>   struct component_match  *match = NULL;
>   void*protect;
> - unsigned inti, larb_nr;
> + int i, larb_nr;
>   int ret;
>  
>   data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);

I merged a similar fix from Andrzej Hajda yesterday.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2] iommu/io-pgtable-armv7s: Fix kmem_cache_alloc() flags

2016-03-02 Thread Joerg Roedel
On Tue, Mar 01, 2016 at 07:07:03PM +, Robin Murphy wrote:
> v2: Add the backtrace to the commit log, add Will's ack.
>  
>  drivers/iommu/io-pgtable-arm-v7s.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

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


Re: Bug: Freeing dma regions

2016-03-02 Thread Joerg Roedel
On Wed, Mar 02, 2016 at 03:58:18PM +0300, David Kiarie wrote:
> On Wed, Mar 2, 2016 at 3:35 PM, Joerg Roedel  wrote:
> > On Wed, Mar 02, 2016 at 11:40:04AM +0300, David Kiarie wrote:
> >> What effect is setting the value next_bit to last invalidated index
> >> supposed to have ?
> >
> > The idea is to safe the IOTLB flush by not re-using the address-range
> > until the allocator wraps around to 0 again. We only allocate address
> > ranges between next_bit and end-of-range. This way we don't need a flush
> > after every unmap operation.
> 
> Okay, I'll look at this issue again.
> 
> Last unrelated question. If I have 'iommu=pt' IOMMU doesn't seem to
> allocate any addresses and doesn't populate the 'root page table'
> until I try to pass-through a device. Why is that set like this ?

The pt stands for passthrough. In this mode the IOMMU is enabled, but
configured in way as if it is disabled. This means that all devices get
access to the physical address space and no remapping through the
DMA-API is done at all. Often this is used for performance reasons.

This only changes when a device is is assigned to a guest. In that case
it will only see the guest-physical address space via a page-table.


Joerg

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


Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704

2016-03-02 Thread Will Deacon
On Wed, Mar 02, 2016 at 03:23:57AM +, Chalamarla, Tirumalesh wrote:
> On 3/1/16, 7:07 PM, "Will Deacon"  wrote:
> >On Wed, Feb 24, 2016 at 01:13:53PM -0800, Tirumalesh Chalamarla wrote:
> >> +  smmu->cavium_smmu_id = cavium_smmu_count;
> >> +  cavium_smmu_count++;
> >> +  smmu->cavium_id_base =
> >> +  (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
> >
> >Can you not use num_context_banks here, instead of the constant?
> We need total context banks so far, so ARM_SMMU_MAX_CBS is best option.
> For Thunder both are same anyway. 

Hmm, so couldn't you instead just update a running total as you go along?
That is, initialise it to zero, then atomic_add_return(num_context_banks)
when you probe?

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


Re: [PATCH 11/12] iommu/arm-smmu: Generic IOMMU DT bindings support

2016-03-02 Thread Robin Murphy

On 29/02/16 18:09, Sricharan wrote:

Hi Robin,


-Original Message-


[...]


+static int __init arm_smmu_of_init(struct device_node *np) {
+   struct arm_smmu_device *smmu;
+   struct platform_device *pdev;
+   int ret = arm_smmu_init();
+
+   if (ret)
+   return ret;
+
+   pdev = of_platform_device_create(np, NULL,
platform_bus_type.dev_root);
+   if (!pdev)
+   return -ENODEV;
+
+   smmu = platform_get_drvdata(pdev);
+   of_iommu_set_ops(np, _smmu_ops);
+
+   return 0;
+}
+IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1",
arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2",
arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400",
arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401",
arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500",
arm_smmu_of_init);
+

  Thanks for this series. I am going to use and test this. Also I wanted to
  ask about the iommu probe deferral series [1] to avoid early device
registration and wanted know the direction on that ?


It's certainly on my near-term to-do list to revisit. I recall running 
into problems with that series if the IOMMU was ready but the device 
itself then requested probe deferral, and I have vague memories of 
thinking more needed to be done generally around the failure/device 
teardown path too. I also had high hopes for the on-demand device 
probing series from around the same time[2], which would have helped 
simplify things quite a bit, but that also seems to have died after a 
brief stint breaking things in -next.


Anyway, Marc reckons that we also have the exact same probe-dependency 
problem for things like IRQ-MSI bridges, so I'll be looking into a more 
general solution at some point unless anyone wants to beat me to it ;)


Thanks,
Robin.

[2]:http://thread.gmane.org/gmane.linux.acpi.devel/78833



[1] http://lkml.iu.edu/hypermail/linux/kernel/1505.3/03280.html

Regards,
  Sricharan


___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



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


Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704

2016-03-02 Thread Robin Murphy

On 24/02/16 21:13, Tirumalesh Chalamarla wrote:

Due to Errata#27704 CN88xx SMMUv2,supports  only shared ASID and VMID
namespaces; specifically within a given node SMMU0 and SMMU1 share,
as does SMMU2 and SMMU3.

This patch address these issuee by supplying asid and vmid
while calculating ASID and VMID for Thunder SMMUv2.

NOTE: resending with commit message fix.

changes from V2:
- removed *_base from DT, and replaced with compatible string

changes from V1:
- rebased on top of 16 bit VMID patch
- removed redundent options from DT
- insted of transform, DT now supplies starting ASID/VMID

Signed-off-by: Tirumalesh Chalamarla 
Signed-off-by: Akula Geethasowjanya 
---
  .../devicetree/bindings/iommu/arm,smmu.txt |  1 +
  drivers/iommu/arm-smmu.c   | 48 +-


I guess Documentation/arm64/silicon-errata.txt wants updating too?


  2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt 
b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 7180745..19fe6f2 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -16,6 +16,7 @@ conditions.
  "arm,mmu-400"
  "arm,mmu-401"
  "arm,mmu-500"
+"cavium,smmu-v2"


That seems a rather broad name for a compatible - will there definitely 
never be another Cavium implementation of SMMUv2? Even then, if there is 
future respin of CN88xx silicon where the SMMU needs some other 
workaround for some other issue, will that be discoverable from 
SMMU_IDR7 or similar? Even an absurdly over-specific compatible such as 
"cavium,thunderx-cn88xx-pass2-smmu" would be preferable to something 
which later turns out to be insufficient to uniquely identify a specific 
implementation.


Otherwise, modulo Will's comments, this looks a lot more reasonable than 
before, thanks.


Robin.


depending on the particular implementation and/or the
version of the architecture implemented.
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 247a469..c704f88 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -326,6 +326,12 @@ struct arm_smmu_device {

struct list_headlist;
struct rb_root  masters;
+   /*
+*The following fields are specific to Cavium, Thunder
+*/
+   u32 cavium_smmu_id;
+   u32 cavium_id_base;
+
  };

  struct arm_smmu_cfg {
@@ -335,8 +341,8 @@ struct arm_smmu_cfg {
  };
  #define INVALID_IRPTNDX   0xff

-#define ARM_SMMU_CB_ASID(cfg)  ((cfg)->cbndx)
-#define ARM_SMMU_CB_VMID(cfg)  ((cfg)->cbndx + 1)
+#define ARM_SMMU_CB_ASID(smmu, cfg) ((u16)(smmu)->cavium_id_base + 
(cfg)->cbndx)
+#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + 
(cfg)->cbndx + 1)

  enum arm_smmu_domain_stage {
ARM_SMMU_DOMAIN_S1 = 0,
@@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
const char *prop;
  };

+static int cavium_smmu_count;
+
  static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
{ 0, NULL},
@@ -575,11 +583,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)

if (stage1) {
base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
-   writel_relaxed(ARM_SMMU_CB_ASID(cfg),
+   writel_relaxed(ARM_SMMU_CB_ASID(smmu, cfg),
   base + ARM_SMMU_CB_S1_TLBIASID);
} else {
base = ARM_SMMU_GR0(smmu);
-   writel_relaxed(ARM_SMMU_CB_VMID(cfg),
+   writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg),
   base + ARM_SMMU_GR0_TLBIVMID);
}

@@ -601,7 +609,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long 
iova, size_t size,

if (!IS_ENABLED(CONFIG_64BIT) || smmu->version == ARM_SMMU_V1) {
iova &= ~12UL;
-   iova |= ARM_SMMU_CB_ASID(cfg);
+   iova |= ARM_SMMU_CB_ASID(smmu, cfg);
do {
writel_relaxed(iova, reg);
iova += granule;
@@ -609,7 +617,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long 
iova, size_t size,
  #ifdef CONFIG_64BIT
} else {
iova >>= 12;
-   iova |= (u64)ARM_SMMU_CB_ASID(cfg) << 48;
+   iova |= (u64)ARM_SMMU_CB_ASID(smmu, cfg) << 48;
do {
writeq_relaxed(iova, reg);
 

Re: Bug: Freeing dma regions

2016-03-02 Thread David Kiarie
On Wed, Mar 2, 2016 at 3:35 PM, Joerg Roedel  wrote:
> On Wed, Mar 02, 2016 at 11:40:04AM +0300, David Kiarie wrote:
>> What effect is setting the value next_bit to last invalidated index
>> supposed to have ?
>
> The idea is to safe the IOTLB flush by not re-using the address-range
> until the allocator wraps around to 0 again. We only allocate address
> ranges between next_bit and end-of-range. This way we don't need a flush
> after every unmap operation.

Okay, I'll look at this issue again.

Last unrelated question. If I have 'iommu=pt' IOMMU doesn't seem to
allocate any addresses and doesn't populate the 'root page table'
until I try to pass-through a device. Why is that set like this ?

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


Re: [patch] iommu/exynos: checking for IS_ERR() instead of NULL

2016-03-02 Thread Marek Szyprowski

Hello,

On 2016-03-02 11:10, Dan Carpenter wrote:

of_platform_device_create() returns NULL on error, it never returns
error pointers.

Fixes: 8ed55c812fa8 ('iommu/exynos: Init from dt-specific callback instead of 
initcall')
Signed-off-by: Dan Carpenter 


Acked-by: Marek Szyprowski 


diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index b066504..cb57bda 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1347,8 +1347,8 @@ static int __init exynos_iommu_of_setup(struct 
device_node *np)
exynos_iommu_init();
  
  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);

-   if (IS_ERR(pdev))
-   return PTR_ERR(pdev);
+   if (!pdev)
+   return -ENOMEM;
  
  	/*

 * use the first registered sysmmu device for performing




Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

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


Re: Bug: Freeing dma regions

2016-03-02 Thread Joerg Roedel
On Wed, Mar 02, 2016 at 11:40:04AM +0300, David Kiarie wrote:
> What effect is setting the value next_bit to last invalidated index
> supposed to have ?

The idea is to safe the IOTLB flush by not re-using the address-range
until the allocator wraps around to 0 again. We only allocate address
ranges between next_bit and end-of-range. This way we don't need a flush
after every unmap operation.



Joerg

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


Re: [PATCH 1/2] iommu/io-pgtable: Add MTK 4GB mode in Short-descriptor

2016-03-02 Thread Robin Murphy

Hi Yong,

On 23/02/16 23:02, Yong Wu wrote:

Mediatek extend bit9 in the lvl1 and lvl2 pgtable descriptor of the
Short-descriptor as the 4GB mode in which the dram size will be
over 4GB.

We add a special quirk for this MTK-4GB mode, And in the standard
spec, Bit9 in the lvl1 is "IMPLEMENTATION DEFINED", while it's AP[2]
in the lvl2, therefore if this quirk is enabled, NO_PERMS is also
expected.


Would you be able to explain exactly what this "4GB mode" actually is? 
I've been trying to make sense of it from the original M4U patches and 
the patch for the I2C driver, but it has me completely baffled.


Is it simply used as an extra physical address bit (although surely that 
would make it "8GB mode"?), or does it do something crazier like 
toggling an interconnect remap that shifts the output addresses up by 
1GB to be relative to the base of DRAM, like a dma_pfn_offset?


I guess from the look of these patches that it doesn't change anything 
between the masters and the M4U, so input addresses are still 32 bits 
and we don't need extended tables, right? Furthermore, what about the 
TTBRs? Does the level 1 table still have to be below 4GB?



Signed-off-by: Yong Wu 
---
In arm_v7s_init_pte, We add bit9 if the 4GB mode is enabled no matter
the current pa is over 4GB or not.


Either way I can't comprehend how it could be fine to just enable 
unconditionally without doing _something_ with the actual addresses.



  drivers/iommu/io-pgtable-arm-v7s.c | 14 +-
  drivers/iommu/io-pgtable.h |  6 ++
  2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm-v7s.c 
b/drivers/iommu/io-pgtable-arm-v7s.c
index 9fcceb1..bf6a6f0 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -121,6 +121,8 @@
  #define ARM_V7S_TEX_MASK  0x7
  #define ARM_V7S_ATTR_TEX(val) (((val) & ARM_V7S_TEX_MASK) << 
ARM_V7S_TEX_SHIFT)

+#define ARM_V7S_ATTR_MTK_4GB   BIT(9) /* MTK extend it for 4GB mode */
+
  /* *well, except for TEX on level 2 large pages, of course :( */
  #define ARM_V7S_CONT_PAGE_TEX_SHIFT   6
  #define ARM_V7S_CONT_PAGE_TEX_MASK(ARM_V7S_TEX_MASK << 
ARM_V7S_CONT_PAGE_TEX_SHIFT)
@@ -364,6 +366,9 @@ static int arm_v7s_init_pte(struct arm_v7s_io_pgtable *data,
if (lvl == 1 && (cfg->quirks & IO_PGTABLE_QUIRK_ARM_NS))
pte |= ARM_V7S_ATTR_NS_SECTION;

+   if (cfg->quirks & IO_PGTABLE_QUIRK_MTK_4GB_EXT)
+   pte |= ARM_V7S_ATTR_MTK_4GB;
+
if (num_entries > 1)
pte = arm_v7s_pte_to_cont(pte, lvl);

@@ -625,9 +630,16 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct 
io_pgtable_cfg *cfg,

if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NO_PERMS |
-   IO_PGTABLE_QUIRK_TLBI_ON_MAP))
+   IO_PGTABLE_QUIRK_TLBI_ON_MAP |
+   IO_PGTABLE_QUIRK_MTK_4GB_EXT))
return NULL;

+   /* If MTK_4GB_EXT is enabled, the NO_PERMS is also expected. */
+   if (cfg->quirks & IO_PGTABLE_QUIRK_MTK_4GB_EXT) {
+   if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_PERMS))


Ha, I wasn't expecting to see Will's argument against generic 
quirk-checking be vindicated quite so soon :)


Anyway, no need for braces and nested ifs:

if (cfg->quirks & IO_PGTABLE_QUIRK_MTK_4GB_EXT &&
!(cfg->quirks & IO_PGTABLE_QUIRK_NO_PERMS))


+   return NULL;
+   }
+
data = kmalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return NULL;
diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
index d4f5027..a84a60a 100644
--- a/drivers/iommu/io-pgtable.h
+++ b/drivers/iommu/io-pgtable.h
@@ -60,10 +60,16 @@ struct io_pgtable_cfg {
 * IO_PGTABLE_QUIRK_TLBI_ON_MAP: If the format forbids caching invalid
 *  (unmapped) entries but the hardware might do so anyway, perform
 *  TLB maintenance when mapping as well as when unmapping.
+*
+* IO_PGTABLE_QUIRK_MTK_4GB_EXT: Mediatek extend bit9 in the lvl1 and
+*  lvl2 descriptor of the Short-descriptor as the 4GB mode.
+*  Note that: Bit9 in the lvl1 is "IMPLEMENTATION DEFINED", while
+*  it is AP[2] in the lvl2.


Unfortunately that comment doesn't really explain anything - I'd be 
happy to suggest a more helpful wording, If only I understood what it 
actually did.


Robin.


 */
#define IO_PGTABLE_QUIRK_ARM_NS BIT(0)
#define IO_PGTABLE_QUIRK_NO_PERMS   BIT(1)
#define IO_PGTABLE_QUIRK_TLBI_ON_MAPBIT(2)
+   #define IO_PGTABLE_QUIRK_MTK_4GB_EXTBIT(3)
unsigned long   quirks;
unsigned long   pgsize_bitmap;
unsigned intias;



___
iommu mailing 

Re: [RFC v5 00/17] KVM PCIe/MSI passthrough on ARM/ARM64

2016-03-02 Thread Eric Auger
Hi Manish,
On 03/02/2016 09:11 AM, Jaggi, Manish wrote:
> 
> 
>>> From: Eric Auger 
>>> Sent: Tuesday, March 1, 2016 11:57 PM
>>> To: eric.au...@st.com; eric.au...@linaro.org; robin.mur...@arm.com; 
>>> alex.william...@redhat.com; will.dea...@arm.com; j...@8bytes.org; 
>>> t...@linutronix.de; >>ja...@lakedaemon.net; marc.zyng...@arm.com; 
>>> christoffer.d...@linaro.org; linux-arm-ker...@lists.infradead.org; 
>>> kvm...@lists.cs.columbia.edu; k...@vger.kernel.org
>>> Cc: suravee.suthikulpa...@amd.com; patc...@linaro.org; 
>>> linux-ker...@vger.kernel.org; Jaggi, Manish; bharat.bhus...@freescale.com; 
>>> >>pranav.sawargaon...@gmail.com; p.fe...@samsung.com; 
>>> iommu@lists.linux-foundation.org
>>> Subject: [RFC v5 00/17] KVM PCIe/MSI passthrough on ARM/ARM64
> 
>>> This series addresses KVM PCIe passthrough with MSI enabled on ARM/ARM64.
>>> It pursues the efforts done on [1], [2], [3]. It also aims at covering the
>>> same need on PowerPC platforms although the same kind of integration
>>> .should be carried out.
>>>
> [snip]
>>> - Not tested: ARM GICv3 ITS
> 
> [snip]
>>> QEMU Integration:
>>> [RFC v2 0/8] KVM PCI/MSI passthrough with mach-virt
>>> (http://lists.gnu.org/archive/html/qemu-arm/2016-01/msg00444.html)
>>> https://git.linaro.org/people/eric.auger/qemu.git/shortlog/refs/heads/v2.5.0-pci-passthrough-rfc-v2
> 
> For gicv3 its, I believe, the below series for qemu and kernel is required 
> for gicv3-its
> 
> [RFC PATCH v3 0/5] vITS support
> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg05197.html
> 
> and in kernel CONFIG_HAVE_KVM_MSI must be enabled so that qemu sees MSI 
> capability KVM_CAP_SIGNAL_MSI
> 
> This has a dependency on gsi routing support
> KVM: arm/arm64: gsi routing support
> https://lkml.org/lkml/2015/6/29/290

which has a dependency on Andre's ITS emulation series too.

The Kernel series will be resent soon on top on new vgic design.
> 
> I had both the above series in 4.2 in my local 4.2 tree. 

Did you have a chance to test with GICv3 ITS already?

Best Regards

Eric


> 
> BR
> -Manish
> 
> 

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


Re: [PATCH v10 0/5] MT8173 IOMMU SUPPORT

2016-03-02 Thread Arnd Bergmann
On Wednesday 02 March 2016 11:48:43 Matthias Brugger wrote:
> 
> On 26/02/16 12:06, Joerg Roedel wrote:
> > Hi Philipp,
> >
> > On Fri, Feb 26, 2016 at 09:55:55AM +0100, Philipp Zabel wrote:
> >> Do you have a stable branch that I could base the mediatek-drm patches
> >> on and that I may include in a pull request to be merged via drm-next?
> >
> > You can base it on the arm/mediatek branch of the iommu tree. I'll push
> > it out in the next minutes.
> >
> 
> I realized that Joerg accidentally merged the dts parts as well through 
> his branch.
> 
> As expected this patches conflict when merging the arm-soc tree.
> 
> Shall Joerg drops the patches and you take it through your branch, or
> I queue them for the next release cycle?

I think this time, we can live with the conflict, it's easy enough
to resolve when Linus pulls both branches.

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


Re: [PATCH v10 0/5] MT8173 IOMMU SUPPORT

2016-03-02 Thread Matthias Brugger

Hi Arnd and Olof,

On 26/02/16 12:06, Joerg Roedel wrote:

Hi Philipp,

On Fri, Feb 26, 2016 at 09:55:55AM +0100, Philipp Zabel wrote:

Do you have a stable branch that I could base the mediatek-drm patches
on and that I may include in a pull request to be merged via drm-next?


You can base it on the arm/mediatek branch of the iommu tree. I'll push
it out in the next minutes.



I realized that Joerg accidentally merged the dts parts as well through 
his branch.


As expected this patches conflict when merging the arm-soc tree.

Shall Joerg drops the patches and you take it through your branch, or
I queue them for the next release cycle?

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


[patch 2/2] iommu/mediatek: checking for IS_ERR() instead of NULL

2016-03-02 Thread Dan Carpenter
of_platform_device_create() returns NULL on error, it never returns
error pointers.

Fixes: 0df4fabe208d ('iommu/mediatek: Add mt8173 IOMMU driver')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 1a4022c..4682da4 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -628,7 +628,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
plarbdev = of_platform_device_create(
larbnode, NULL,
platform_bus_type.dev_root);
-   if (IS_ERR(plarbdev))
+   if (!plarbdev)
return -EPROBE_DEFER;
}
data->smi_imu.larb_imu[i].dev = >dev;
@@ -721,8 +721,8 @@ static int mtk_iommu_init_fn(struct device_node *np)
struct platform_device *pdev;
 
pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
-   if (IS_ERR(pdev))
-   return PTR_ERR(pdev);
+   if (!pdev)
+   return -ENOMEM;
 
ret = platform_driver_register(_iommu_driver);
if (ret) {
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[patch 1/2] iommu/mediatek: signedness bug in probe function

2016-03-02 Thread Dan Carpenter
"larb_nr" needs to be signed for the error handling to work.  "i" can
be int as well.

Fixes: 0df4fabe208d ('iommu/mediatek: Add mt8173 IOMMU driver')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 721ffdb..1a4022c 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -578,7 +578,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
struct resource *res;
struct component_match  *match = NULL;
void*protect;
-   unsigned inti, larb_nr;
+   int i, larb_nr;
int ret;
 
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[patch] iommu/exynos: checking for IS_ERR() instead of NULL

2016-03-02 Thread Dan Carpenter
of_platform_device_create() returns NULL on error, it never returns
error pointers.

Fixes: 8ed55c812fa8 ('iommu/exynos: Init from dt-specific callback instead of 
initcall')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index b066504..cb57bda 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1347,8 +1347,8 @@ static int __init exynos_iommu_of_setup(struct 
device_node *np)
exynos_iommu_init();
 
pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
-   if (IS_ERR(pdev))
-   return PTR_ERR(pdev);
+   if (!pdev)
+   return -ENOMEM;
 
/*
 * use the first registered sysmmu device for performing
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/vt-d: Assign old irt entries a common valid vector in kdump kernel

2016-03-02 Thread Xunlei Pang
Currently, the kernel copies the old irt entries during iommu
initialization for kdump, so old vectors in the first kernel are
used but having no related kernel irq handlers set explicitly,
this can lead to some problems after lapics are enabled:
 - When some in-flight dma finished and triggered an interrupt,
   the kernel will throw a warning message in do_IRQ() like "No
   irq handler", because handle_irq() will return false with no
   irq_desc handlers. This may confuse users.
 - When the in-flight dma interrupt arrives, and if there happens
   to be an irq with the same vector allocated in kdump kernel,
   it will invoke the existing ISR registered in kdump kernel as
   if one valid interrupt in the kdump kernel happens. This might
   cause some wrong software logic, for example if the ISR always
   wakes up a process.

This patch addresses the issue by modifying the old irt entries
to use a special allocated vector and related irq handler.

Signed-off-by: Xunlei Pang 
---
 drivers/iommu/intel_irq_remapping.c | 78 -
 1 file changed, 68 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c 
b/drivers/iommu/intel_irq_remapping.c
index ac59692..e044e0f 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -400,6 +400,68 @@ static int set_msi_sid(struct irte *irte, struct pci_dev 
*dev)
return 0;
 }
 
+/*
+ * Old entries may contain vector data with no related irq handlers
+ * in the new kernel, replace them with this common vector assigned
+ * with related irq handler.
+ */
+static u8 redirected_vector;
+
+static void lapic_mask_ack(struct irq_data *dummy)
+{
+   /* We don't know how to mask it, only do lapic-level ack */
+   ack_APIC_irq();
+}
+
+static struct irq_chip fake_chip = {
+   .irq_mask_ack = lapic_mask_ack,
+};
+
+/* Allocate the common vector for all iommus' old irte */
+static void iommu_alloc_redirected_vector(struct intel_iommu *iommu)
+{
+   struct irq_alloc_info info;
+   int irq;
+
+   if (redirected_vector)
+   return;
+
+   init_irq_alloc_info(, NULL);
+   irq = irq_domain_alloc_irqs(x86_vector_domain, 1, -1, );
+   if (irq < 0) {
+   pr_err("Failed to alloc redirected vector for %s's old IRTEs\n",
+   iommu->name);
+   return;
+   }
+
+   /*
+* NOTE: we can assign the edge handler here to be shared
+* by all irt entries with the same redirected_vector but
+* different trigger mode, because edge and level handlers
+* behave similarly with disabled irq or no actions.
+*/
+   irq_set_chip_and_handler(irq, _chip, handle_edge_irq);
+   redirected_vector = irqd_cfg(irq_get_irq_data(irq))->vector;
+
+   pr_info("Redirect %s's old IRTEs to use Vector%u and IRQ%d\n",
+   iommu->name, redirected_vector, irq);
+}
+
+/* Make sure we have a valid vector and irq handler after copy */
+static inline void iommu_assign_old_irte(struct ir_table *new_table,
+   struct irte *old_entry, unsigned int i)
+{
+   struct irte *new_entry = _table->base[i];
+
+   if (!old_entry->present)
+   return;
+
+   memcpy(new_entry, old_entry, sizeof(struct irte));
+   if (redirected_vector)
+   new_entry->vector = redirected_vector;
+   bitmap_set(new_table->bitmap, i, 1);
+}
+
 static int iommu_load_old_irte(struct intel_iommu *iommu)
 {
struct irte *old_ir_table;
@@ -430,21 +492,17 @@ static int iommu_load_old_irte(struct intel_iommu *iommu)
if (!old_ir_table)
return -ENOMEM;
 
-   /* Copy data over */
-   memcpy(iommu->ir_table->base, old_ir_table, size);
-
-   __iommu_flush_cache(iommu, iommu->ir_table->base, size);
+   iommu_alloc_redirected_vector(iommu);
 
/*
-* Now check the table for used entries and mark those as
-* allocated in the bitmap
+* Copy and adjust old data, and check the table for used entries,
+* and mark those as allocated in the bitmap
 */
-   for (i = 0; i < INTR_REMAP_TABLE_ENTRIES; i++) {
-   if (iommu->ir_table->base[i].present)
-   bitmap_set(iommu->ir_table->bitmap, i, 1);
-   }
+   for (i = 0; i < INTR_REMAP_TABLE_ENTRIES; i++)
+   iommu_assign_old_irte(iommu->ir_table, _ir_table[i], i);
 
memunmap(old_ir_table);
+   __iommu_flush_cache(iommu, iommu->ir_table->base, size);
 
return 0;
 }
-- 
2.5.0

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


Re: Bug: Freeing dma regions

2016-03-02 Thread David Kiarie
On Wed, Mar 2, 2016 at 11:37 AM, David Kiarie  wrote:
> On Wed, Mar 2, 2016 at 11:29 AM, Joerg Roedel  wrote:
>> On Wed, Mar 02, 2016 at 03:45:39AM +0300, David Kiarie wrote:
>>> Hello,
>>>
>>> This patch seems to have introduced a bug -
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4eeca8c5e72fad752eba9efc293c924d65faa86e
>>>
>>> As the commit message says, it should check for regions behind the
>>> next_bit but it checks for regions beyond.
>>
>> Hmm, the patch looks good to me. Do you actually see a reproducible
>> issue with it?
>
> Yep,  I can't tell what exactly is happening but it seems to break
> Qemu emulated IOMMU. I haven't tried on real hardware.

What effect is setting the value next_bit to last invalidated index
supposed to have ?

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


Re: Bug: Freeing dma regions

2016-03-02 Thread David Kiarie
On Wed, Mar 2, 2016 at 11:29 AM, Joerg Roedel  wrote:
> On Wed, Mar 02, 2016 at 03:45:39AM +0300, David Kiarie wrote:
>> Hello,
>>
>> This patch seems to have introduced a bug -
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4eeca8c5e72fad752eba9efc293c924d65faa86e
>>
>> As the commit message says, it should check for regions behind the
>> next_bit but it checks for regions beyond.
>
> Hmm, the patch looks good to me. Do you actually see a reproducible
> issue with it?

Yep,  I can't tell what exactly is happening but it seems to break
Qemu emulated IOMMU. I haven't tried on real hardware.

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


Re: Bug: Freeing dma regions

2016-03-02 Thread Joerg Roedel
On Wed, Mar 02, 2016 at 03:45:39AM +0300, David Kiarie wrote:
> Hello,
> 
> This patch seems to have introduced a bug -
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4eeca8c5e72fad752eba9efc293c924d65faa86e
> 
> As the commit message says, it should check for regions behind the
> next_bit but it checks for regions beyond.

Hmm, the patch looks good to me. Do you actually see a reproducible
issue with it?


Joerg

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


Re: [PATCH] iommu/ipmmu-vmsa: Add r8a7795 DT binding

2016-03-02 Thread Geert Uytterhoeven
Hi Magnus,

On Mon, Feb 29, 2016 at 3:33 PM, Magnus Damm  wrote:
> From: Magnus Damm 
>
> Update the IPMMU DT binding documentation to include the r8a7795 compat
> string as well as the "renesas,ipmmu-main" property that on r8a7795 will
> be used to describe the topology and the relationship between the various
> cache IPMMU instances and the main IPMMU.
>
> Signed-off-by: Magnus Damm 

Thanks for your patch!

> ---
>
>  Written against linux-next tag next-20160229
>
>  Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt |   15 
> --
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> --- 0001/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> +++ work/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt 
> 2016-02-29 23:25:15.540513000 +0900
> @@ -7,23 +7,34 @@ connected to the IPMMU through a port ca
>
>  Required Properties:
>
> -  - compatible: Must contain SoC-specific and generic entries from below.
> +  - compatible: Must contain SoC-specific and generic entry below in case
> +the device is compatible with the R-Car Gen2 VMSA-compatible IPMMU.
>
>  - "renesas,ipmmu-r8a73a4" for the R8A73A4 (R-Mobile APE6) IPMMU.
>  - "renesas,ipmmu-r8a7790" for the R8A7790 (R-Car H2) IPMMU.
>  - "renesas,ipmmu-r8a7791" for the R8A7791 (R-Car M2-W) IPMMU.
>  - "renesas,ipmmu-r8a7793" for the R8A7793 (R-Car M2-N) IPMMU.
>  - "renesas,ipmmu-r8a7794" for the R8A7794 (R-Car E2) IPMMU.
> +- "renesas,ipmmu-r8a7795" for the R8A7795 (R-Car H3) IPMMU.
>  - "renesas,ipmmu-vmsa" for generic R-Car Gen2 VMSA-compatible IPMMU.
>
>- reg: Base address and size of the IPMMU registers.
>- interrupts: Specifiers for the MMU fault interrupts. For instances that
>  support secure mode two interrupts must be specified, for non-secure and
>  secure mode, in that order. For instances that don't support secure mode 
> a
> -single interrupt must be specified.
> +single interrupt must be specified. Not required for cache IPMMUs.
>
>- #iommu-cells: Must be 1.
>
> +Optional properties:
> +
> +  - renesas,ipmmu-main: reference to the main IPMMU instance in two cells.
> +The first cell is a phandle to the main IPMMU and the second cell is
> +the interrupt bit number associated with the particular cache IPMMU 
> device.
> +The interrupt bit number needs to match the main IPMMU IMSSTR register.
> +Only used by cache IPMMU instances.
> +
> +

I think it would be good to include an example of how the optional properties
should be used.

Gr{oetje,eeting}s,

Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2] iommu/io-pgtable-armv7s: Fix kmem_cache_alloc() flags

2016-03-02 Thread Geert Uytterhoeven
On Tue, Mar 1, 2016 at 8:07 PM, Robin Murphy  wrote:
> Whilst the default SLUB allocator happily just merges the original
> allocation flags from kmem_cache_create() with those passed through
> kmem_cache_alloc(), there is a code path in the SLAB allocator which
> will aggressively BUG_ON() if the cache was created with SLAB_CACHE_DMA
> but GFP_DMA is not specified for an allocation:
>
>   kernel BUG at mm/slab.c:2536!
>   Internal error: Oops - BUG: 0 [#1] SMP ARM
>   Modules linked in:[1.299311] Modules linked in:
>
>   CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>   4.5.0-rc6-koelsch-05892-ge7e45ad53ab6795e #2270
>   Hardware name: Generic R8A7791 (Flattened Device Tree)
>   task: ef422040 ti: ef442000 task.ti: ef442000
>   PC is at cache_alloc_refill+0x2a0/0x530
>   LR is at _raw_spin_unlock+0x8/0xc
> ...
>   [] (cache_alloc_refill) from [] 
> (kmem_cache_alloc+0x7c/0xd4)
>   [] (kmem_cache_alloc) from []
>   (__arm_v7s_alloc_table+0x5c/0x278)
>   [] (__arm_v7s_alloc_table) from []
>   (__arm_v7s_map.constprop.6+0x68/0x25c)
>   [] (__arm_v7s_map.constprop.6) from []
>   (arm_v7s_map+0x34/0xa4)
>   [] (arm_v7s_map) from [] 
> (arm_v7s_do_selftests+0x140/0x418)
>   [] (arm_v7s_do_selftests) from []
>   (do_one_initcall+0x100/0x1b4)
>   [] (do_one_initcall) from []
>   (kernel_init_freeable+0x120/0x1e8)
>   [] (kernel_init_freeable) from [] (kernel_init+0x8/0xec)
>   [] (kernel_init) from [] (ret_from_fork+0x14/0x2c)
>   Code: 1a03 e7f001f2 e3130001 0a00 (e7f001f2)
>   ---[ end trace 190f6f6b84352efd ]---
>
> Keep the peace by adding GFP_DMA when allocating a table.

Thanks, this makes the test succeed:

arm-v7s io-pgtable: self test ok

> Reported-by: Geert Uytterhoeven 
> Acked-by: Will Deacon 
> Signed-off-by: Robin Murphy 

Tested-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu