iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
@ 2019-10-13 14:28 Daniele Alessandrelli
  2019-10-14 13:54 ` Robin Murphy
  0 siblings, 1 reply; 15+ messages in thread
From: Daniele Alessandrelli @ 2019-10-13 14:28 UTC (permalink / raw)
  To: Christoph Hellwig, Marek Szyprowski, Robin Murphy
  Cc: iommu, Vladimir Murzin, Arnd Bergmann

Hi,

It looks like dma_alloc_coherent() is setting the dma_handle output
parameter to the memory physical address and not the device bus
address when the device is using reserved memory regions for DMA
allocation. This is despite using 'dma_ranges' in the device tree to
describe the DMA memory mapping. Is this expected behavior or a bug?

Here is a reduced version of the device tree I'm using:
\ {
        reserved-memory {
                #address-cells = <2>;
                #size-cells = <2>;
                ranges;
                mydev_rsvd: rsvd_mem@494800000 {
                        compatible = "shared-dma-pool";
                        reg = <0x4 0x94800000 0x0 0x200000>;
                        no-map;
                };
        };
        soc {
                compatible = "simple-bus";
                #address-cells = <2>;
                #size-cells = <2>;
                ranges;
                dma_ranges;

                mybus {
                        ranges = <>;
                        dma-ranges = <>;
                        compatible = "simple-bus";
                        #address-cells = <2>;
                        #size-cells = <2>;
                        ranges =     <0x0 0x0  0x0 0x0  0x0 0x80000000>;
                        dma-ranges = <0x0 0x80000000  0x4 0x80000000
0x0 0x80000000>;

                        mydevice {
                                compatible = "my-compatible-string";
                                memory-region = <&mydev_rsvd>;
                        }
                }
        }
};

It looks like this issue was previously fixed by commit c41f9ea998f3
("drivers: dma-coherent: Account dma_pfn_offset when used with device
tree") which introduced a new function ('dma_get_device_base()') to
return the reserved memory address as seen by the device. However,
such a function, even if still there, is not used anymore in latest
code (as of v5.4-rc2). Was that done for a specific reason? Or is it
just a mistake?

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-13 14:28 [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug? Daniele Alessandrelli
@ 2019-10-14 13:54 ` Robin Murphy
  2019-10-14 15:01   ` Vladimir Murzin
  0 siblings, 1 reply; 15+ messages in thread
From: Robin Murphy @ 2019-10-14 13:54 UTC (permalink / raw)
  To: Daniele Alessandrelli, Christoph Hellwig, Marek Szyprowski,
	Vladimir Murzin
  Cc: iommu, Arnd Bergmann

On 13/10/2019 15:28, Daniele Alessandrelli wrote:
> Hi,
> 
> It looks like dma_alloc_coherent() is setting the dma_handle output
> parameter to the memory physical address and not the device bus
> address when the device is using reserved memory regions for DMA
> allocation. This is despite using 'dma_ranges' in the device tree to
> describe the DMA memory mapping. Is this expected behavior or a bug?

That does sound like a bug :(

> Here is a reduced version of the device tree I'm using:
> \ {
>          reserved-memory {
>                  #address-cells = <2>;
>                  #size-cells = <2>;
>                  ranges;
>                  mydev_rsvd: rsvd_mem@494800000 {
>                          compatible = "shared-dma-pool";
>                          reg = <0x4 0x94800000 0x0 0x200000>;
>                          no-map;
>                  };
>          };
>          soc {
>                  compatible = "simple-bus";
>                  #address-cells = <2>;
>                  #size-cells = <2>;
>                  ranges;
>                  dma_ranges;
> 
>                  mybus {
>                          ranges = <>;
>                          dma-ranges = <>;
>                          compatible = "simple-bus";
>                          #address-cells = <2>;
>                          #size-cells = <2>;
>                          ranges =     <0x0 0x0  0x0 0x0  0x0 0x80000000>;
>                          dma-ranges = <0x0 0x80000000  0x4 0x80000000
> 0x0 0x80000000>;
> 
>                          mydevice {
>                                  compatible = "my-compatible-string";
>                                  memory-region = <&mydev_rsvd>;
>                          }
>                  }
>          }
> };
> 
> It looks like this issue was previously fixed by commit c41f9ea998f3
> ("drivers: dma-coherent: Account dma_pfn_offset when used with device
> tree") which introduced a new function ('dma_get_device_base()') to
> return the reserved memory address as seen by the device. However,
> such a function, even if still there, is not used anymore in latest
> code (as of v5.4-rc2). Was that done for a specific reason? Or is it
> just a mistake?

Hmm, it looks like 43fc509c3efb ("dma-coherent: introduce interface for 
default DMA pool") removed the caller of dma_get_device_base() in the 
alloc path shortly after it was introduced, which certainly appears as 
if it may have been unintentional - Vladimir?

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-14 13:54 ` Robin Murphy
@ 2019-10-14 15:01   ` Vladimir Murzin
  2019-10-17  9:46     ` Vladimir Murzin
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Murzin @ 2019-10-14 15:01 UTC (permalink / raw)
  To: Robin Murphy, Daniele Alessandrelli, Christoph Hellwig, Marek Szyprowski
  Cc: iommu, alexandre.torgue, Arnd Bergmann

