Re: [PATCH v2] arm64/dma-mapping: fix DMA_ATTR_FORCE_CONTIGUOUS mmaping code

[Catalin Marinas]

Hi Geert,

On Fri, Apr 21, 2017 at 07:39:43PM +0200, Geert Uytterhoeven wrote:
> On Fri, Apr 21, 2017 at 7:32 PM, Catalin Marinas
>  wrote:
> > On Fri, Mar 31, 2017 at 01:02:51PM +0200, Andrzej Hajda wrote:
> >> In case of DMA_ATTR_FORCE_CONTIGUOUS allocations vm_area->pages
> >> is invalid. __iommu_mmap_attrs and __iommu_get_sgtable should not
> >> use it and take advantage of contiguous nature of the allocation.
> >
> > As I replied to your original patch, I think
> > dma_common_contiguous_remap() should be fixed not to leave a dangling
> > area->pages pointer.
> >
> >>  arch/arm64/mm/dma-mapping.c | 17 -
> >>  1 file changed, 16 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> >> index f7b5401..41c7c36 100644
> >> --- a/arch/arm64/mm/dma-mapping.c
> >> +++ b/arch/arm64/mm/dma-mapping.c
> >> @@ -703,6 +703,10 @@ static int __iommu_mmap_attrs(struct device *dev, 
> >> struct vm_area_struct *vma,
> >>   if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, ))
> >>   return ret;
> >>
> >> + if (attrs & DMA_ATTR_FORCE_CONTIGUOUS)
> >> + return vm_iomap_memory(vma,
> >> + page_to_phys(vmalloc_to_page(cpu_addr)), 
> >> size);
> >
> > I replied to Geert's patch on whether we actually need to call
> > dma_common_contiguous_remap() in __iommu_alloc_attrs() if the device is
> > coherent. We don't do this for the swiotlb allocation. If we are going
> > to change this, cpu_addr may or may not be in the vmalloc range and
> > vmalloc_to_page() would fail (I guess we could add an is_vmalloc_addr()
> > check).
> >
> > Alternatively, just open-code dma_common_contiguous_remap() in
> > __iommu_alloc_attrs() to keep a valid area->pages around (I think Geert
> > already suggested this). AFAICT, arch/arm does this already in its own
> > __iommu_alloc_buffer().
> >
> > Yet another option would be for iommu_dma_alloc() to understand
> > DMA_ATTR_FORCE_CONTIGUOUS and remove the arm64-specific handling.
> 
> That was actually the approach I took in my v1.
> V2 changed that to provide standalone iommu_dma_{alloc,free}_contiguous()
> functions.
> V3 changed that to call everything directly from the arm64 code.
> ...

I now read through the past discussions (as I ignored the threads
previously). I got Robin's point of not extending iommu_dma_alloc()
(though it looked simpler) and open-coding dma_common_contiguous_remap()
wouldn't make sense either as a way to pass this buffer to
iommu_dma_mmap() since it wasn't returned by iommu_dma_alloc().

So I think we should just fall back to the swiotlb ops for the mmap and
get_sgtable but since we don't have a dma_addr_t handle (we only have
the one of the other side of the IOMMU), we'd need to factor out the
common code from __swiotlb_mmap into a __swiotlb_mmap_page (similarly
for __swiotlb_get_sgtable). The __iommu_* functions would call these
with the correct page (from vmalloc_to_page) if
DMA_ATTR_FORCE_CONTIGUOUS and the buffer is not a candidate for
*_from_coherent.

