Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1540522 > unrolled thread

[PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED Sricharan R <sricharan@codeaurora.org> - 2016-12-12 19:40 +0100
    Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED Robin Murphy <robin.murphy@arm.com> - 2016-12-13 15:10 +0100
      RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED "Sricharan" <sricharan@codeaurora.org> - 2016-12-13 15:40 +0100
        Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED Robin Murphy <robin.murphy@arm.com> - 2016-12-13 15:50 +0100
          RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED "Sricharan" <sricharan@codeaurora.org> - 2016-12-13 16:00 +0100
          RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED "Sricharan" <sricharan@codeaurora.org> - 2016-12-13 19:50 +0100
            Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED Robin Murphy <robin.murphy@arm.com> - 2016-12-13 20:20 +0100
              RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED "Sricharan" <sricharan@codeaurora.org> - 2017-01-02 14:30 +0100

#1540522 — [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

From: Mitchel Humpherys <mitchelh@codeaurora.org>

The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
are only accessible to privileged DMA engines.  Implement it in
dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
 arch/arm64/mm/dma-mapping.c |  6 +++---
 drivers/iommu/dma-iommu.c   | 10 ++++++++--
 include/linux/dma-iommu.h   |  3 ++-
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 401f79a..ae76ead 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
 				 unsigned long attrs)
 {
 	bool coherent = is_device_dma_coherent(dev);
-	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
+	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
 	size_t iosize = size;
 	void *addr;
 
@@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
 				   unsigned long attrs)
 {
 	bool coherent = is_device_dma_coherent(dev);
-	int prot = dma_direction_to_prot(dir, coherent);
+	int prot = dma_info_to_prot(dir, coherent, attrs);
 	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
 
 	if (!iommu_dma_mapping_error(dev, dev_addr) &&
@@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
 		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
 
 	return iommu_dma_map_sg(dev, sgl, nelems,
-			dma_direction_to_prot(dir, coherent));
+				dma_info_to_prot(dir, coherent, attrs));
 }
 
 static void __iommu_unmap_sg_attrs(struct device *dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d2a7a46..756d5e0 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 EXPORT_SYMBOL(iommu_dma_init_domain);
 
 /**
- * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
+ * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
+ *                    page flags.
  * @dir: Direction of DMA transfer
  * @coherent: Is the DMA master cache-coherent?
+ * @attrs: DMA attributes for the mapping
  *
  * Return: corresponding IOMMU API page protection flags
  */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+		     unsigned long attrs)
 {
 	int prot = coherent ? IOMMU_CACHE : 0;
 
+	if (attrs & DMA_ATTR_PRIVILEGED)
+		prot |= IOMMU_PRIV;
+
 	switch (dir) {
 	case DMA_BIDIRECTIONAL:
 		return prot | IOMMU_READ | IOMMU_WRITE;
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 32c5890..a203181 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -34,7 +34,8 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 		u64 size, struct device *dev);
 
 /* General helpers for DMA-API <-> IOMMU-API interaction */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+		     unsigned long attrs);
 
 /*
  * These implement the bulk of the relevant DMA mapping callbacks, but require
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[toc] | [next] | [standalone]

#1541094

On 12/12/16 18:38, Sricharan R wrote:
> From: Mitchel Humpherys <mitchelh@codeaurora.org>
> 
> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
> are only accessible to privileged DMA engines.  Implement it in
> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
> 
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Tested-by: Robin Murphy <robin.murphy@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
> ---
>  arch/arm64/mm/dma-mapping.c |  6 +++---
>  drivers/iommu/dma-iommu.c   | 10 ++++++++--
>  include/linux/dma-iommu.h   |  3 ++-
>  3 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 401f79a..ae76ead 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>  				 unsigned long attrs)
>  {
>  	bool coherent = is_device_dma_coherent(dev);
> -	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
> +	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>  	size_t iosize = size;
>  	void *addr;
>  
> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>  				   unsigned long attrs)
>  {
>  	bool coherent = is_device_dma_coherent(dev);
> -	int prot = dma_direction_to_prot(dir, coherent);
> +	int prot = dma_info_to_prot(dir, coherent, attrs);
>  	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>  
>  	if (!iommu_dma_mapping_error(dev, dev_addr) &&
> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>  		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>  
>  	return iommu_dma_map_sg(dev, sgl, nelems,
> -			dma_direction_to_prot(dir, coherent));
> +				dma_info_to_prot(dir, coherent, attrs));
>  }
>  
>  static void __iommu_unmap_sg_attrs(struct device *dev,
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index d2a7a46..756d5e0 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>  EXPORT_SYMBOL(iommu_dma_init_domain);
>  
>  /**
> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
> + *                    page flags.
>   * @dir: Direction of DMA transfer
>   * @coherent: Is the DMA master cache-coherent?
> + * @attrs: DMA attributes for the mapping
>   *
>   * Return: corresponding IOMMU API page protection flags
>   */
> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
> +		     unsigned long attrs)
>  {
>  	int prot = coherent ? IOMMU_CACHE : 0;
>  
> +	if (attrs & DMA_ATTR_PRIVILEGED)
> +		prot |= IOMMU_PRIV;
> +
>  	switch (dir) {
>  	case DMA_BIDIRECTIONAL:
>  		return prot | IOMMU_READ | IOMMU_WRITE;

...and applying against -next now also needs this hunk:

@@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
*dev, phys_addr_t phys,
 		size_t size, enum dma_data_direction dir, unsigned long attrs)
 {
 	return __iommu_dma_map(dev, phys, size,
-			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
+			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
 }

 void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,

With those two issues fixed up, I've given the series (applied to
next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.

Robin.

> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
> index 32c5890..a203181 100644
> --- a/include/linux/dma-iommu.h
> +++ b/include/linux/dma-iommu.h
> @@ -34,7 +34,8 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>  		u64 size, struct device *dev);
>  
>  /* General helpers for DMA-API <-> IOMMU-API interaction */
> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
> +		     unsigned long attrs);
>  
>  /*
>   * These implement the bulk of the relevant DMA mapping callbacks, but require
> 

[toc] | [prev] | [next] | [standalone]

#1541132

Hi Robin,

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Robin Murphy
>Sent: Tuesday, December 13, 2016 7:33 PM
>To: Sricharan R <sricharan@codeaurora.org>; jcrouse@codeaurora.org; pdaly@codeaurora.org; jgebben@codeaurora.org;
>joro@8bytes.org; linux-kernel@vger.kernel.org; pratikp@codeaurora.org; iommu@lists.linux-foundation.org; tzeng@codeaurora.org;
>linux-arm-kernel@lists.infradead.org; will.deacon@arm.com; mitchelh@codeaurora.org; vinod.koul@intel.com
>Cc: dan.j.williams@intel.com
>Subject: Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>
>On 12/12/16 18:38, Sricharan R wrote:
>> From: Mitchel Humpherys <mitchelh@codeaurora.org>
>>
>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>> are only accessible to privileged DMA engines.  Implement it in
>> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
>>
>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>> Tested-by: Robin Murphy <robin.murphy@arm.com>
>> Acked-by: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>> ---
>>  arch/arm64/mm/dma-mapping.c |  6 +++---
>>  drivers/iommu/dma-iommu.c   | 10 ++++++++--
>>  include/linux/dma-iommu.h   |  3 ++-
>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>> index 401f79a..ae76ead 100644
>> --- a/arch/arm64/mm/dma-mapping.c
>> +++ b/arch/arm64/mm/dma-mapping.c
>> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>>  				 unsigned long attrs)
>>  {
>>  	bool coherent = is_device_dma_coherent(dev);
>> -	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>> +	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>>  	size_t iosize = size;
>>  	void *addr;
>>
>> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>>  				   unsigned long attrs)
>>  {
>>  	bool coherent = is_device_dma_coherent(dev);
>> -	int prot = dma_direction_to_prot(dir, coherent);
>> +	int prot = dma_info_to_prot(dir, coherent, attrs);
>>  	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>>
>>  	if (!iommu_dma_mapping_error(dev, dev_addr) &&
>> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>>  		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>>
>>  	return iommu_dma_map_sg(dev, sgl, nelems,
>> -			dma_direction_to_prot(dir, coherent));
>> +				dma_info_to_prot(dir, coherent, attrs));
>>  }
>>
>>  static void __iommu_unmap_sg_attrs(struct device *dev,
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index d2a7a46..756d5e0 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>  EXPORT_SYMBOL(iommu_dma_init_domain);
>>
>>  /**
>> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
>> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
>> + *                    page flags.
>>   * @dir: Direction of DMA transfer
>>   * @coherent: Is the DMA master cache-coherent?
>> + * @attrs: DMA attributes for the mapping
>>   *
>>   * Return: corresponding IOMMU API page protection flags
>>   */
>> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
>> +		     unsigned long attrs)
>>  {
>>  	int prot = coherent ? IOMMU_CACHE : 0;
>>
>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>> +		prot |= IOMMU_PRIV;
>> +
>>  	switch (dir) {
>>  	case DMA_BIDIRECTIONAL:
>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>
>...and applying against -next now also needs this hunk:
>
>@@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>*dev, phys_addr_t phys,
> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
> {
> 	return __iommu_dma_map(dev, phys, size,
>-			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>+			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
> }
>
> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>
>With those two issues fixed up, I've given the series (applied to
>next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>

oops, sorry that i missed this in rebase. I can repost now with this fixed,
'checks out' you mean something is not working correct ?

Regards,
 Sricharan  

[toc] | [prev] | [next] | [standalone]

#1541140

On 13/12/16 14:38, Sricharan wrote:
> Hi Robin,
> 
>> -----Original Message-----
>> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Robin Murphy
>> Sent: Tuesday, December 13, 2016 7:33 PM
>> To: Sricharan R <sricharan@codeaurora.org>; jcrouse@codeaurora.org; pdaly@codeaurora.org; jgebben@codeaurora.org;
>> joro@8bytes.org; linux-kernel@vger.kernel.org; pratikp@codeaurora.org; iommu@lists.linux-foundation.org; tzeng@codeaurora.org;
>> linux-arm-kernel@lists.infradead.org; will.deacon@arm.com; mitchelh@codeaurora.org; vinod.koul@intel.com
>> Cc: dan.j.williams@intel.com
>> Subject: Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>
>> On 12/12/16 18:38, Sricharan R wrote:
>>> From: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>
>>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>>> are only accessible to privileged DMA engines.  Implement it in
>>> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
>>>
>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>> Tested-by: Robin Murphy <robin.murphy@arm.com>
>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>>> ---
>>>  arch/arm64/mm/dma-mapping.c |  6 +++---
>>>  drivers/iommu/dma-iommu.c   | 10 ++++++++--
>>>  include/linux/dma-iommu.h   |  3 ++-
>>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>> index 401f79a..ae76ead 100644
>>> --- a/arch/arm64/mm/dma-mapping.c
>>> +++ b/arch/arm64/mm/dma-mapping.c
>>> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>>>  				 unsigned long attrs)
>>>  {
>>>  	bool coherent = is_device_dma_coherent(dev);
>>> -	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>>> +	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>>>  	size_t iosize = size;
>>>  	void *addr;
>>>
>>> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>>>  				   unsigned long attrs)
>>>  {
>>>  	bool coherent = is_device_dma_coherent(dev);
>>> -	int prot = dma_direction_to_prot(dir, coherent);
>>> +	int prot = dma_info_to_prot(dir, coherent, attrs);
>>>  	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>>>
>>>  	if (!iommu_dma_mapping_error(dev, dev_addr) &&
>>> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>>>  		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>>>
>>>  	return iommu_dma_map_sg(dev, sgl, nelems,
>>> -			dma_direction_to_prot(dir, coherent));
>>> +				dma_info_to_prot(dir, coherent, attrs));
>>>  }
>>>
>>>  static void __iommu_unmap_sg_attrs(struct device *dev,
>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index d2a7a46..756d5e0 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>>  EXPORT_SYMBOL(iommu_dma_init_domain);
>>>
>>>  /**
>>> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
>>> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
>>> + *                    page flags.
>>>   * @dir: Direction of DMA transfer
>>>   * @coherent: Is the DMA master cache-coherent?
>>> + * @attrs: DMA attributes for the mapping
>>>   *
>>>   * Return: corresponding IOMMU API page protection flags
>>>   */
>>> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>>> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
>>> +		     unsigned long attrs)
>>>  {
>>>  	int prot = coherent ? IOMMU_CACHE : 0;
>>>
>>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>>> +		prot |= IOMMU_PRIV;
>>> +
>>>  	switch (dir) {
>>>  	case DMA_BIDIRECTIONAL:
>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>
>> ...and applying against -next now also needs this hunk:
>>
>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>> *dev, phys_addr_t phys,
>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>> {
>> 	return __iommu_dma_map(dev, phys, size,
>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>> }
>>
>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>
>> With those two issues fixed up, I've given the series (applied to
>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>
> 
> oops, sorry that i missed this in rebase. I can repost now with this fixed,
> 'checks out' you mean something is not working correct ?

No, I mean it _is_ still correct - I guess that's more of an idiom than
I thought :)

Robin.

> 
> Regards,
>  Sricharan  
> 

[toc] | [prev] | [next] | [standalone]

#1541147

Hi,

>
>On 13/12/16 14:38, Sricharan wrote:
>> Hi Robin,
>>
>>> -----Original Message-----
>>> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Robin Murphy
>>> Sent: Tuesday, December 13, 2016 7:33 PM
>>> To: Sricharan R <sricharan@codeaurora.org>; jcrouse@codeaurora.org; pdaly@codeaurora.org; jgebben@codeaurora.org;
>>> joro@8bytes.org; linux-kernel@vger.kernel.org; pratikp@codeaurora.org; iommu@lists.linux-foundation.org;
>tzeng@codeaurora.org;
>>> linux-arm-kernel@lists.infradead.org; will.deacon@arm.com; mitchelh@codeaurora.org; vinod.koul@intel.com
>>> Cc: dan.j.williams@intel.com
>>> Subject: Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>>
>>> On 12/12/16 18:38, Sricharan R wrote:
>>>> From: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>>
>>>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>>>> are only accessible to privileged DMA engines.  Implement it in
>>>> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
>>>>
>>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>>> Tested-by: Robin Murphy <robin.murphy@arm.com>
>>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>> ---
>>>>  arch/arm64/mm/dma-mapping.c |  6 +++---
>>>>  drivers/iommu/dma-iommu.c   | 10 ++++++++--
>>>>  include/linux/dma-iommu.h   |  3 ++-
>>>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>>> index 401f79a..ae76ead 100644
>>>> --- a/arch/arm64/mm/dma-mapping.c
>>>> +++ b/arch/arm64/mm/dma-mapping.c
>>>> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>>>>  				 unsigned long attrs)
>>>>  {
>>>>  	bool coherent = is_device_dma_coherent(dev);
>>>> -	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>>>> +	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>>>>  	size_t iosize = size;
>>>>  	void *addr;
>>>>
>>>> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>>>>  				   unsigned long attrs)
>>>>  {
>>>>  	bool coherent = is_device_dma_coherent(dev);
>>>> -	int prot = dma_direction_to_prot(dir, coherent);
>>>> +	int prot = dma_info_to_prot(dir, coherent, attrs);
>>>>  	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>>>>
>>>>  	if (!iommu_dma_mapping_error(dev, dev_addr) &&
>>>> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>>>>  		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>>>>
>>>>  	return iommu_dma_map_sg(dev, sgl, nelems,
>>>> -			dma_direction_to_prot(dir, coherent));
>>>> +				dma_info_to_prot(dir, coherent, attrs));
>>>>  }
>>>>
>>>>  static void __iommu_unmap_sg_attrs(struct device *dev,
>>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>>> index d2a7a46..756d5e0 100644
>>>> --- a/drivers/iommu/dma-iommu.c
>>>> +++ b/drivers/iommu/dma-iommu.c
>>>> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>>>  EXPORT_SYMBOL(iommu_dma_init_domain);
>>>>
>>>>  /**
>>>> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
>>>> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
>>>> + *                    page flags.
>>>>   * @dir: Direction of DMA transfer
>>>>   * @coherent: Is the DMA master cache-coherent?
>>>> + * @attrs: DMA attributes for the mapping
>>>>   *
>>>>   * Return: corresponding IOMMU API page protection flags
>>>>   */
>>>> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>>>> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
>>>> +		     unsigned long attrs)
>>>>  {
>>>>  	int prot = coherent ? IOMMU_CACHE : 0;
>>>>
>>>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>>>> +		prot |= IOMMU_PRIV;
>>>> +
>>>>  	switch (dir) {
>>>>  	case DMA_BIDIRECTIONAL:
>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>
>>> ...and applying against -next now also needs this hunk:
>>>
>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>> *dev, phys_addr_t phys,
>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>> {
>>> 	return __iommu_dma_map(dev, phys, size,
>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>> }
>>>
>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>
>>> With those two issues fixed up, I've given the series (applied to
>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>
>>
>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>> 'checks out' you mean something is not working correct ?
>
>No, I mean it _is_ still correct - I guess that's more of an idiom than
>I thought :)
>

ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.

Regards,
 Sricharan

[toc] | [prev] | [next] | [standalone]

#1541384

Hi Robin,

<snip..>

>>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>>
>>>> ...and applying against -next now also needs this hunk:
>>>>
>>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>>> *dev, phys_addr_t phys,
>>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>>> {
>>>> 	return __iommu_dma_map(dev, phys, size,
>>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>>> }
>>>>
>>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>>
>>>> With those two issues fixed up, I've given the series (applied to
>>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>>
>>>
>>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>>> 'checks out' you mean something is not working correct ?
>>
>>No, I mean it _is_ still correct - I guess that's more of an idiom than
>>I thought :)
>>
>
>ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.

Just while checking that i have not missed anything else, realized that the
dma-mapping apis in arm as to be modified to pass the PRIVILIGED attributes
as well. While my testing path was using the iommu_map directly i was not
seeing this, but then i did a patch like below. I will just figure out another
other codebase where the master uses the dma apis, test and add it in the
V8 that i would send.


From: Sricharan R <sricharan@codeaurora.org>
Date: Tue, 13 Dec 2016 23:25:01 +0530
Subject: [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED

The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
are only accessible to privileged DMA engines.  Implementing it in dma-mapping
for it to get used from the dma mappings apis.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 arch/arm/mm/dma-mapping.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ab77100..e0d9923 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1394,7 +1394,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
  * Create a mapping in device IO address space for specified pages
  */
 static dma_addr_t
-__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
+__iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
+		       unsigned long attrs)
 {
 	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
 	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
@@ -1419,7 +1420,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
 
 		len = (j - i) << PAGE_SHIFT;
 		ret = iommu_map(mapping->domain, iova, phys, len,
-				IOMMU_READ|IOMMU_WRITE);
+				__dma_info_to_prot(DMA_BIRECTIONAL, attrs));
 		if (ret < 0)
 			goto fail;
 		iova += len;
@@ -1476,7 +1477,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs)
 }
 
 static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
-				  dma_addr_t *handle, int coherent_flag)
+				  dma_addr_t *handle, int coherent_flag,
+				  unsigned long attrs)
 {
 	struct page *page;
 	void *addr;
@@ -1488,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
 	if (!addr)
 		return NULL;
 
-	*handle = __iommu_create_mapping(dev, &page, size);
+	*handle = __iommu_create_mapping(dev, &page, size, attrs);
 	if (*handle == DMA_ERROR_CODE)
 		goto err_mapping;
 
@@ -1522,7 +1524,8 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
 
 	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
 		return __iommu_alloc_simple(dev, size, gfp, handle,
-					    coherent_flag);
+					    coherent_flag,
+					    attrs);
 
 	/*
 	 * Following is a work-around (a.k.a. hack) to prevent pages
@@ -1672,10 +1675,13 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
 					 GFP_KERNEL);
 }
 
-static int __dma_direction_to_prot(enum dma_data_direction dir)
+static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
 {
 	int prot;
 
+	if (attrs & DMA_ATTR_PRIVILEGED)
+		prot |= IOMMU_PRIV;
+
 	switch (dir) {
 	case DMA_BIDIRECTIONAL:
 		prot = IOMMU_READ | IOMMU_WRITE;
@@ -1722,7 +1728,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
 		if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
 			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
 
-		prot = __dma_direction_to_prot(dir);
+		prot = __dma_info_to_prot(dir, attrs);
 
 		ret = iommu_map(mapping->domain, iova, phys, len, prot);
 		if (ret < 0)
@@ -1930,7 +1936,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
 	if (dma_addr == DMA_ERROR_CODE)
 		return dma_addr;
 
-	prot = __dma_direction_to_prot(dir);
+	prot = __dma_info_to_prot(dir, attrs);
 
 	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
 	if (ret < 0)
@@ -2036,7 +2042,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
 	if (dma_addr == DMA_ERROR_CODE)
 		return dma_addr;
 
-	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
+	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
 
 	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
 	if (ret < 0)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Regards,
 Sricharan

[toc] | [prev] | [next] | [standalone]

#1541405

On 13/12/16 18:46, Sricharan wrote:
> Hi Robin,
> 
> <snip..>
> 
>>>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>>>
>>>>> ...and applying against -next now also needs this hunk:
>>>>>
>>>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>>>> *dev, phys_addr_t phys,
>>>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>>>> {
>>>>> 	return __iommu_dma_map(dev, phys, size,
>>>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>>>> }
>>>>>
>>>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>>>
>>>>> With those two issues fixed up, I've given the series (applied to
>>>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>>>
>>>>
>>>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>>>> 'checks out' you mean something is not working correct ?
>>>
>>> No, I mean it _is_ still correct - I guess that's more of an idiom than
>>> I thought :)
>>>
>>
>> ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.
> 
> Just while checking that i have not missed anything else, realized that the
> dma-mapping apis in arm as to be modified to pass the PRIVILIGED attributes
> as well. While my testing path was using the iommu_map directly i was not
> seeing this, but then i did a patch like below. I will just figure out another
> other codebase where the master uses the dma apis, test and add it in the
> V8 that i would send.

True, adding support to 32-bit as well can't hurt, and I guess it's
equally relevant to QC's GPU use-case. I haven't considered it myself
because AArch32 is immune to the specific PL330 problem which caught me
out - that subtle corner of VMSAv8 is unique to AArch64.

> From: Sricharan R <sricharan@codeaurora.org>
> Date: Tue, 13 Dec 2016 23:25:01 +0530
> Subject: [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED
> 
> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
> are only accessible to privileged DMA engines.  Implementing it in dma-mapping
> for it to get used from the dma mappings apis.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
>  arch/arm/mm/dma-mapping.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index ab77100..e0d9923 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1394,7 +1394,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>   * Create a mapping in device IO address space for specified pages
>   */
>  static dma_addr_t
> -__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
> +__iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
> +		       unsigned long attrs)
>  {
>  	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
>  	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> @@ -1419,7 +1420,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>  
>  		len = (j - i) << PAGE_SHIFT;
>  		ret = iommu_map(mapping->domain, iova, phys, len,
> -				IOMMU_READ|IOMMU_WRITE);
> +				__dma_info_to_prot(DMA_BIRECTIONAL, attrs));
>  		if (ret < 0)
>  			goto fail;
>  		iova += len;
> @@ -1476,7 +1477,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs)
>  }
>  
>  static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
> -				  dma_addr_t *handle, int coherent_flag)
> +				  dma_addr_t *handle, int coherent_flag,
> +				  unsigned long attrs)
>  {
>  	struct page *page;
>  	void *addr;
> @@ -1488,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
>  	if (!addr)
>  		return NULL;
>  
> -	*handle = __iommu_create_mapping(dev, &page, size);
> +	*handle = __iommu_create_mapping(dev, &page, size, attrs);
>  	if (*handle == DMA_ERROR_CODE)
>  		goto err_mapping;
>  
> @@ -1522,7 +1524,8 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
>  
>  	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
>  		return __iommu_alloc_simple(dev, size, gfp, handle,
> -					    coherent_flag);
> +					    coherent_flag,
> +					    attrs);