On 10/14/19 2:54 PM, Robin Murphy wrote:
> On 13/10/2019 15:28, Daniele Alessandrelli wrote:
>> Hi,
>>
>> It looks like dma_alloc_coherent() is setting the dma_handle output
>> parameter to the memory physical address and not the device bus
>> address when the device is using reserved memory regions for DMA
>> allocation. This is despite using 'dma_ranges' in the device tree to
>> describe the DMA memory mapping. Is this expected behavior or a bug?
> 
> That does sound like a bug :(
> 
>> Here is a reduced version of the device tree I'm using:
>> \ {
>>          reserved-memory {
>>                  #address-cells = <2>;
>>                  #size-cells = <2>;
>>                  ranges;
>>                  mydev_rsvd: rsvd_mem@494800000 {
>>                          compatible = "shared-dma-pool";
>>                          reg = <0x4 0x94800000 0x0 0x200000>;
>>                          no-map;
>>                  };
>>          };
>>          soc {
>>                  compatible = "simple-bus";
>>                  #address-cells = <2>;
>>                  #size-cells = <2>;
>>                  ranges;
>>                  dma_ranges;
>>
>>                  mybus {
>>                          ranges = <>;
>>                          dma-ranges = <>;
>>                          compatible = "simple-bus";
>>                          #address-cells = <2>;
>>                          #size-cells = <2>;
>>                          ranges =     <0x0 0x0  0x0 0x0  0x0 0x80000000>;
>>                          dma-ranges = <0x0 0x80000000  0x4 0x80000000
>> 0x0 0x80000000>;
>>
>>                          mydevice {
>>                                  compatible = "my-compatible-string";
>>                                  memory-region = <&mydev_rsvd>;
>>                          }
>>                  }
>>          }
>> };
>>
>> It looks like this issue was previously fixed by commit c41f9ea998f3
>> ("drivers: dma-coherent: Account dma_pfn_offset when used with device
>> tree") which introduced a new function ('dma_get_device_base()') to
>> return the reserved memory address as seen by the device. However,
>> such a function, even if still there, is not used anymore in latest
>> code (as of v5.4-rc2). Was that done for a specific reason? Or is it
>> just a mistake?
> 
> Hmm, it looks like 43fc509c3efb ("dma-coherent: introduce interface for default DMA pool") removed the caller of dma_get_device_base() in the alloc path shortly after it was introduced, which certainly appears as if it may have been unintentional - Vladimir?

I do not remember it was intentional. Looking at history, default DMA pool was a response
to another report. However, now I'm wondering why it was not caught by STM32 - most of that
work was required to support "dma-ranges" with NOMMU+caches (Cortex-M7).

Alex (or anybody else from ST), maybe you have some input?

Cheers
Vladimir

> 
> Robin.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-14 15:01   ` Vladimir Murzin
@ 2019-10-17  9:46     ` Vladimir Murzin
  2019-10-17 10:03       ` Alexandre Torgue
  2019-10-24 12:43       ` Vladimir Murzin
  0 siblings, 2 replies; 15+ messages in thread
From: Vladimir Murzin @ 2019-10-17  9:46 UTC (permalink / raw)
  To: Robin Murphy, Daniele Alessandrelli, Christoph Hellwig, Marek Szyprowski
  Cc: iommu, alexandre.torgue, Arnd Bergmann

On 10/14/19 4:01 PM, Vladimir Murzin wrote:
> On 10/14/19 2:54 PM, Robin Murphy wrote:
>> On 13/10/2019 15:28, Daniele Alessandrelli wrote:
>>> Hi,
>>>
>>> It looks like dma_alloc_coherent() is setting the dma_handle output
>>> parameter to the memory physical address and not the device bus
>>> address when the device is using reserved memory regions for DMA
>>> allocation. This is despite using 'dma_ranges' in the device tree to
>>> describe the DMA memory mapping. Is this expected behavior or a bug?
>>
>> That does sound like a bug :(
>>
>>> Here is a reduced version of the device tree I'm using:
>>> \ {
>>>          reserved-memory {
>>>                  #address-cells = <2>;
>>>                  #size-cells = <2>;
>>>                  ranges;
>>>                  mydev_rsvd: rsvd_mem@494800000 {
>>>                          compatible = "shared-dma-pool";
>>>                          reg = <0x4 0x94800000 0x0 0x200000>;
>>>                          no-map;
>>>                  };
>>>          };
>>>          soc {
>>>                  compatible = "simple-bus";
>>>                  #address-cells = <2>;
>>>                  #size-cells = <2>;
>>>                  ranges;
>>>                  dma_ranges;
>>>
>>>                  mybus {
>>>                          ranges = <>;
>>>                          dma-ranges = <>;
>>>                          compatible = "simple-bus";
>>>                          #address-cells = <2>;
>>>                          #size-cells = <2>;
>>>                          ranges =     <0x0 0x0  0x0 0x0  0x0 0x80000000>;
>>>                          dma-ranges = <0x0 0x80000000  0x4 0x80000000
>>> 0x0 0x80000000>;
>>>
>>>                          mydevice {
>>>                                  compatible = "my-compatible-string";
>>>                                  memory-region = <&mydev_rsvd>;
>>>                          }
>>>                  }
>>>          }
>>> };
>>>
>>> It looks like this issue was previously fixed by commit c41f9ea998f3
>>> ("drivers: dma-coherent: Account dma_pfn_offset when used with device
>>> tree") which introduced a new function ('dma_get_device_base()') to
>>> return the reserved memory address as seen by the device. However,
>>> such a function, even if still there, is not used anymore in latest
>>> code (as of v5.4-rc2). Was that done for a specific reason? Or is it
>>> just a mistake?
>>
>> Hmm, it looks like 43fc509c3efb ("dma-coherent: introduce interface for default DMA pool") removed the caller of dma_get_device_base() in the alloc path shortly after it was introduced, which certainly appears as if it may have been unintentional - Vladimir?
> 
> I do not remember it was intentional. Looking at history, default DMA pool was a response
> to another report. However, now I'm wondering why it was not caught by STM32 - most of that
> work was required to support "dma-ranges" with NOMMU+caches (Cortex-M7).
> 
> Alex (or anybody else from ST), maybe you have some input?

Seem they do not care :)

I'm wondering if I've missed something with diff bellow (it was a long time ago when I touched DMA)?

diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index db92478..287ef89 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
 				 unsigned long attrs)
 
 {
-	void *ret = dma_alloc_from_global_coherent(size, dma_handle);
+	void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
 
 	/*
 	 * dma_alloc_from_global_coherent() may fail because:
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4a1c4fc..10918c5 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
 int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
 			    void *cpu_addr, size_t size, int *ret);
 
-void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
+void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
 int dma_release_from_global_coherent(int order, void *vaddr);
 int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
 				  size_t size, int *ret);
@@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
 #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
 #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
 
-static inline void *dma_alloc_from_global_coherent(ssize_t size,
+static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
 						   dma_addr_t *dma_handle)
 {
 	return NULL;
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index 545e386..551b0eb 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
 	return ret;
 }
 
-static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
-		ssize_t size, dma_addr_t *dma_handle)
+static void *__dma_alloc_from_coherent(struct device *dev,
+				       struct dma_coherent_mem *mem,
+				       ssize_t size, dma_addr_t *dma_handle)
 {
 	int order = get_order(size);
 	unsigned long flags;
@@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
 	/*
 	 * Memory was found in the coherent area.
 	 */
-	*dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
+	*dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
 	ret = mem->virt_base + (pageno << PAGE_SHIFT);
 	spin_unlock_irqrestore(&mem->spinlock, flags);
 	memset(ret, 0, size);
@@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
 	if (!mem)
 		return 0;
 
-	*ret = __dma_alloc_from_coherent(mem, size, dma_handle);
+	*ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
 	return 1;
 }
 
-void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
+void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
+				     dma_addr_t *dma_handle)
 {
 	if (!dma_coherent_default_memory)
 		return NULL;
 
-	return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
-			dma_handle);
+	return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
+					 dma_handle);
 }
 
 static int __dma_release_from_coherent(struct dma_coherent_mem *mem,

> 
> Cheers
> Vladimir
> 
>>
>> Robin.
> 

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-17  9:46     ` Vladimir Murzin
@ 2019-10-17 10:03       ` Alexandre Torgue
  2019-10-17 10:16         ` Vladimir Murzin
  2019-10-24 12:43       ` Vladimir Murzin
  1 sibling, 1 reply; 15+ messages in thread
From: Alexandre Torgue @ 2019-10-17 10:03 UTC (permalink / raw)
  To: Vladimir Murzin, Robin Murphy, Daniele Alessandrelli,
	Christoph Hellwig, Marek Szyprowski
  Cc: iommu, Arnd Bergmann

Hi Vlad

On 10/17/19 11:46 AM, Vladimir Murzin wrote:
> On 10/14/19 4:01 PM, Vladimir Murzin wrote:
>> On 10/14/19 2:54 PM, Robin Murphy wrote:
>>> On 13/10/2019 15:28, Daniele Alessandrelli wrote:
>>>> Hi,
>>>>
>>>> It looks like dma_alloc_coherent() is setting the dma_handle output
>>>> parameter to the memory physical address and not the device bus
>>>> address when the device is using reserved memory regions for DMA
>>>> allocation. This is despite using 'dma_ranges' in the device tree to
>>>> describe the DMA memory mapping. Is this expected behavior or a bug?
>>>
>>> That does sound like a bug :(
>>>
>>>> Here is a reduced version of the device tree I'm using:
>>>> \ {
>>>>           reserved-memory {
>>>>                   #address-cells = <2>;
>>>>                   #size-cells = <2>;
>>>>                   ranges;
>>>>                   mydev_rsvd: rsvd_mem@494800000 {
>>>>                           compatible = "shared-dma-pool";
>>>>                           reg = <0x4 0x94800000 0x0 0x200000>;
>>>>                           no-map;
>>>>                   };
>>>>           };
>>>>           soc {
>>>>                   compatible = "simple-bus";
>>>>                   #address-cells = <2>;
>>>>                   #size-cells = <2>;
>>>>                   ranges;
>>>>                   dma_ranges;
>>>>
>>>>                   mybus {
>>>>                           ranges = <>;
>>>>                           dma-ranges = <>;
>>>>                           compatible = "simple-bus";
>>>>                           #address-cells = <2>;
>>>>                           #size-cells = <2>;
>>>>                           ranges =     <0x0 0x0  0x0 0x0  0x0 0x80000000>;
>>>>                           dma-ranges = <0x0 0x80000000  0x4 0x80000000
>>>> 0x0 0x80000000>;
>>>>
>>>>                           mydevice {
>>>>                                   compatible = "my-compatible-string";
>>>>                                   memory-region = <&mydev_rsvd>;
>>>>                           }
>>>>                   }
>>>>           }
>>>> };
>>>>
>>>> It looks like this issue was previously fixed by commit c41f9ea998f3
>>>> ("drivers: dma-coherent: Account dma_pfn_offset when used with device
>>>> tree") which introduced a new function ('dma_get_device_base()') to
>>>> return the reserved memory address as seen by the device. However,
>>>> such a function, even if still there, is not used anymore in latest
>>>> code (as of v5.4-rc2). Was that done for a specific reason? Or is it
>>>> just a mistake?
>>>
>>> Hmm, it looks like 43fc509c3efb ("dma-coherent: introduce interface for default DMA pool") removed the caller of dma_get_device_base() in the alloc path shortly after it was introduced, which certainly appears as if it may have been unintentional - Vladimir?
>>
>> I do not remember it was intentional. Looking at history, default DMA pool was a response
>> to another report. However, now I'm wondering why it was not caught by STM32 - most of that
>> work was required to support "dma-ranges" with NOMMU+caches (Cortex-M7).
>>
>> Alex (or anybody else from ST), maybe you have some input?
> 
> Seem they do not care :)
> 


Concerning, the default dma pool:

- It was introduced because dma on cortexM7 couldn't use cache-able 
memory region. So we configured MPU in bootloader (I think it can be 
done now by Kernel) a small "no cache" memory region and then declare 
this same region in kernel device tree for the dma pool. Without that we 
can't execute any dma trasferts.

Sorry for the late answer :(

alex> I'm wondering if I've missed something with diff bellow (it was a 
long time ago when I touched DMA)?
> 
> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
> index db92478..287ef89 100644
> --- a/arch/arm/mm/dma-mapping-nommu.c
> +++ b/arch/arm/mm/dma-mapping-nommu.c
> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>   				 unsigned long attrs)
>   
>   {
> -	void *ret = dma_alloc_from_global_coherent(size, dma_handle);
> +	void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
>   
>   	/*
>   	 * dma_alloc_from_global_coherent() may fail because:
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 4a1c4fc..10918c5 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
>   int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
>   			    void *cpu_addr, size_t size, int *ret);
>   
> -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
>   int dma_release_from_global_coherent(int order, void *vaddr);
>   int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>   				  size_t size, int *ret);
> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>   #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
>   #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
>   
> -static inline void *dma_alloc_from_global_coherent(ssize_t size,
> +static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>   						   dma_addr_t *dma_handle)
>   {
>   	return NULL;
> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> index 545e386..551b0eb 100644
> --- a/kernel/dma/coherent.c
> +++ b/kernel/dma/coherent.c
> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>   	return ret;
>   }
>   
> -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
> -		ssize_t size, dma_addr_t *dma_handle)
> +static void *__dma_alloc_from_coherent(struct device *dev,
> +				       struct dma_coherent_mem *mem,
> +				       ssize_t size, dma_addr_t *dma_handle)
>   {
>   	int order = get_order(size);
>   	unsigned long flags;
> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>   	/*
>   	 * Memory was found in the coherent area.
>   	 */
> -	*dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
> +	*dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
>   	ret = mem->virt_base + (pageno << PAGE_SHIFT);
>   	spin_unlock_irqrestore(&mem->spinlock, flags);
>   	memset(ret, 0, size);
> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
>   	if (!mem)
>   		return 0;
>   
> -	*ret = __dma_alloc_from_coherent(mem, size, dma_handle);
> +	*ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
>   	return 1;
>   }
>   
> -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
> +				     dma_addr_t *dma_handle)
>   {
>   	if (!dma_coherent_default_memory)
>   		return NULL;
>   
> -	return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
> -			dma_handle);
> +	return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
> +					 dma_handle);
>   }
>   
>   static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
> 
>>
>> Cheers
>> Vladimir
>>
>>>
>>> Robin.
>>
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-17 10:03       ` Alexandre Torgue
@ 2019-10-17 10:16         ` Vladimir Murzin
  2019-10-17 10:33           ` Alexandre Torgue
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Murzin @ 2019-10-17 10:16 UTC (permalink / raw)
  To: Alexandre Torgue, Robin Murphy, Daniele Alessandrelli,
	Christoph Hellwig, Marek Szyprowski
  Cc: iommu, Arnd Bergmann

On 10/17/19 11:03 AM, Alexandre Torgue wrote:
> Hi Vlad
> 
> On 10/17/19 11:46 AM, Vladimir Murzin wrote:
>> On 10/14/19 4:01 PM, Vladimir Murzin wrote:
>>> On 10/14/19 2:54 PM, Robin Murphy wrote:
>>>> On 13/10/2019 15:28, Daniele Alessandrelli wrote:
>>>>> Hi,
>>>>>
>>>>> It looks like dma_alloc_coherent() is setting the dma_handle output
>>>>> parameter to the memory physical address and not the device bus
>>>>> address when the device is using reserved memory regions for DMA
>>>>> allocation. This is despite using 'dma_ranges' in the device tree to
>>>>> describe the DMA memory mapping. Is this expected behavior or a bug?
>>>>
>>>> That does sound like a bug :(
>>>>
>>>>> Here is a reduced version of the device tree I'm using:
>>>>> \ {
>>>>>           reserved-memory {
>>>>>                   #address-cells = <2>;
>>>>>                   #size-cells = <2>;
>>>>>                   ranges;
>>>>>                   mydev_rsvd: rsvd_mem@494800000 {
>>>>>                           compatible = "shared-dma-pool";
>>>>>                           reg = <0x4 0x94800000 0x0 0x200000>;
>>>>>                           no-map;
>>>>>                   };
>>>>>           };
>>>>>           soc {
>>>>>                   compatible = "simple-bus";
>>>>>                   #address-cells = <2>;
>>>>>                   #size-cells = <2>;
>>>>>                   ranges;
>>>>>                   dma_ranges;
>>>>>
>>>>>                   mybus {
>>>>>                           ranges = <>;
>>>>>                           dma-ranges = <>;
>>>>>                           compatible = "simple-bus";
>>>>>                           #address-cells = <2>;
>>>>>                           #size-cells = <2>;
>>>>>                           ranges =     <0x0 0x0  0x0 0x0  0x0 0x80000000>;
>>>>>                           dma-ranges = <0x0 0x80000000  0x4 0x80000000
>>>>> 0x0 0x80000000>;
>>>>>
>>>>>                           mydevice {
>>>>>                                   compatible = "my-compatible-string";
>>>>>                                   memory-region = <&mydev_rsvd>;
>>>>>                           }
>>>>>                   }
>>>>>           }
>>>>> };
>>>>>
>>>>> It looks like this issue was previously fixed by commit c41f9ea998f3
>>>>> ("drivers: dma-coherent: Account dma_pfn_offset when used with device
>>>>> tree") which introduced a new function ('dma_get_device_base()') to
>>>>> return the reserved memory address as seen by the device. However,
>>>>> such a function, even if still there, is not used anymore in latest
>>>>> code (as of v5.4-rc2). Was that done for a specific reason? Or is it
>>>>> just a mistake?
>>>>
>>>> Hmm, it looks like 43fc509c3efb ("dma-coherent: introduce interface for default DMA pool") removed the caller of dma_get_device_base() in the alloc path shortly after it was introduced, which certainly appears as if it may have been unintentional - Vladimir?
>>>
>>> I do not remember it was intentional. Looking at history, default DMA pool was a response
>>> to another report. However, now I'm wondering why it was not caught by STM32 - most of that
>>> work was required to support "dma-ranges" with NOMMU+caches (Cortex-M7).
>>>
>>> Alex (or anybody else from ST), maybe you have some input?
>>
>> Seem they do not care :)
>>
> 
> 
> Concerning, the default dma pool:
> 
> - It was introduced because dma on cortexM7 couldn't use cache-able memory region. So we configured MPU in bootloader (I think it can be done now by Kernel) a small "no cache" memory region and then declare this same region in kernel device tree for the dma pool. Without that we can't execute any dma trasferts.
> 
> Sorry for the late answer :(

Still better then no answer ;)

Yes, I remember why we have default DMA pool. IIRC, STM used dma-ranges, it is how
c41f9ea998f3 ("drivers: dma-coherent: Account dma_pfn_offset when used with device
tree") was discovered. It also means that STM is broken after 878ec36 (ARM: NOMMU:
Wire-up default DMA interface) with combination of dma-ranges and default DMA pool.
Hoverer, I've never seen report from STM, so either I'm missing something or such
combination is not in use nowdays...

Cheers
Vladimir

> 
> alex> I'm wondering if I've missed something with diff bellow (it was a long time ago when I touched DMA)?
>>
>> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
>> index db92478..287ef89 100644
>> --- a/arch/arm/mm/dma-mapping-nommu.c
>> +++ b/arch/arm/mm/dma-mapping-nommu.c
>> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>>                    unsigned long attrs)
>>     {
>> -    void *ret = dma_alloc_from_global_coherent(size, dma_handle);
>> +    void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
>>         /*
>>        * dma_alloc_from_global_coherent() may fail because:
>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>> index 4a1c4fc..10918c5 100644
>> --- a/include/linux/dma-mapping.h
>> +++ b/include/linux/dma-mapping.h
>> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
>>   int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
>>                   void *cpu_addr, size_t size, int *ret);
>>   -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
>>   int dma_release_from_global_coherent(int order, void *vaddr);
>>   int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>                     size_t size, int *ret);
>> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>   #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
>>   #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
>>   -static inline void *dma_alloc_from_global_coherent(ssize_t size,
>> +static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>                              dma_addr_t *dma_handle)
>>   {
>>       return NULL;
>> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
>> index 545e386..551b0eb 100644
>> --- a/kernel/dma/coherent.c
>> +++ b/kernel/dma/coherent.c
>> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>>       return ret;
>>   }
>>   -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>> -        ssize_t size, dma_addr_t *dma_handle)
>> +static void *__dma_alloc_from_coherent(struct device *dev,
>> +                       struct dma_coherent_mem *mem,
>> +                       ssize_t size, dma_addr_t *dma_handle)
>>   {
>>       int order = get_order(size);
>>       unsigned long flags;
>> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>       /*
>>        * Memory was found in the coherent area.
>>        */
>> -    *dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
>> +    *dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
>>       ret = mem->virt_base + (pageno << PAGE_SHIFT);
>>       spin_unlock_irqrestore(&mem->spinlock, flags);
>>       memset(ret, 0, size);
>> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
>>       if (!mem)
>>           return 0;
>>   -    *ret = __dma_alloc_from_coherent(mem, size, dma_handle);
>> +    *ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
>>       return 1;
>>   }
>>   -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>> +                     dma_addr_t *dma_handle)
>>   {
>>       if (!dma_coherent_default_memory)
>>           return NULL;
>>   -    return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
>> -            dma_handle);
>> +    return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
>> +                     dma_handle);
>>   }
>>     static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
>>
>>>
>>> Cheers
>>> Vladimir
>>>
>>>>
>>>> Robin.
>>>
>>

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-17 10:16         ` Vladimir Murzin
@ 2019-10-17 10:33           ` Alexandre Torgue
  0 siblings, 0 replies; 15+ messages in thread
From: Alexandre Torgue @ 2019-10-17 10:33 UTC (permalink / raw)
  To: Vladimir Murzin, Robin Murphy, Daniele Alessandrelli,
	Christoph Hellwig, Marek Szyprowski
  Cc: iommu, Arnd Bergmann



On 10/17/19 12:16 PM, Vladimir Murzin wrote:
> On 10/17/19 11:03 AM, Alexandre Torgue wrote:
>> Hi Vlad
>>
>> On 10/17/19 11:46 AM, Vladimir Murzin wrote:
>>> On 10/14/19 4:01 PM, Vladimir Murzin wrote:
>>>> On 10/14/19 2:54 PM, Robin Murphy wrote:
>>>>> On 13/10/2019 15:28, Daniele Alessandrelli wrote:
>>>>>> Hi,
>>>>>>
>>>>>> It looks like dma_alloc_coherent() is setting the dma_handle output
>>>>>> parameter to the memory physical address and not the device bus
>>>>>> address when the device is using reserved memory regions for DMA
>>>>>> allocation. This is despite using 'dma_ranges' in the device tree to
>>>>>> describe the DMA memory mapping. Is this expected behavior or a bug?
>>>>>
>>>>> That does sound like a bug :(
>>>>>
>>>>>> Here is a reduced version of the device tree I'm using:
>>>>>> \ {
>>>>>>            reserved-memory {
>>>>>>                    #address-cells = <2>;
>>>>>>                    #size-cells = <2>;
>>>>>>                    ranges;
>>>>>>                    mydev_rsvd: rsvd_mem@494800000 {
>>>>>>                            compatible = "shared-dma-pool";
>>>>>>                            reg = <0x4 0x94800000 0x0 0x200000>;
>>>>>>                            no-map;
>>>>>>                    };
>>>>>>            };
>>>>>>            soc {
>>>>>>                    compatible = "simple-bus";
>>>>>>                    #address-cells = <2>;
>>>>>>                    #size-cells = <2>;
>>>>>>                    ranges;
>>>>>>                    dma_ranges;
>>>>>>
>>>>>>                    mybus {
>>>>>>                            ranges = <>;
>>>>>>                            dma-ranges = <>;
>>>>>>                            compatible = "simple-bus";
>>>>>>                            #address-cells = <2>;
>>>>>>                            #size-cells = <2>;
>>>>>>                            ranges =     <0x0 0x0  0x0 0x0  0x0 0x80000000>;
>>>>>>                            dma-ranges = <0x0 0x80000000  0x4 0x80000000
>>>>>> 0x0 0x80000000>;
>>>>>>
>>>>>>                            mydevice {
>>>>>>                                    compatible = "my-compatible-string";
>>>>>>                                    memory-region = <&mydev_rsvd>;
>>>>>>                            }
>>>>>>                    }
>>>>>>            }
>>>>>> };
>>>>>>
>>>>>> It looks like this issue was previously fixed by commit c41f9ea998f3
>>>>>> ("drivers: dma-coherent: Account dma_pfn_offset when used with device
>>>>>> tree") which introduced a new function ('dma_get_device_base()') to
>>>>>> return the reserved memory address as seen by the device. However,
>>>>>> such a function, even if still there, is not used anymore in latest
>>>>>> code (as of v5.4-rc2). Was that done for a specific reason? Or is it
>>>>>> just a mistake?
>>>>>
>>>>> Hmm, it looks like 43fc509c3efb ("dma-coherent: introduce interface for default DMA pool") removed the caller of dma_get_device_base() in the alloc path shortly after it was introduced, which certainly appears as if it may have been unintentional - Vladimir?
>>>>
>>>> I do not remember it was intentional. Looking at history, default DMA pool was a response
>>>> to another report. However, now I'm wondering why it was not caught by STM32 - most of that
>>>> work was required to support "dma-ranges" with NOMMU+caches (Cortex-M7).
>>>>
>>>> Alex (or anybody else from ST), maybe you have some input?
>>>
>>> Seem they do not care :)
>>>
>>
>>
>> Concerning, the default dma pool:
>>
>> - It was introduced because dma on cortexM7 couldn't use cache-able memory region. So we configured MPU in bootloader (I think it can be done now by Kernel) a small "no cache" memory region and then declare this same region in kernel device tree for the dma pool. Without that we can't execute any dma trasferts.
>>
>> Sorry for the late answer :(
> 
> Still better then no answer ;)
> 
> Yes, I remember why we have default DMA pool. IIRC, STM used dma-ranges, it is how
> c41f9ea998f3 ("drivers: dma-coherent: Account dma_pfn_offset when used with device
> tree") was discovered. It also means that STM is broken after 878ec36 (ARM: NOMMU:
> Wire-up default DMA interface) with combination of dma-ranges and default DMA pool.
> Hoverer, I've never seen report from STM, so either I'm missing something or such
> combination is not in use nowdays...

IIRC, I didn't added this support by default in device tree because I 
got some troubles with XIP setup (it would be nice that I check the 
status with a latest kernel). SO I don't verify this configuration when 
I execute my no-regression tests for pull STM32 pull request.

Nevertheless, I think I tested it since kernel 4.13 without issue but I 
didn't use "dma-ranges" property.


> 
> Cheers
> Vladimir
> 
>>
>> alex> I'm wondering if I've missed something with diff bellow (it was a long time ago when I touched DMA)?
>>>
>>> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
>>> index db92478..287ef89 100644
>>> --- a/arch/arm/mm/dma-mapping-nommu.c
>>> +++ b/arch/arm/mm/dma-mapping-nommu.c
>>> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>>>                     unsigned long attrs)
>>>      {
>>> -    void *ret = dma_alloc_from_global_coherent(size, dma_handle);
>>> +    void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
>>>          /*
>>>         * dma_alloc_from_global_coherent() may fail because:
>>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>>> index 4a1c4fc..10918c5 100644
>>> --- a/include/linux/dma-mapping.h
>>> +++ b/include/linux/dma-mapping.h
>>> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
>>>    int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
>>>                    void *cpu_addr, size_t size, int *ret);
>>>    -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
>>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
>>>    int dma_release_from_global_coherent(int order, void *vaddr);
>>>    int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>>                      size_t size, int *ret);
>>> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>>    #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
>>>    #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
>>>    -static inline void *dma_alloc_from_global_coherent(ssize_t size,
>>> +static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>>                               dma_addr_t *dma_handle)
>>>    {
>>>        return NULL;
>>> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
>>> index 545e386..551b0eb 100644
>>> --- a/kernel/dma/coherent.c
>>> +++ b/kernel/dma/coherent.c
>>> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>>>        return ret;
>>>    }
>>>    -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>> -        ssize_t size, dma_addr_t *dma_handle)
>>> +static void *__dma_alloc_from_coherent(struct device *dev,
>>> +                       struct dma_coherent_mem *mem,
>>> +                       ssize_t size, dma_addr_t *dma_handle)
>>>    {
>>>        int order = get_order(size);
>>>        unsigned long flags;
>>> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>>        /*
>>>         * Memory was found in the coherent area.
>>>         */
>>> -    *dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
>>> +    *dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
>>>        ret = mem->virt_base + (pageno << PAGE_SHIFT);
>>>        spin_unlock_irqrestore(&mem->spinlock, flags);
>>>        memset(ret, 0, size);
>>> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
>>>        if (!mem)
>>>            return 0;
>>>    -    *ret = __dma_alloc_from_coherent(mem, size, dma_handle);
>>> +    *ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
>>>        return 1;
>>>    }
>>>    -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
>>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>> +                     dma_addr_t *dma_handle)
>>>    {
>>>        if (!dma_coherent_default_memory)
>>>            return NULL;
>>>    -    return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
>>> -            dma_handle);
>>> +    return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
>>> +                     dma_handle);
>>>    }
>>>      static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>> Robin.
>>>>
>>>
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-17  9:46     ` Vladimir Murzin
  2019-10-17 10:03       ` Alexandre Torgue
@ 2019-10-24 12:43       ` Vladimir Murzin
  2019-10-24 15:20         ` Alexandre Torgue
  1 sibling, 1 reply; 15+ messages in thread
From: Vladimir Murzin @ 2019-10-24 12:43 UTC (permalink / raw)
  To: Robin Murphy, Daniele Alessandrelli, Christoph Hellwig, Marek Szyprowski
  Cc: iommu, alexandre.torgue, Arnd Bergmann

On 10/17/19 10:46 AM, Vladimir Murzin wrote:
> I'm wondering if I've missed something with diff bellow (it was a long time ago when I touched DMA)?

Any comments on that? I can only build test it, so lack of testing stopping me from sending it as a
proper patch :(

> 
> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
> index db92478..287ef89 100644
> --- a/arch/arm/mm/dma-mapping-nommu.c
> +++ b/arch/arm/mm/dma-mapping-nommu.c
> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>  				 unsigned long attrs)
>  
>  {
> -	void *ret = dma_alloc_from_global_coherent(size, dma_handle);
> +	void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
>  
>  	/*
>  	 * dma_alloc_from_global_coherent() may fail because:
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 4a1c4fc..10918c5 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
>  int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
>  			    void *cpu_addr, size_t size, int *ret);
>  
> -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
>  int dma_release_from_global_coherent(int order, void *vaddr);
>  int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>  				  size_t size, int *ret);
> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>  #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
>  #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
>  
> -static inline void *dma_alloc_from_global_coherent(ssize_t size,
> +static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>  						   dma_addr_t *dma_handle)
>  {
>  	return NULL;
> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> index 545e386..551b0eb 100644
> --- a/kernel/dma/coherent.c
> +++ b/kernel/dma/coherent.c
> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>  	return ret;
>  }
>  
> -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
> -		ssize_t size, dma_addr_t *dma_handle)
> +static void *__dma_alloc_from_coherent(struct device *dev,
> +				       struct dma_coherent_mem *mem,
> +				       ssize_t size, dma_addr_t *dma_handle)
>  {
>  	int order = get_order(size);
>  	unsigned long flags;
> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>  	/*
>  	 * Memory was found in the coherent area.
>  	 */
> -	*dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
> +	*dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
>  	ret = mem->virt_base + (pageno << PAGE_SHIFT);
>  	spin_unlock_irqrestore(&mem->spinlock, flags);
>  	memset(ret, 0, size);
> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
>  	if (!mem)
>  		return 0;
>  
> -	*ret = __dma_alloc_from_coherent(mem, size, dma_handle);
> +	*ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
>  	return 1;
>  }
>  
> -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
> +				     dma_addr_t *dma_handle)
>  {
>  	if (!dma_coherent_default_memory)
>  		return NULL;
>  
> -	return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
> -			dma_handle);
> +	return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
> +					 dma_handle);
>  }
>  
>  static int __dma_release_from_coherent(struct dma_coherent_mem *mem,


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-24 12:43       ` Vladimir Murzin
@ 2019-10-24 15:20         ` Alexandre Torgue
  2019-10-24 15:27           ` Vladimir Murzin
  0 siblings, 1 reply; 15+ messages in thread