While fixing/removing dma_get_sgtable() is a nice goal, we first need to
address DMA_ATTR_FORCE_CONTIGUOUS on arm64 since the patch breaks
existing use-cases (and I'd like to avoid reverting this patch).

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

Re: [PATCH] iommu/arm-smmu-v3: Increase SMMU CMD queue poll timeout

[Will Deacon]

On Mon, Apr 24, 2017 at 10:26:53PM +0530, Sunil Kovvuri wrote:
> On Mon, Apr 24, 2017 at 9:38 PM, Will Deacon  wrote:
> > On Mon, Apr 24, 2017 at 05:29:36PM +0530, Geetha sowjanya wrote:
> >> From: Geetha 
> >>
> >> When large memory is being unmapped, huge no of tlb invalidation cmds are
> >> submitted followed by a SYNC command. This sometimes hits CMD queue full 
> >> and
> >> poll on queue drain is being timedout throwing error message 'CMD_SYNC 
> >> timeout'.
> >>
> >> Although there is no functional issue, error message confuses user. Hence 
> >> increased
> >> poll timeout to 500us
> >
> > Hmm, what are you doing to unmap that much? Is this VFIO teardown? Do you
> > have 7c6d90e2bb1a ("iommu/io-pgtable-arm: Fix iova_to_phys for block
> > entries") applied?
> 
> Yes it's VFIO teardown and again yes the above fix is applied.
> But i didn't get how above fix is related.
> TLB invalidation commands are submitted at 'arm_smmu_tlb_inv_range_nosync()'
> and it's a loop over granule size.
> 
> 1357 do {
> 1358 arm_smmu_cmdq_issue_cmd(smmu, );
> 1359 cmd.tlbi.addr += granule;
> 1360 } while (size -= granule);
> 
> So if invalidation size is big then huge no of invalidation commands
> will be submitted
> irrespective of fix that you pointed above, right ?

VFIO has some logic to batch up invalidations, but this didn't work properly
for us without the fix above. However, I guess you have a huge memory range
that's mapped with 2M sections or something, so there are still loads of
entries to invalidate.

I would much prefer it if VFIO could just teardown the whole address space
so that we could do an invalidate all, but there's a chicken-and-egg problem
with page accounting iirc.

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

Re: [PATCH] arm64/dma-mapping: fix DMA_ATTR_FORCE_CONTIGUOUS mmaping code

[Laura Abbott]

On 04/21/2017 09:12 AM, Catalin Marinas wrote:
> On Wed, Mar 29, 2017 at 01:55:32PM +0100, Robin Murphy wrote:
>> On 29/03/17 11:05, Andrzej Hajda wrote:
>>> In case of DMA_ATTR_FORCE_CONTIGUOUS allocations vm_area->pages
>>> is invalid. __iommu_mmap_attrs and __iommu_get_sgtable cannot use
>>> it. In first case temporary pages array is passed to iommu_dma_mmap,
>>> in 2nd case single entry sg table is created directly instead
>>> of calling helper.
>>>
>>> Fixes: 44176bb ("arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to IOMMU")
>>> Signed-off-by: Andrzej Hajda 
>>> ---
>>> Hi,
>>>
>>> I am not familiar with this framework so please don't be too cruel ;)
>>> Alternative solution I see is to always create vm_area->pages,
>>> I do not know which approach is preferred.
>>>
>>> Regards
>>> Andrzej
>>> ---
>>>  arch/arm64/mm/dma-mapping.c | 40 ++--
>>>  1 file changed, 38 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>> index f7b5401..bba2bc8 100644
>>> --- a/arch/arm64/mm/dma-mapping.c
>>> +++ b/arch/arm64/mm/dma-mapping.c
>>> @@ -704,7 +704,30 @@ static int __iommu_mmap_attrs(struct device *dev, 
>>> struct vm_area_struct *vma,
>>> return ret;
>>>  
>>> area = find_vm_area(cpu_addr);
>>> -   if (WARN_ON(!area || !area->pages))
>>> +   if (WARN_ON(!area))
>>
>> From the look of things, it doesn't seem strictly necessary to change
>> this, but whether that's a good thing is another matter. I'm not sure
>> that dma_common_contiguous_remap() should really be leaving a dangling
>> pointer in area->pages as it apparently does... :/
> 
> On this specific point, I don't think area->pages should be set either
> (cc'ing Laura). As in the vmalloc vs vmap case, area->pages when pages
> need to be freed (via vfree), which is not the case here. The
> dma_common_pages_remap() would need to set area->pages when called
> directly from the iommu DMA ops. Proposal below, not tested with the
> iommu ops. I assume the patch would cause __iommu_mmap_attrs() to return
> -ENXIO if DMA_ATTR_FORCE_CONTIGUOUS is set.
> 
> 8<-
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index efd71cf4fdea..ab7071041141 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -277,8 +277,8 @@ EXPORT_SYMBOL(dma_common_mmap);
>   * remaps an array of PAGE_SIZE pages into another vm_area
>   * Cannot be used in non-sleeping contexts
>   */
> -void *dma_common_pages_remap(struct page **pages, size_t size,
> - unsigned long vm_flags, pgprot_t prot,
> +static struct vm_struct *__dma_common_pages_remap(struct page **pages,
> + size_t size, unsigned long vm_flags, pgprot_t prot,
>   const void *caller)
>  {
>   struct vm_struct *area;
> @@ -287,13 +287,26 @@ void *dma_common_pages_remap(struct page **pages, 
> size_t size,
>   if (!area)
>   return NULL;
>  
> - area->pages = pages;
> -
>   if (map_vm_area(area, prot, pages)) {
>   vunmap(area->addr);
>   return NULL;
>   }
>  
> + return area;
> +}
> +
> +void *dma_common_pages_remap(struct page **pages, size_t size,
> + unsigned long vm_flags, pgprot_t prot,
> + const void *caller)
> +{
> + struct vm_struct *area;
> +
> + area = __dma_common_pages_remap(pages, size, vm_flags, prot, caller);
> + if (!area)
> + return NULL;
> +
> + area->pages = pages;
> +
>   return area->addr;
>  }
>  
> @@ -308,7 +321,7 @@ void *dma_common_contiguous_remap(struct page *page, 
> size_t size,
>  {
>   int i;
>   struct page **pages;
> - void *ptr;
> + struct vm_struct *area;
>   unsigned long pfn;
>  
>   pages = kmalloc(sizeof(struct page *) << get_order(size), GFP_KERNEL);
> @@ -318,11 +331,13 @@ void *dma_common_contiguous_remap(struct page *page, 
> size_t size,
>   for (i = 0, pfn = page_to_pfn(page); i < (size >> PAGE_SHIFT); i++)
>   pages[i] = pfn_to_page(pfn + i);
>  
> - ptr = dma_common_pages_remap(pages, size, vm_flags, prot, caller);
> + area = __dma_common_pages_remap(pages, size, vm_flags, prot, caller);
>  
>   kfree(pages);
>  
> - return ptr;
> + if (!area)
> + return NULL;
> + return area->addr;
>  }
>  
>  /*
> 

>From a quick glance, this looks okay. I can give a proper tag when
the fix to allow mmaping comes in since I'm guessing -ENXIO is not the
end goal.

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

Re: [PATCH] iommu/arm-smmu-v3: Increase SMMU CMD queue poll timeout

[Sunil Kovvuri]

On Mon, Apr 24, 2017 at 9:38 PM, Will Deacon  wrote:
> On Mon, Apr 24, 2017 at 05:29:36PM +0530, Geetha sowjanya wrote:
>> From: Geetha 
>>
>> When large memory is being unmapped, huge no of tlb invalidation cmds are
>> submitted followed by a SYNC command. This sometimes hits CMD queue full and
>> poll on queue drain is being timedout throwing error message 'CMD_SYNC 
>> timeout'.
>>
>> Although there is no functional issue, error message confuses user. Hence 
>> increased
>> poll timeout to 500us
>
> Hmm, what are you doing to unmap that much? Is this VFIO teardown? Do you
> have 7c6d90e2bb1a ("iommu/io-pgtable-arm: Fix iova_to_phys for block
> entries") applied?

Yes it's VFIO teardown and again yes the above fix is applied.
But i didn't get how above fix is related.
TLB invalidation commands are submitted at 'arm_smmu_tlb_inv_range_nosync()'
and it's a loop over granule size.

1357 do {
1358 arm_smmu_cmdq_issue_cmd(smmu, );
1359 cmd.tlbi.addr += granule;
1360 } while (size -= granule);

So if invalidation size is big then huge no of invalidation commands
will be submitted
irrespective of fix that you pointed above, right ?

Thanks,
Sunil.

>
> Will
>
>>
>> Signed-off-by: Geetha 
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 591bb96..1dcd154 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -407,7 +407,7 @@
>>  #define PRIQ_1_ADDR_MASK 0xfUL
>>
>>  /* High-level queue structures */
>> -#define ARM_SMMU_POLL_TIMEOUT_US 100
>> +#define ARM_SMMU_POLL_TIMEOUT_US 500
>>
>>  #define MSI_IOVA_BASE0x800
>>  #define MSI_IOVA_LENGTH  0x10
>> --
>> 1.9.1
>>
>
> ___
> 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-v3: Increase SMMU CMD queue poll timeout

[Akula, Geethasowjanya]

>>On Mon, Apr 24, 2017 at 05:29:36PM +0530, Geetha sowjanya wrote:
>> From: Geetha 
>>
>> When large memory is being unmapped, huge no of tlb invalidation cmds are
>> submitted followed by a SYNC command. This sometimes hits CMD queue full and
>> poll on queue drain is being timedout throwing error message 'CMD_SYNC 
>> timeout'.
>>
>> Although there is no functional issue, error message confuses user. Hence 
>> increased
>> poll timeout to 500us

>Hmm, what are you doing to unmap that much? Is this VFIO teardown? Do you


Yes. I see this issue while poweroff the guest with VFIO device attached (40G 
Intel VF).


>have 7c6d90e2bb1a ("iommu/io-pgtable-arm: Fix iova_to_phys for block
>entries") applied?

 Using 4.11-rc5 kernel and patch is in place.



Thanks,

Geetha.


Will

>>
>> Signed-off-by: Geetha 
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 591bb96..1dcd154 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>>@@ -407,7 +407,7 @@
>>  #define PRIQ_1_ADDR_MASK 0xfUL
>>
>> /* High-level queue structures */
>> -#define ARM_SMMU_POLL_TIMEOUT_US 100
>> +#define ARM_SMMU_POLL_TIMEOUT_US 500
>>
>>  #define MSI_IOVA_BASE0x800
>>  #define MSI_IOVA_LENGTH  0x10
>> -



From: Will Deacon 
Sent: Monday, April 24, 2017 9:38:41 PM
To: Akula, Geethasowjanya
Cc: robin.mur...@arm.com; iommu@lists.linux-foundation.org; 
catalin.mari...@arm.com; linux-arm-ker...@lists.infradead.org; 
linux-ker...@vger.kernel.org; Goutham, Sunil; Akula, Geethasowjanya
Subject: Re: [PATCH] iommu/arm-smmu-v3: Increase SMMU CMD queue poll timeout

On Mon, Apr 24, 2017 at 05:29:36PM +0530, Geetha sowjanya wrote:
> From: Geetha 
>
> When large memory is being unmapped, huge no of tlb invalidation cmds are
> submitted followed by a SYNC command. This sometimes hits CMD queue full and
> poll on queue drain is being timedout throwing error message 'CMD_SYNC 
> timeout'.
>
> Although there is no functional issue, error message confuses user. Hence 
> increased
> poll timeout to 500us

Hmm, what are you doing to unmap that much? Is this VFIO teardown? Do you
have 7c6d90e2bb1a ("iommu/io-pgtable-arm: Fix iova_to_phys for block
entries") applied?

Will

>
> Signed-off-by: Geetha 
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 591bb96..1dcd154 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -407,7 +407,7 @@
>  #define PRIQ_1_ADDR_MASK 0xfUL
>
>  /* High-level queue structures */
> -#define ARM_SMMU_POLL_TIMEOUT_US 100
> +#define ARM_SMMU_POLL_TIMEOUT_US 500
>
>  #define MSI_IOVA_BASE0x800
>  #define MSI_IOVA_LENGTH  0x10
> --
> 1.9.1
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/arm-smmu: Return IOVA in iova_to_phys when SMMU is bypassed

[Sunil Kovvuri]

On Mon, Apr 24, 2017 at 9:30 PM, Will Deacon  wrote:
> On Mon, Apr 24, 2017 at 09:23:16PM +0530, Sunil Kovvuri wrote:
>> On Mon, Apr 24, 2017 at 8:14 PM, Will Deacon  wrote:
>> > On Mon, Apr 17, 2017 at 05:27:26PM +0530, sunil.kovv...@gmail.com wrote:
>> >> From: Sunil Goutham 
>> >>
>> >> For software initiated address translation, when domain type is
>> >> IOMMU_DOMAIN_IDENTITY i.e SMMU is bypassed, mimic HW behavior
>> >> i.e return the same IOVA as translated address.
>> >>
>> >> This patch is an extension to Will Deacon's patchset
>> >> "Implement SMMU passthrough using the default domain".
>> >
>> > Are you actually seeing an issue here? If so, why isn't SMMUv3 affected 
>> > too?
>> Yes and SMMUv3 should also be effected but as of now I don't see any use 
>> case.
>> If needed, i can re-submit the patch with changes in SMMUv3 as well.
>
> Yes, please.
>
>> >> Signed-off-by: Sunil Goutham 
>> >> ---
>> >>  drivers/iommu/arm-smmu.c | 3 +++
>> >>  1 file changed, 3 insertions(+)
>> >>
>> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> >> index 41afb07..2f4a130 100644
>> >> --- a/drivers/iommu/arm-smmu.c
>> >> +++ b/drivers/iommu/arm-smmu.c
>> >> @@ -1405,6 +1405,9 @@ static phys_addr_t arm_smmu_iova_to_phys(struct 
>> >> iommu_domain *domain,
>> >>   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>> >>   struct io_pgtable_ops *ops= smmu_domain->pgtbl_ops;
>> >>
>> >> + if (domain->type == IOMMU_DOMAIN_IDENTITY)
>> >> + return iova;
>> >> +
>> >>   if (!ops)
>> >>   return 0;
>> >
>> > I'd have thought ops would be NULL, since arm_smmu_init_domain_context
>> > doesn't allocate them for an identity domain.
>> Yes ops is set to NULL.
>
> Argh, sorry, I completely overlooked that we return 0 in that case, rather
> than the iova.
>
>> > I don't understand this patch. Please can you explain the problem more
>> > clearly?
>> AFAIK for any driver outside IOMMU there is only one way to identify
>> if device is attached to
>> IOMMU or not and that is by checking iommu_domain. And I don't think
>> it would be appropriate
>> for the driver to check domain->type before calling 'iommu_iova_to_phys()' 
>> API.
>>
>> The difference between IOMMU disabled and IOMMU being in passthrough
>> mode is that, in the
>> later case device is still attached to default domain but in former's
>> case it's NULL. So there is no
>> way to differentiate for the external driver whether IOMMU is in
>> passthrough mode or DMA mode.
>>
>> And since ops is NULL in passthrough mode, 'iommu_iova_to_phys()' will
>> return zero.
>>
>> Use case for your reference
>> https://lkml.org/lkml/2017/3/7/299
>> This driver is for a NIC interface on platform which supports SMMUv2.
>
> Blimey, that driver is horrible, but I take your point on the API. Please
> repost, fixing SMMUv3 at the same time.

Sure, will re-submit the patch with SMMUv3 changes.

On a separate note, if you have time, I would definitely like to know
your feedback and what's horrible in that driver, probably in a different
email to keep that out of scope of this patch.

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

Re: [PATCH v5 09/32] x86/mm: Provide general kernel support for memory encryption

[Tom Lendacky]

On 4/24/2017 10:57 AM, Dave Hansen wrote:

On 04/24/2017 08:53 AM, Tom Lendacky wrote:

On 4/21/2017 4:52 PM, Dave Hansen wrote:

On 04/18/2017 02:17 PM, Tom Lendacky wrote:

@@ -55,7 +57,7 @@ static inline void copy_user_page(void *to, void
*from, unsigned long vaddr,
 __phys_addr_symbol(__phys_reloc_hide((unsigned long)(x)))

 #ifndef __va
-#define __va(x)((void *)((unsigned long)(x)+PAGE_OFFSET))
+#define __va(x)((void *)(__sme_clr(x) + PAGE_OFFSET))
 #endif


It seems wrong to be modifying __va().  It currently takes a physical
address, and this modifies it to take a physical address plus the SME
bits.


This actually modifies it to be sure the encryption bit is not part of
the physical address.


If SME bits make it this far, we have a bug elsewhere.  Right?  Probably
best not to paper over it.


That all depends on the approach.  Currently that's not the case for
the one situation that you mentioned with cr3.  But if we do take the
approach that we should never feed physical addresses to __va() with
the encryption bit set then, yes, it would imply a bug elsewhere - which
is probably a good approach.

I'll work on that. I could even add a debug config option that would
issue a warning should __va() encounter the encryption bit if SME is
enabled or active.

Thanks,
Tom




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

Re: [PATCH] iommu/arm-smmu-v3: Increase SMMU CMD queue poll timeout

[Will Deacon]

On Mon, Apr 24, 2017 at 05:29:36PM +0530, Geetha sowjanya wrote:
> From: Geetha 
> 
> When large memory is being unmapped, huge no of tlb invalidation cmds are
> submitted followed by a SYNC command. This sometimes hits CMD queue full and
> poll on queue drain is being timedout throwing error message 'CMD_SYNC 
> timeout'.
> 
> Although there is no functional issue, error message confuses user. Hence 
> increased
> poll timeout to 500us

Hmm, what are you doing to unmap that much? Is this VFIO teardown? Do you
have 7c6d90e2bb1a ("iommu/io-pgtable-arm: Fix iova_to_phys for block
entries") applied?

Will

> 
> Signed-off-by: Geetha 
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 591bb96..1dcd154 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -407,7 +407,7 @@
>  #define PRIQ_1_ADDR_MASK 0xfUL
>  
>  /* High-level queue structures */
> -#define ARM_SMMU_POLL_TIMEOUT_US 100
> +#define ARM_SMMU_POLL_TIMEOUT_US 500
>  
>  #define MSI_IOVA_BASE0x800
>  #define MSI_IOVA_LENGTH  0x10
> -- 
> 1.9.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/arm-smmu: Return IOVA in iova_to_phys when SMMU is bypassed

[Will Deacon]

On Mon, Apr 24, 2017 at 09:23:16PM +0530, Sunil Kovvuri wrote:
> On Mon, Apr 24, 2017 at 8:14 PM, Will Deacon  wrote:
> > On Mon, Apr 17, 2017 at 05:27:26PM +0530, sunil.kovv...@gmail.com wrote:
> >> From: Sunil Goutham 
> >>
> >> For software initiated address translation, when domain type is
> >> IOMMU_DOMAIN_IDENTITY i.e SMMU is bypassed, mimic HW behavior
> >> i.e return the same IOVA as translated address.
> >>
> >> This patch is an extension to Will Deacon's patchset
> >> "Implement SMMU passthrough using the default domain".
> >
> > Are you actually seeing an issue here? If so, why isn't SMMUv3 affected too?
> Yes and SMMUv3 should also be effected but as of now I don't see any use case.
> If needed, i can re-submit the patch with changes in SMMUv3 as well.

Yes, please.

> >> Signed-off-by: Sunil Goutham 
> >> ---
> >>  drivers/iommu/arm-smmu.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >> index 41afb07..2f4a130 100644
> >> --- a/drivers/iommu/arm-smmu.c
> >> +++ b/drivers/iommu/arm-smmu.c
> >> @@ -1405,6 +1405,9 @@ static phys_addr_t arm_smmu_iova_to_phys(struct 
> >> iommu_domain *domain,
> >>   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> >>   struct io_pgtable_ops *ops= smmu_domain->pgtbl_ops;
> >>
> >> + if (domain->type == IOMMU_DOMAIN_IDENTITY)
> >> + return iova;
> >> +
> >>   if (!ops)
> >>   return 0;
> >
> > I'd have thought ops would be NULL, since arm_smmu_init_domain_context
> > doesn't allocate them for an identity domain.
> Yes ops is set to NULL.

Argh, sorry, I completely overlooked that we return 0 in that case, rather
than the iova.

> > I don't understand this patch. Please can you explain the problem more
> > clearly?
> AFAIK for any driver outside IOMMU there is only one way to identify
> if device is attached to
> IOMMU or not and that is by checking iommu_domain. And I don't think
> it would be appropriate
> for the driver to check domain->type before calling 'iommu_iova_to_phys()' 
> API.
> 
> The difference between IOMMU disabled and IOMMU being in passthrough
> mode is that, in the
> later case device is still attached to default domain but in former's
> case it's NULL. So there is no
> way to differentiate for the external driver whether IOMMU is in
> passthrough mode or DMA mode.
> 
> And since ops is NULL in passthrough mode, 'iommu_iova_to_phys()' will
> return zero.
> 
> Use case for your reference
> https://lkml.org/lkml/2017/3/7/299
> This driver is for a NIC interface on platform which supports SMMUv2.

Blimey, that driver is horrible, but I take your point on the API. Please
repost, fixing SMMUv3 at the same time.

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

Re: [PATCH v5 09/32] x86/mm: Provide general kernel support for memory encryption

[Dave Hansen]

On 04/24/2017 08:53 AM, Tom Lendacky wrote:
> On 4/21/2017 4:52 PM, Dave Hansen wrote:
>> On 04/18/2017 02:17 PM, Tom Lendacky wrote:
>>> @@ -55,7 +57,7 @@ static inline void copy_user_page(void *to, void
>>> *from, unsigned long vaddr,
>>>  __phys_addr_symbol(__phys_reloc_hide((unsigned long)(x)))
>>>
>>>  #ifndef __va
>>> -#define __va(x)((void *)((unsigned long)(x)+PAGE_OFFSET))
>>> +#define __va(x)((void *)(__sme_clr(x) + PAGE_OFFSET))
>>>  #endif
>>
>> It seems wrong to be modifying __va().  It currently takes a physical
>> address, and this modifies it to take a physical address plus the SME
>> bits.
> 
> This actually modifies it to be sure the encryption bit is not part of
> the physical address.

If SME bits make it this far, we have a bug elsewhere.  Right?  Probably
best not to paper over it.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu: arm-smmu: correct sid to mask

[Will Deacon]

On Fri, Apr 21, 2017 at 05:03:36PM +0800, Peng Fan wrote:
> From code "SMR mask 0x%x out of range for SMMU",
> so, we need to use mask, not sid.
> 
> Signed-off-by: Peng Fan 
> Cc: Will Deacon 
> Cc: Robin Murphy 
> ---
>  drivers/iommu/arm-smmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index b493c99..5a06de2 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1467,7 +1467,7 @@ static int arm_smmu_add_device(struct device *dev)
>   }
>   if (mask & ~smmu->smr_mask_mask) {
>   dev_err(dev, "SMR mask 0x%x out of range for SMMU 
> (0x%x)\n",
> - sid, smmu->smr_mask_mask);
> + mask, smmu->smr_mask_mask);
>   goto out_free;

Looks like a copy-paste error to me:

Acked-by: Will Deacon 

Joerg: do you mind picking this one up, please?

Thanks,

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

Re: [PATCH v5 09/32] x86/mm: Provide general kernel support for memory encryption

[Tom Lendacky]

On 4/21/2017 4:52 PM, Dave Hansen wrote:

On 04/18/2017 02:17 PM, Tom Lendacky wrote:

@@ -55,7 +57,7 @@ static inline void copy_user_page(void *to, void *from, 
unsigned long vaddr,
__phys_addr_symbol(__phys_reloc_hide((unsigned long)(x)))

 #ifndef __va
-#define __va(x)((void *)((unsigned 
long)(x)+PAGE_OFFSET))
+#define __va(x)((void *)(__sme_clr(x) + PAGE_OFFSET))
 #endif


It seems wrong to be modifying __va().  It currently takes a physical
address, and this modifies it to take a physical address plus the SME bits.


This actually modifies it to be sure the encryption bit is not part of
the physical address.



How does that end up ever happening?  If we are pulling physical
addresses out of the page tables, we use p??_phys().  I'd expect *those*
to be masking off the SME bits.

Is it these cases?

pgd_t *base = __va(read_cr3());

For those, it seems like we really want to create two modes of reading
cr3.  One that truly reads CR3 and another that reads the pgd's physical
address out of CR3.  Then you only do the SME masking on the one
fetching a physical address, and the SME bits never leak into __va().


I'll investigate this and see if I can remove the mod to __va().

Thanks,
Tom




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

Re: [PATCH] iommu/arm-smmu: Return IOVA in iova_to_phys when SMMU is bypassed

[Sunil Kovvuri]

On Mon, Apr 24, 2017 at 8:14 PM, Will Deacon  wrote:
> On Mon, Apr 17, 2017 at 05:27:26PM +0530, sunil.kovv...@gmail.com wrote:
>> From: Sunil Goutham 
>>
>> For software initiated address translation, when domain type is
>> IOMMU_DOMAIN_IDENTITY i.e SMMU is bypassed, mimic HW behavior
>> i.e return the same IOVA as translated address.
>>
>> This patch is an extension to Will Deacon's patchset
>> "Implement SMMU passthrough using the default domain".
>
> Are you actually seeing an issue here? If so, why isn't SMMUv3 affected too?
Yes and SMMUv3 should also be effected but as of now I don't see any use case.
If needed, i can re-submit the patch with changes in SMMUv3 as well.

>
>> Signed-off-by: Sunil Goutham 
>> ---
>>  drivers/iommu/arm-smmu.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 41afb07..2f4a130 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -1405,6 +1405,9 @@ static phys_addr_t arm_smmu_iova_to_phys(struct 
>> iommu_domain *domain,
>>   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>>   struct io_pgtable_ops *ops= smmu_domain->pgtbl_ops;
>>
>> + if (domain->type == IOMMU_DOMAIN_IDENTITY)
>> + return iova;
>> +
>>   if (!ops)
>>   return 0;
>
> I'd have thought ops would be NULL, since arm_smmu_init_domain_context
> doesn't allocate them for an identity domain.
Yes ops is set to NULL.

>
> I don't understand this patch. Please can you explain the problem more
> clearly?
AFAIK for any driver outside IOMMU there is only one way to identify
if device is attached to
IOMMU or not and that is by checking iommu_domain. And I don't think
it would be appropriate
for the driver to check domain->type before calling 'iommu_iova_to_phys()' API.

The difference between IOMMU disabled and IOMMU being in passthrough
mode is that, in the
later case device is still attached to default domain but in former's
case it's NULL. So there is no
way to differentiate for the external driver whether IOMMU is in
passthrough mode or DMA mode.

And since ops is NULL in passthrough mode, 'iommu_iova_to_phys()' will
return zero.

Use case for your reference
https://lkml.org/lkml/2017/3/7/299
This driver is for a NIC interface on platform which supports SMMUv2.

Let me know if any more details are needed.

Thanks,
Sunil.

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

Re: [RFC 3/3] virtio-iommu: future work

[Jean-Philippe Brucker]

On 21/04/17 09:31, Tian, Kevin wrote:
>> From: Jean-Philippe Brucker
>> Sent: Saturday, April 8, 2017 3:18 AM
>>
>> Here I propose a few ideas for extensions and optimizations. This is all
>> very exploratory, feel free to correct mistakes and suggest more things.
> 
> [...]
>>
>>   II. Page table sharing
>>   ==
>>
>>   1. Sharing IOMMU page tables
>>   
>>
>> VIRTIO_IOMMU_F_PT_SHARING
>>
>> This is independent of the nested mode described in I.2, but relies on a
>> similar feature in the physical IOMMU: having two stages of page tables,
>> one for the host and one for the guest.
>>
>> When this is supported, the guest can manage its own s1 page directory, to
>> avoid sending MAP/UNMAP requests. Feature
>> VIRTIO_IOMMU_F_PT_SHARING allows
>> a driver to give a page directory pointer (pgd) to the host and send
>> invalidations when removing or changing a mapping. In this mode, three
>> requests are used: probe, attach and invalidate. An address space cannot
>> be using the MAP/UNMAP interface and PT_SHARING at the same time.
>>
>> Device and driver first need to negotiate which page table format they
>> will be using. This depends on the physical IOMMU, so the request contains
>> a negotiation part to probe the device capabilities.
>>
>> (1) Driver attaches devices to address spaces as usual, but a flag
>> VIRTIO_IOMMU_ATTACH_F_PRIVATE (working title) tells the device not to
>> create page tables for use with the MAP/UNMAP API. The driver intends
>> to manage the address space itself.
>>
>> (2) Driver sends a PROBE_TABLE request. It sets len > 0 with the size of
>> pg_format array.
>>
>>  VIRTIO_IOMMU_T_PROBE_TABLE
>>
>>  struct virtio_iommu_req_probe_table {
>>  le32address_space;
>>  le32flags;
>>  le32len;
>>
>>  le32nr_contexts;
>>  struct {
>>  le32model;
>>  u8  format[64];
>>  } pg_format[len];
>>  };
>>
>> Introducing a probe request is more flexible than advertising those
>> features in virtio config, because capabilities are dynamic, and depend on
>> which devices are attached to an address space. Within a single address
>> space, devices may support different numbers of contexts (PASIDs), and
>> some may not support recoverable faults.
>>
>> (3) Device responds success with all page table formats implemented by the
>> physical IOMMU in pg_format. 'model' 0 is invalid, so driver can
>> initialize the array to 0 and deduce from there which entries have
>> been filled by the device.
>>
>> Using a probe method seems preferable over trying to attach every possible
>> format until one sticks. For instance, with an ARM guest running on an x86
>> host, PROBE_TABLE would return the Intel IOMMU page table format, and
>> the
>> guest could use that page table code to handle its mappings, hidden behind
>> the IOMMU API. This requires that the page-table code is reasonably
>> abstracted from the architecture, as is done with drivers/iommu/io-pgtable
>> (an x86 guest could use any format implement by io-pgtable for example.)
> 
> So essentially you need modify all existing IOMMU drivers to support page 
> table sharing in pvIOMMU. After abstraction is done the core pvIOMMU files 
> can be kept vendor agnostic. But if we talk about the whole pvIOMMU 
> module, it actually includes vendor specific logic thus unlike typical 
> para-virtualized virtio drivers being completely vendor agnostic. Is this 
> understanding accurate?

Yes, although kernel modules would be separate. For Linux on ARM we
already have the page-table logic abstracted in iommu/io-pgtable module,
because multiple IOMMUs share the same PT formats (SMMUv2, SMMUv3, Renesas
IPMMU, Qcom MSM, Mediatek). It offers a simple interface:

* When attaching devices to an IOMMU domain, the IOMMU driver registers
its page table format and provides invalidation callbacks.

* On iommu_map/unmap, the IOMMU driver calls into io_pgtable_ops, which
provide map, unmap and iova_to_phys functions.

* Page table operations call back into the driver via iommu_gather_ops
when they need to invalidate TLB entries.

Currently only the few flavors of ARM PT formats are implemented, but
other page table formats could be added if they fit this model.

> It also means in the host-side pIOMMU driver needs to propagate all
> supported formats through VFIO to Qemu vIOMMU, meaning
> such format definitions need be consistently agreed across all those 
> components.

Yes, that's the icky part. We need to define a format that every OS and
hypervisor implementing virtio-iommu can understand (similarly to the
PASID table sharing interface that Yi L is working on for VFIO, although
that one is contained in Linux UAPI and doesn't require other OSes to know
about it).

>>   2. Sharing MMU page tables
>>   --
>>
>> The guest can share process 

Re: [RFC 2/3] virtio-iommu: device probing and operations

[Jean-Philippe Brucker]

On 21/04/17 10:02, Tian, Kevin wrote:
>> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
>> Sent: Wednesday, April 19, 2017 2:46 AM
>>
>> On 18/04/17 11:26, Tian, Kevin wrote:
 From: Jean-Philippe Brucker
 Sent: Saturday, April 8, 2017 3:18 AM

>>> [...]
   II. Feature bits
   

 VIRTIO_IOMMU_F_INPUT_RANGE (0)
  Available range of virtual addresses is described in input_range
>>>
>>> Usually only the maximum supported address bits are important.
>>> Curious do you see such situation where low end of the address
>>> space is not usable (since you have both start/end defined later)?
>>
>> A start address would allow to provide something resembling a GART to the
>> guest: an IOMMU with one address space (ioasid_bits=0) and a small IOVA
>> aperture. I'm not sure how useful that would be in practice.
> 
> Intel VT-d has no such limitation, which I can tell. :-)
> 
>>
>> On a related note, the virtio-iommu itself doesn't provide a
>> per-address-space aperture as it stands. For example, attaching a device
>> to an address space might restrict the available IOVA range for the whole
>> AS if that device cannot write to high memory (above 32-bit). If the guest
>> attempts to map an IOVA outside this window into the device's address
>> space, it should expect the MAP request to fail. And when attaching, if
>> the address space already has mappings outside this window, then ATTACH
>> should fail.
>>
>> This too seems to be something that ought to be communicated by firmware,
>> but bits are missing (I can't find anything equivalent to DT's dma-ranges
>> for PCI root bridges in ACPI tables, for example). In addition VFIO
>> doesn't communicate any DMA mask for devices, and doesn't check them
>> itself. I guess that the host could find out the DMA mask of devices one
>> way or another, but it is tricky to enforce, so I didn't make this a hard
>> requirement. Although I should probably add a few words about it.
> 
> If there is no such communication on bare metal, then same for pvIOMMU.
> 
>>
>>> [...]
   1. Attach device
   

 struct virtio_iommu_req_attach {
le32address_space;
le32device;
le32flags/reserved;
 };

 Attach a device to an address space. 'address_space' is an identifier
 unique to the guest. If the address space doesn't exist in the IOMMU
>>>
>>> Based on your description this address space ID is per operation right?
>>> MAP/UNMAP and page-table sharing should have different ID spaces...
>>
>> I think it's simpler if we keep a single IOASID space per virtio-iommu
>> device, because the maximum number of address spaces (described by
>> ioasid_bits) might be a restriction of the pIOMMU. For page-table sharing
>> you still need to define which devices will share a page directory using
>> ATTACH requests, though that interface is not set in stone.
> 
> got you. yes VM is supposed to consume less IOASIDs than physically
> available. It doesn’t hurt to have one IOASID space for both IOVA
> map/unmap usages (one IOASID per device) and SVM usages (multiple
> IOASIDs per device). The former is digested by software and the latter
> will be bound to hardware.
> 

Hmm, I'm using address space indexed by IOASID for "classic" IOMMU, and
then contexts indexed by PASID when talking about SVM. So in my mind an
address space can have multiple sub-address-spaces (contexts). Number of
IOASIDs is a limitation of the pIOMMU, and number of PASIDs is a
limitation of the device. Therefore attaching devices to address spaces
would update the number of available contexts in that address space. The
terminology is not ideal, and I'd be happy to change it for something more
clear.

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

Re: [RFC 1/3] virtio-iommu: firmware description of the virtual topology

[Jean-Philippe Brucker]

On 21/04/17 09:43, Tian, Kevin wrote:
>> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
>> Sent: Wednesday, April 19, 2017 2:41 AM
>>
>> On 18/04/17 10:51, Tian, Kevin wrote:
 From: Jean-Philippe Brucker
 Sent: Saturday, April 8, 2017 3:18 AM

 Unlike other virtio devices, the virtio-iommu doesn't work independently,
 it is linked to other virtual or assigned devices. So before jumping into
 device operations, we need to define a way for the guest to discover the
 virtual IOMMU and the devices it translates.

 The host must describe the relation between IOMMU and devices to the
 guest
 using either device-tree or ACPI. The virtual IOMMU identifies each
>>>
>>> Do you plan to support both device tree and ACPI?
>>
>> Yes, with ACPI the topology would be described using IORT nodes. I didn't
>> include an example in my driver because DT is sufficient for a prototype
>> and is readily available (both in Linux and kvmtool), whereas IORT would
>> be quite easy to reuse in Linux, but isn't present in kvmtool at the
>> moment. However, both interfaces have to be supported for the virtio-
>> iommu
>> to be portable.
> 
> 'portable' means whether guest enables ACPI?

Sorry, "supported" isn't the right term for what I meant. It is for
firmware interface to accommodate devices, not the other way around, so
firmware consideration is outside the scope of the virtio-iommu
specification and virtio-iommu itself doesn't need to "support" any interface.

For the purpose of this particular document however, both popular firmware
interfaces (ACPI and DT) must be taken into account. Those are the two
interfaces I know about, there might be others. But I figure that a VMM
implementing a virtual IOMMU is complex enough to be able to also
implement one of these two interfaces, so talking about DT and ACPI should
fit all use cases. It also provides two examples for other firmware
interfaces that wish to describe the IOMMU topology.

 virtual device with a 32-bit ID, that we will call "Device ID" in this
 document. Device IDs are not necessarily unique system-wide, but they
>> may
 not overlap within a single virtual IOMMU. Device ID of passed-through
 devices do not need to match IDs seen by the physical IOMMU.

 The virtual IOMMU uses virtio-mmio transport exclusively, not virtio-pci,
 because with PCI the IOMMU interface would itself be an endpoint, and
 existing firmware interfaces don't allow to describe IOMMU<->master
 relations between PCI endpoints.
>>>
>>> I'm not familiar with virtio-mmio mechanism. Curious how devices in
>>> virtio-mmio are enumerated today? Could we use that mechanism to
>>> identify vIOMMUs and then invent a purely para-virtualized method to
>>> enumerate devices behind each vIOMMU?
>>
>> Using DT, virtio-mmio devices are described with "virtio-mmio" compatible
>> node, and with ACPI they use _HID LNRO0005. Since the host already
>> describes available devices to a guest using a firmware interface, I think
>> we should reuse the tools provided by that interface for describing
>> relations between DMA masters and IOMMU.
> 
> OK, I didn't realize virtio-mmio is defined to rely on DT for enumeration.

Not necessarily DT, you can have virtio-mmio devices in ACPI namespace as
well. Qemu has a an example of LNRO0005 with ACPI.

>>> Asking this is because each vendor has its own enumeration methods.
>>> ARM has device tree and ACPI IORT. AMR has ACPI IVRS and device
>>> tree (same format as ARM?). Intel has APCI DMAR and sub-tables. Your
>>> current proposal looks following ARM definitions which I'm not sure
>>> extensible enough to cover features defined only in other vendors'
>>> structures.
>>
>> ACPI IORT can be extended to incorporate para-virtualized IOMMUs,
>> regardless of the underlying architecture. It isn't defined solely for the
>> ARM SMMU, but serves a more general purpose of describing a map of
>> device
>> identifiers communicated from one components to another. Both DMAR and
>> IVRS have such description (respectively DRHD and IVHD), but they are
>> designed for a specific IOMMU, whereas IORT could host other kinds.
> 
> I'll take a look at IORT definition. DRHD includes information more
> than device mapping.

I guess that most information provided by DMAR and others are
IOMMU-specific and the equivalent for virtio-iommu would fit in virtio
config space. But describing device mapping relative to IOMMUs is the same
problem for all systems. Doing it with a virtio-iommu probing mechanism
would require to reinvent a way to identify devices every time a host
wants to add support for a new bus (RID for PCI, base address for MMIO,
others in the future), when firmwares would have to provide this
information anyway for bare metal.

>> It seems that all we really need is an interface that says "there is a
>> virtio-iommu at address X, here are the devices it translates and their
>> corresponding IDs", 

Re: [PATCH] iommu/arm-smmu: Return IOVA in iova_to_phys when SMMU is bypassed

[Will Deacon]

On Mon, Apr 17, 2017 at 05:27:26PM +0530, sunil.kovv...@gmail.com wrote:
> From: Sunil Goutham 
> 
> For software initiated address translation, when domain type is
> IOMMU_DOMAIN_IDENTITY i.e SMMU is bypassed, mimic HW behavior
> i.e return the same IOVA as translated address.
> 
> This patch is an extension to Will Deacon's patchset 
> "Implement SMMU passthrough using the default domain".

Are you actually seeing an issue here? If so, why isn't SMMUv3 affected too?

> Signed-off-by: Sunil Goutham 
> ---
>  drivers/iommu/arm-smmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 41afb07..2f4a130 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1405,6 +1405,9 @@ static phys_addr_t arm_smmu_iova_to_phys(struct 
> iommu_domain *domain,
>   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>   struct io_pgtable_ops *ops= smmu_domain->pgtbl_ops;
>  
> + if (domain->type == IOMMU_DOMAIN_IDENTITY)
> + return iova;
> +
>   if (!ops)
>   return 0;

I'd have thought ops would be NULL, since arm_smmu_init_domain_context
doesn't allocate them for an identity domain.

I don't understand this patch. Please can you explain the problem more
clearly?

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

Re: [RFC PATH] of/pci/dma: fix DMA configruation for PCI masters

[Rob Herring]

On Sat, Apr 22, 2017 at 3:08 AM, Oza Pawandeep  wrote:
> current device frmework and of framework integration assumes dma-ranges
> in a way where memory-mapped devices define their dma-ranges.
> dma-ranges: (child-bus-address, parent-bus-address, length).
>
> but iproc based SOCs and other SOCs(suc as rcar) have PCI world dma-ranges.
> dma-ranges = <0x4300 0x00 0x00 0x00 0x00 0x80 0x00>;
>
> of_dma_configure is specifically witten to take care of memory mapped devices.
> but no implementation exists for pci to take care of pcie based memory ranges.
> in fact pci world doesnt seem to define standard dma-ranges
>
> this patch served following purposes
>
> 1) exposes intrface to the pci host driver for thir inbound memory ranges
>
> 2) provide an interface to callers such as of_dma_get_ranges.
> so then the returned size get best possible (largest) dma_mask.
> for e.g.
> dma-ranges = <0x4300 0x00 0x00 0x00 0x00 0x80 0x00>;
> we should get dev->coherent_dma_mask=0x7f.
>
> 3) this patch handles multiple inbound windows and dma-ranges.
> it is left to the caller, how it wants to use them.
> the new function returns the resources in a standard and unform way
>
> 4) this way the callers of of_dma_get_ranges does not need to change.
> and
>
> 5) leaves scope of adding PCI flag handling for inbound memory
> by the new function.
>
> Signed-off-by: Oza Pawandeep 
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 02b2903..ec21191 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -829,10 +830,30 @@ int of_dma_get_range(struct device_node *np, u64 
> *dma_addr, u64 *paddr, u64 *siz
> int len, naddr, nsize, pna;
> int ret = 0;
> u64 dmaaddr;
> +   struct resource_entry *window;
> +   LIST_HEAD(res);
>
> if (!node)
> return -EINVAL;
>
> +   if (strcmp(np->name, "pci")) {

Using the name is not reliable though I did recently add a dtc check
for this. Of course, 'pcie' is valid too (and probably should be used
for what you are testing). type is what you want to use here. We
already have bus matching function and bus specific handlers in
address.c. Whatever solution you come up with should be integrated
with the existing bus specific handlers.

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

[PATCH] iommu/arm-smmu-v3: Increase SMMU CMD queue poll timeout

[Geetha sowjanya]

From: Geetha 

When large memory is being unmapped, huge no of tlb invalidation cmds are
submitted followed by a SYNC command. This sometimes hits CMD queue full and
poll on queue drain is being timedout throwing error message 'CMD_SYNC timeout'.

Although there is no functional issue, error message confuses user. Hence 
increased
poll timeout to 500us

Signed-off-by: Geetha 
---
 drivers/iommu/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 591bb96..1dcd154 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -407,7 +407,7 @@
 #define PRIQ_1_ADDR_MASK   0xfUL
 
 /* High-level queue structures */
-#define ARM_SMMU_POLL_TIMEOUT_US   100
+#define ARM_SMMU_POLL_TIMEOUT_US   500
 
 #define MSI_IOVA_BASE  0x800
 #define MSI_IOVA_LENGTH0x10
-- 
1.9.1

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

Re: [PATCH 1/1] iommu/amd: fix incorrect error handling

[Joerg Roedel]

On Sun, Apr 23, 2017 at 06:23:21PM +0800, Pan Bian wrote:
> From: Pan Bian 
> 
> In function amd_iommu_bind_pasid(), the control flow jumps to label
> out_free when pasid_state->mm and mm is NULL. And mmput(mm) is called.
> In function mmput(mm), mm is referenced without validation. This will
> result in a NULL dereference bug. This patch fixes the bug.
> 
> Signed-off-by: Pan Bian 

Applied, thanks.

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