Super-nit: unnecessary line break.

>  
>  	/*
>  	 * Following is a work-around (a.k.a. hack) to prevent pages
> @@ -1672,10 +1675,13 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
>  					 GFP_KERNEL);
>  }
>  
> -static int __dma_direction_to_prot(enum dma_data_direction dir)
> +static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
>  {
>  	int prot;
>  
> +	if (attrs & DMA_ATTR_PRIVILEGED)
> +		prot |= IOMMU_PRIV;
> +
>  	switch (dir) {
>  	case DMA_BIDIRECTIONAL:
>  		prot = IOMMU_READ | IOMMU_WRITE;
> @@ -1722,7 +1728,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
>  		if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
>  			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
>  
> -		prot = __dma_direction_to_prot(dir);
> +		prot = __dma_info_to_prot(dir, attrs);
>  
>  		ret = iommu_map(mapping->domain, iova, phys, len, prot);
>  		if (ret < 0)
> @@ -1930,7 +1936,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
>  	if (dma_addr == DMA_ERROR_CODE)
>  		return dma_addr;
>  
> -	prot = __dma_direction_to_prot(dir);
> +	prot = __dma_info_to_prot(dir, attrs);
>  
>  	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
>  	if (ret < 0)
> @@ -2036,7 +2042,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
>  	if (dma_addr == DMA_ERROR_CODE)
>  		return dma_addr;
>  
> -	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
> +	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
>  
>  	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
>  	if (ret < 0)
> 

Looks reasonable to me. Assuming it survives testing:

Acked-by: Robin Murphy <robin.murphy@arm.com>

[toc] | [prev] | [next] | [standalone]

#1549181

Hi Robin,

>>
>> <snip..>
>>
>>>>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>>>>
>>>>>> ...and applying against -next now also needs this hunk:
>>>>>>
>>>>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>>>>> *dev, phys_addr_t phys,
>>>>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>>>>> {
>>>>>> 	return __iommu_dma_map(dev, phys, size,
>>>>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>>>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>>>>> }
>>>>>>
>>>>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>>>>
>>>>>> With those two issues fixed up, I've given the series (applied to
>>>>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>>>>
>>>>>
>>>>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>>>>> 'checks out' you mean something is not working correct ?
>>>>
>>>> No, I mean it _is_ still correct - I guess that's more of an idiom than
>>>> I thought :)
>>>>
>>>
>>> ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.
>>
>> Just while checking that i have not missed anything else, realized that the
>> dma-mapping apis in arm as to be modified to pass the PRIVILIGED attributes
>> as well. While my testing path was using the iommu_map directly i was not
>> seeing this, but then i did a patch like below. I will just figure out another
>> other codebase where the master uses the dma apis, test and add it in the
>> V8 that i would send.
>
>True, adding support to 32-bit as well can't hurt, and I guess it's
>equally relevant to QC's GPU use-case. I haven't considered it myself
>because AArch32 is immune to the specific PL330 problem which caught me
>out - that subtle corner of VMSAv8 is unique to AArch64.
>
>> From: Sricharan R <sricharan@codeaurora.org>
>> Date: Tue, 13 Dec 2016 23:25:01 +0530
>> Subject: [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>
>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>> are only accessible to privileged DMA engines.  Implementing it in dma-mapping
>> for it to get used from the dma mappings apis.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>>  arch/arm/mm/dma-mapping.c | 24 +++++++++++++++---------
>>  1 file changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index ab77100..e0d9923 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -1394,7 +1394,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>>   * Create a mapping in device IO address space for specified pages
>>   */
>>  static dma_addr_t
>> -__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
>> +__iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
>> +		       unsigned long attrs)
>>  {
>>  	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
>>  	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> @@ -1419,7 +1420,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>>
>>  		len = (j - i) << PAGE_SHIFT;
>>  		ret = iommu_map(mapping->domain, iova, phys, len,
>> -				IOMMU_READ|IOMMU_WRITE);
>> +				__dma_info_to_prot(DMA_BIRECTIONAL, attrs));
>>  		if (ret < 0)
>>  			goto fail;
>>  		iova += len;
>> @@ -1476,7 +1477,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs)
>>  }
>>
>>  static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
>> -				  dma_addr_t *handle, int coherent_flag)
>> +				  dma_addr_t *handle, int coherent_flag,
>> +				  unsigned long attrs)
>>  {
>>  	struct page *page;
>>  	void *addr;
>> @@ -1488,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
>>  	if (!addr)
>>  		return NULL;
>>
>> -	*handle = __iommu_create_mapping(dev, &page, size);
>> +	*handle = __iommu_create_mapping(dev, &page, size, attrs);
>>  	if (*handle == DMA_ERROR_CODE)
>>  		goto err_mapping;
>>
>> @@ -1522,7 +1524,8 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
>>
>>  	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
>>  		return __iommu_alloc_simple(dev, size, gfp, handle,
>> -					    coherent_flag);
>> +					    coherent_flag,
>> +					    attrs);
>
>Super-nit: unnecessary line break.
>
>>
>>  	/*
>>  	 * Following is a work-around (a.k.a. hack) to prevent pages
>> @@ -1672,10 +1675,13 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
>>  					 GFP_KERNEL);
>>  }
>>
>> -static int __dma_direction_to_prot(enum dma_data_direction dir)
>> +static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
>>  {
>>  	int prot;
>>
>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>> +		prot |= IOMMU_PRIV;
>> +
>>  	switch (dir) {
>>  	case DMA_BIDIRECTIONAL:
>>  		prot = IOMMU_READ | IOMMU_WRITE;
>> @@ -1722,7 +1728,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
>>  		if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
>>  			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
>>
>> -		prot = __dma_direction_to_prot(dir);
>> +		prot = __dma_info_to_prot(dir, attrs);
>>
>>  		ret = iommu_map(mapping->domain, iova, phys, len, prot);
>>  		if (ret < 0)
>> @@ -1930,7 +1936,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
>>  	if (dma_addr == DMA_ERROR_CODE)
>>  		return dma_addr;
>>
>> -	prot = __dma_direction_to_prot(dir);
>> +	prot = __dma_info_to_prot(dir, attrs);
>>
>>  	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
>>  	if (ret < 0)
>> @@ -2036,7 +2042,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
>>  	if (dma_addr == DMA_ERROR_CODE)
>>  		return dma_addr;
>>
>> -	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
>> +	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
>>
>>  	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
>>  	if (ret < 0)
>>
>
>Looks reasonable to me. Assuming it survives testing:
>
>Acked-by: Robin Murphy <robin.murphy@arm.com>

Posted V8 [1]. I changed a few more things after the testing,
though functionally the same. So have not taken your ack and
would be nice to have it once again

[1] https://lkml.org/lkml/2017/1/2/224

Regards,
 Sricharan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web