From: Alexandre Torgue @ 2019-10-24 15:20 UTC (permalink / raw)
  To: Vladimir Murzin, Robin Murphy, Daniele Alessandrelli,
	Christoph Hellwig, Marek Szyprowski
  Cc: iommu, Arnd Bergmann

Hi Vlad,

On 10/24/19 2:43 PM, Vladimir Murzin wrote:
> On 10/17/19 10:46 AM, Vladimir Murzin wrote:
>> I'm wondering if I've missed something with diff bellow (it was a long time ago when I touched DMA)?
> 
> Any comments on that? I can only build test it, so lack of testing stopping me from sending it as a
> proper patch :(

I can make some tests tomorrow. Which particular setup I need to test: 
cortex M7 + cache + dma + xip ? Let me know.

regards
alex

> 
>>
>> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
>> index db92478..287ef89 100644
>> --- a/arch/arm/mm/dma-mapping-nommu.c
>> +++ b/arch/arm/mm/dma-mapping-nommu.c
>> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>>   				 unsigned long attrs)
>>   
>>   {
>> -	void *ret = dma_alloc_from_global_coherent(size, dma_handle);
>> +	void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
>>   
>>   	/*
>>   	 * dma_alloc_from_global_coherent() may fail because:
>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>> index 4a1c4fc..10918c5 100644
>> --- a/include/linux/dma-mapping.h
>> +++ b/include/linux/dma-mapping.h
>> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
>>   int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
>>   			    void *cpu_addr, size_t size, int *ret);
>>   
>> -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
>>   int dma_release_from_global_coherent(int order, void *vaddr);
>>   int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>   				  size_t size, int *ret);
>> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>   #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
>>   #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
>>   
>> -static inline void *dma_alloc_from_global_coherent(ssize_t size,
>> +static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>   						   dma_addr_t *dma_handle)
>>   {
>>   	return NULL;
>> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
>> index 545e386..551b0eb 100644
>> --- a/kernel/dma/coherent.c
>> +++ b/kernel/dma/coherent.c
>> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>>   	return ret;
>>   }
>>   
>> -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>> -		ssize_t size, dma_addr_t *dma_handle)
>> +static void *__dma_alloc_from_coherent(struct device *dev,
>> +				       struct dma_coherent_mem *mem,
>> +				       ssize_t size, dma_addr_t *dma_handle)
>>   {
>>   	int order = get_order(size);
>>   	unsigned long flags;
>> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>   	/*
>>   	 * Memory was found in the coherent area.
>>   	 */
>> -	*dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
>> +	*dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
>>   	ret = mem->virt_base + (pageno << PAGE_SHIFT);
>>   	spin_unlock_irqrestore(&mem->spinlock, flags);
>>   	memset(ret, 0, size);
>> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
>>   	if (!mem)
>>   		return 0;
>>   
>> -	*ret = __dma_alloc_from_coherent(mem, size, dma_handle);
>> +	*ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
>>   	return 1;
>>   }
>>   
>> -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>> +				     dma_addr_t *dma_handle)
>>   {
>>   	if (!dma_coherent_default_memory)
>>   		return NULL;
>>   
>> -	return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
>> -			dma_handle);
>> +	return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
>> +					 dma_handle);
>>   }
>>   
>>   static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
> 
> 
> Thanks
> Vladimir
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-24 15:20         ` Alexandre Torgue
@ 2019-10-24 15:27           ` Vladimir Murzin
  2019-10-28 10:55             ` Alexandre Torgue
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Murzin @ 2019-10-24 15:27 UTC (permalink / raw)
  To: Alexandre Torgue, Robin Murphy, Daniele Alessandrelli,
	Christoph Hellwig, Marek Szyprowski
  Cc: iommu, Arnd Bergmann

Hi Alex,

On 10/24/19 4:20 PM, Alexandre Torgue wrote:
> Hi Vlad,
> 
> On 10/24/19 2:43 PM, Vladimir Murzin wrote:
>> On 10/17/19 10:46 AM, Vladimir Murzin wrote:
>>> I'm wondering if I've missed something with diff bellow (it was a long time ago when I touched DMA)?
>>
>> Any comments on that? I can only build test it, so lack of testing stopping me from sending it as a
>> proper patch :(
> 
> I can make some tests tomorrow. Which particular setup I need to test: cortex M7 + cache + dma + xip ? Let me know.

I assume xip implies dma-ranges in dt, then yes it looks like what we need.

Great thanks!

Vladimir

> 
> regards
> alex
> 
>>
>>>
>>> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
>>> index db92478..287ef89 100644
>>> --- a/arch/arm/mm/dma-mapping-nommu.c
>>> +++ b/arch/arm/mm/dma-mapping-nommu.c
>>> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>>>                    unsigned long attrs)
>>>     {
>>> -    void *ret = dma_alloc_from_global_coherent(size, dma_handle);
>>> +    void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
>>>         /*
>>>        * dma_alloc_from_global_coherent() may fail because:
>>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>>> index 4a1c4fc..10918c5 100644
>>> --- a/include/linux/dma-mapping.h
>>> +++ b/include/linux/dma-mapping.h
>>> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
>>>   int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
>>>                   void *cpu_addr, size_t size, int *ret);
>>>   -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
>>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
>>>   int dma_release_from_global_coherent(int order, void *vaddr);
>>>   int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>>                     size_t size, int *ret);
>>> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>>   #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
>>>   #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
>>>   -static inline void *dma_alloc_from_global_coherent(ssize_t size,
>>> +static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>>                              dma_addr_t *dma_handle)
>>>   {
>>>       return NULL;
>>> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
>>> index 545e386..551b0eb 100644
>>> --- a/kernel/dma/coherent.c
>>> +++ b/kernel/dma/coherent.c
>>> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>>>       return ret;
>>>   }
>>>   -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>> -        ssize_t size, dma_addr_t *dma_handle)
>>> +static void *__dma_alloc_from_coherent(struct device *dev,
>>> +                       struct dma_coherent_mem *mem,
>>> +                       ssize_t size, dma_addr_t *dma_handle)
>>>   {
>>>       int order = get_order(size);
>>>       unsigned long flags;
>>> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>>       /*
>>>        * Memory was found in the coherent area.
>>>        */
>>> -    *dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
>>> +    *dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
>>>       ret = mem->virt_base + (pageno << PAGE_SHIFT);
>>>       spin_unlock_irqrestore(&mem->spinlock, flags);
>>>       memset(ret, 0, size);
>>> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
>>>       if (!mem)
>>>           return 0;
>>>   -    *ret = __dma_alloc_from_coherent(mem, size, dma_handle);
>>> +    *ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
>>>       return 1;
>>>   }
>>>   -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
>>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>> +                     dma_addr_t *dma_handle)
>>>   {
>>>       if (!dma_coherent_default_memory)
>>>           return NULL;
>>>   -    return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
>>> -            dma_handle);
>>> +    return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
>>> +                     dma_handle);
>>>   }
>>>     static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
>>
>>
>> Thanks
>> Vladimir
>>

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-24 15:27           ` Vladimir Murzin
@ 2019-10-28 10:55             ` Alexandre Torgue
  2019-10-28 10:59               ` Vladimir Murzin
  0 siblings, 1 reply; 15+ messages in thread
From: Alexandre Torgue @ 2019-10-28 10:55 UTC (permalink / raw)
  To: Vladimir Murzin, Robin Murphy, Daniele Alessandrelli,
	Christoph Hellwig, Marek Szyprowski
  Cc: iommu, Arnd Bergmann

Hi Vlad,

On 10/24/19 5:27 PM, Vladimir Murzin wrote:
> Hi Alex,
> 
> On 10/24/19 4:20 PM, Alexandre Torgue wrote:
>> Hi Vlad,
>>
>> On 10/24/19 2:43 PM, Vladimir Murzin wrote:
>>> On 10/17/19 10:46 AM, Vladimir Murzin wrote:
>>>> I'm wondering if I've missed something with diff bellow (it was a long time ago when I touched DMA)?
>>>
>>> Any comments on that? I can only build test it, so lack of testing stopping me from sending it as a
>>> proper patch :(
>>
>> I can make some tests tomorrow. Which particular setup I need to test: cortex M7 + cache + dma + xip ? Let me know.
> 
> I assume xip implies dma-ranges in dt, then yes it looks like what we need.
> 

I tested your patch on stm32h7431-eval board (cortex-M7). No issues 
reported. I use reserved dma memory pool for dma access ( Dcache 
disabled) and dma_alloc_coherent is ok. Note there is no "dma-ranges" 
for this board.

I also tested it on stm32f469-disco (cortex-M4) which uses dma-ranges. 
No issues reported too. Note there is no data cache at all in this case.

regards

Alex

> Great thanks!
> 
> Vladimir
> 
>>
>> regards
>> alex
>>
>>>
>>>>
>>>> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
>>>> index db92478..287ef89 100644
>>>> --- a/arch/arm/mm/dma-mapping-nommu.c
>>>> +++ b/arch/arm/mm/dma-mapping-nommu.c
>>>> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>>>>                     unsigned long attrs)
>>>>      {
>>>> -    void *ret = dma_alloc_from_global_coherent(size, dma_handle);
>>>> +    void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
>>>>          /*
>>>>         * dma_alloc_from_global_coherent() may fail because:
>>>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>>>> index 4a1c4fc..10918c5 100644
>>>> --- a/include/linux/dma-mapping.h
>>>> +++ b/include/linux/dma-mapping.h
>>>> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
>>>>    int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
>>>>                    void *cpu_addr, size_t size, int *ret);
>>>>    -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
>>>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
>>>>    int dma_release_from_global_coherent(int order, void *vaddr);
>>>>    int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>>>                      size_t size, int *ret);
>>>> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>>>    #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
>>>>    #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
>>>>    -static inline void *dma_alloc_from_global_coherent(ssize_t size,
>>>> +static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>>>                               dma_addr_t *dma_handle)
>>>>    {
>>>>        return NULL;
>>>> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
>>>> index 545e386..551b0eb 100644
>>>> --- a/kernel/dma/coherent.c
>>>> +++ b/kernel/dma/coherent.c
>>>> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>>>>        return ret;
>>>>    }
>>>>    -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>>> -        ssize_t size, dma_addr_t *dma_handle)
>>>> +static void *__dma_alloc_from_coherent(struct device *dev,
>>>> +                       struct dma_coherent_mem *mem,
>>>> +                       ssize_t size, dma_addr_t *dma_handle)
>>>>    {
>>>>        int order = get_order(size);
>>>>        unsigned long flags;
>>>> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>>>        /*
>>>>         * Memory was found in the coherent area.
>>>>         */
>>>> -    *dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
>>>> +    *dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
>>>>        ret = mem->virt_base + (pageno << PAGE_SHIFT);
>>>>        spin_unlock_irqrestore(&mem->spinlock, flags);
>>>>        memset(ret, 0, size);
>>>> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
>>>>        if (!mem)
>>>>            return 0;
>>>>    -    *ret = __dma_alloc_from_coherent(mem, size, dma_handle);
>>>> +    *ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
>>>>        return 1;
>>>>    }
>>>>    -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
>>>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>>> +                     dma_addr_t *dma_handle)
>>>>    {
>>>>        if (!dma_coherent_default_memory)
>>>>            return NULL;
>>>>    -    return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
>>>> -            dma_handle);
>>>> +    return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
>>>> +                     dma_handle);
>>>>    }
>>>>      static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
>>>
>>>
>>> Thanks
>>> Vladimir
>>>
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-28 10:55             ` Alexandre Torgue
@ 2019-10-28 10:59               ` Vladimir Murzin
  2019-10-29  9:43                 ` Daniele Alessandrelli
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Murzin @ 2019-10-28 10:59 UTC (permalink / raw)
  To: Alexandre Torgue, Robin Murphy, Daniele Alessandrelli,
	Christoph Hellwig, Marek Szyprowski
  Cc: iommu, Arnd Bergmann

On 10/28/19 10:55 AM, Alexandre Torgue wrote:
> Hi Vlad,
> 
> On 10/24/19 5:27 PM, Vladimir Murzin wrote:
>> Hi Alex,
>>
>> On 10/24/19 4:20 PM, Alexandre Torgue wrote:
>>> Hi Vlad,
>>>
>>> On 10/24/19 2:43 PM, Vladimir Murzin wrote:
>>>> On 10/17/19 10:46 AM, Vladimir Murzin wrote:
>>>>> I'm wondering if I've missed something with diff bellow (it was a long time ago when I touched DMA)?
>>>>
>>>> Any comments on that? I can only build test it, so lack of testing stopping me from sending it as a
>>>> proper patch :(
>>>
>>> I can make some tests tomorrow. Which particular setup I need to test: cortex M7 + cache + dma + xip ? Let me know.
>>
>> I assume xip implies dma-ranges in dt, then yes it looks like what we need.
>>
> 
> I tested your patch on stm32h7431-eval board (cortex-M7). No issues reported. I use reserved dma memory pool for dma access ( Dcache disabled) and dma_alloc_coherent is ok. Note there is no "dma-ranges" for this board.
> 
> I also tested it on stm32f469-disco (cortex-M4) which uses dma-ranges. No issues reported too. Note there is no data cache at all in this case.

Happy to hear it doesn't introduce regression for NOMM - thanks got giving it a try!

@Daniele, it'd be handy to know if that fix issue for you...

Cheers
Vladimir

> 
> regards
> 
> Alex
> 
>> Great thanks!
>>
>> Vladimir
>>
>>>
>>> regards
>>> alex
>>>
>>>>
>>>>>
>>>>> diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
>>>>> index db92478..287ef89 100644
>>>>> --- a/arch/arm/mm/dma-mapping-nommu.c
>>>>> +++ b/arch/arm/mm/dma-mapping-nommu.c
>>>>> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
>>>>>                     unsigned long attrs)
>>>>>      {
>>>>> -    void *ret = dma_alloc_from_global_coherent(size, dma_handle);
>>>>> +    void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
>>>>>          /*
>>>>>         * dma_alloc_from_global_coherent() may fail because:
>>>>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>>>>> index 4a1c4fc..10918c5 100644
>>>>> --- a/include/linux/dma-mapping.h
>>>>> +++ b/include/linux/dma-mapping.h
>>>>> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
>>>>>    int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
>>>>>                    void *cpu_addr, size_t size, int *ret);
>>>>>    -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
>>>>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size, dma_addr_t *dma_handle);
>>>>>    int dma_release_from_global_coherent(int order, void *vaddr);
>>>>>    int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>>>>                      size_t size, int *ret);
>>>>> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
>>>>>    #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
>>>>>    #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
>>>>>    -static inline void *dma_alloc_from_global_coherent(ssize_t size,
>>>>> +static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>>>>                               dma_addr_t *dma_handle)
>>>>>    {
>>>>>        return NULL;
>>>>> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
>>>>> index 545e386..551b0eb 100644
>>>>> --- a/kernel/dma/coherent.c
>>>>> +++ b/kernel/dma/coherent.c
>>>>> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
>>>>>        return ret;
>>>>>    }
>>>>>    -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>>>> -        ssize_t size, dma_addr_t *dma_handle)
>>>>> +static void *__dma_alloc_from_coherent(struct device *dev,
>>>>> +                       struct dma_coherent_mem *mem,
>>>>> +                       ssize_t size, dma_addr_t *dma_handle)
>>>>>    {
>>>>>        int order = get_order(size);
>>>>>        unsigned long flags;
>>>>> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
>>>>>        /*
>>>>>         * Memory was found in the coherent area.
>>>>>         */
>>>>> -    *dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
>>>>> +    *dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
>>>>>        ret = mem->virt_base + (pageno << PAGE_SHIFT);
>>>>>        spin_unlock_irqrestore(&mem->spinlock, flags);
>>>>>        memset(ret, 0, size);
>>>>> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
>>>>>        if (!mem)
>>>>>            return 0;
>>>>>    -    *ret = __dma_alloc_from_coherent(mem, size, dma_handle);
>>>>> +    *ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
>>>>>        return 1;
>>>>>    }
>>>>>    -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
>>>>> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
>>>>> +                     dma_addr_t *dma_handle)
>>>>>    {
>>>>>        if (!dma_coherent_default_memory)
>>>>>            return NULL;
>>>>>    -    return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
>>>>> -            dma_handle);
>>>>> +    return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
>>>>> +                     dma_handle);
>>>>>    }
>>>>>      static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
>>>>
>>>>
>>>> Thanks
>>>> Vladimir
>>>>
>>

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-28 10:59               ` Vladimir Murzin
@ 2019-10-29  9:43                 ` Daniele Alessandrelli
  2019-10-29 16:52                   ` Daniele Alessandrelli
  0 siblings, 1 reply; 15+ messages in thread
From: Daniele Alessandrelli @ 2019-10-29  9:43 UTC (permalink / raw)
  To: Vladimir Murzin
  Cc: Alexandre Torgue, Arnd Bergmann, iommu, Robin Murphy, Christoph Hellwig

On Mon, Oct 28, 2019 at 10:59 AM Vladimir Murzin
<vladimir.murzin@arm.com> wrote:
>
> @Daniele, it'd be handy to know if that fix issue for you...
>

Apologies, I've been traveling for the last few days and haven't
managed to try it yet.

I'll do it later today though and let you know.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-29  9:43                 ` Daniele Alessandrelli
@ 2019-10-29 16:52                   ` Daniele Alessandrelli
  2019-10-30 10:16                     ` Vladimir Murzin
  0 siblings, 1 reply; 15+ messages in thread
From: Daniele Alessandrelli @ 2019-10-29 16:52 UTC (permalink / raw)
  To: Vladimir Murzin
  Cc: Alexandre Torgue, Arnd Bergmann, iommu, Robin Murphy, Christoph Hellwig

On Tue, Oct 29, 2019 at 9:43 AM Daniele Alessandrelli
<daniele.alessandrelli@gmail.com> wrote:
>
> On Mon, Oct 28, 2019 at 10:59 AM Vladimir Murzin
> <vladimir.murzin@arm.com> wrote:
> >
> > @Daniele, it'd be handy to know if that fix issue for you...
> >
>
> Apologies, I've been traveling for the last few days and haven't
> managed to try it yet.
>
> I'll do it later today though and let you know.

Hi Vladimir,

I can confirm that your patch fixes the issue I was having. Thanks!

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
  2019-10-29 16:52                   ` Daniele Alessandrelli
@ 2019-10-30 10:16                     ` Vladimir Murzin
  0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Murzin @ 2019-10-30 10:16 UTC (permalink / raw)
  To: Daniele Alessandrelli
  Cc: Alexandre Torgue, Arnd Bergmann, iommu, Robin Murphy, Christoph Hellwig

On 10/29/19 4:52 PM, Daniele Alessandrelli wrote:
> On Tue, Oct 29, 2019 at 9:43 AM Daniele Alessandrelli
> <daniele.alessandrelli@gmail.com> wrote:
>>
>> On Mon, Oct 28, 2019 at 10:59 AM Vladimir Murzin
>> <vladimir.murzin@arm.com> wrote:
>>>
>>> @Daniele, it'd be handy to know if that fix issue for you...
>>>
>>
>> Apologies, I've been traveling for the last few days and haven't
>> managed to try it yet.
>>
>> I'll do it later today though and let you know.
> 
> Hi Vladimir,
> 
> I can confirm that your patch fixes the issue I was having. Thanks!

Great thanks!

Christoph, I've just sent it as a proper patch, please consider to apply.

Cheers
Vladimir

> 
> Regards,
> Daniele
> 

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-10-30 10:16 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-13 14:28 [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug? Daniele Alessandrelli
2019-10-14 13:54 ` Robin Murphy
2019-10-14 15:01   ` Vladimir Murzin
2019-10-17  9:46     ` Vladimir Murzin
2019-10-17 10:03       ` Alexandre Torgue
2019-10-17 10:16         ` Vladimir Murzin
2019-10-17 10:33           ` Alexandre Torgue
2019-10-24 12:43       ` Vladimir Murzin
2019-10-24 15:20         ` Alexandre Torgue
2019-10-24 15:27           ` Vladimir Murzin
2019-10-28 10:55             ` Alexandre Torgue
2019-10-28 10:59               ` Vladimir Murzin
2019-10-29  9:43                 ` Daniele Alessandrelli
2019-10-29 16:52                   ` Daniele Alessandrelli
2019-10-30 10:16                     ` Vladimir Murzin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).