dma-coherent: add support for multi coherent rmems per dev

Add support for multiple coherent rmems per device. This patch addes dma_mem_list to device structure to store multiple rmems.

These multiple rmems can be assigned to the device one by one by of_reserved_mem_device_init_by_idx() with the memory-region declaration in device tree as below and store the rmem to the dma_mem_list.

	device1@0 {
		...
		memory-region = <&reserved_mem0>, <&reserved_mem1>;
		...
	};

When driver tries to allocate memory from the rmems, looks for the first available rmem and allocates the memory from this rmem.

Then if driver removed, of_reserved_mem_device_release() needs to be invoked to release all the rmems assigned to the device.

Signed-off-by Howard Yen <[email protected]>

---
  include/linux/device.h |  1 +
  kernel/dma/coherent.c  | 66 +++++++++++++++++++++++++++++++++++-------
  2 files changed, 56 insertions(+), 11 deletions(-)

include/linux/device.h +0 -0

index 97c4b046c09d..c8682ee507cf 100644

include/linux/device.h +0 -0

  #endif
  #ifdef CONFIG_DMA_CMA
  	struct cma *cma_area;		/* contiguous memory area for dma

kernel/dma/coherent.c +55 -11

index ff5683a57f77..f31befd2e6f8 100644
@@ -18,6 +18,7 @@ struct dma_coherent_mem {
  	unsigned long	*bitmap;
  	spinlock_t	spinlock;
  	bool		use_dev_dma_pfn_offset;
+	struct list_head	node;
  };
 
  static inline struct dma_coherent_mem *dev_get_coherent_memory(struct device *dev)

@@ -61,6 +62,7 @@ static struct dma_coherent_mem *dma_init_coherent_memory(phys_addr_t phys_addr,
  	dma_mem->pfn_base = PFN_DOWN(phys_addr);
  	dma_mem->size = pages;
  	dma_mem->use_dev_dma_pfn_offset = use_dma_pfn_offset;
+	INIT_LIST_HEAD(&dma_mem->node);
  	spin_lock_init(&dma_mem->spinlock);
 
  	return dma_mem;

@@ -90,10 +92,13 @@ static int dma_assign_coherent_memory(struct device *dev,
  	if (!dev)
  		return -ENODEV;
 
-	if (dev->dma_mem)
-		return -EBUSY;
+	if (!dev->dma_mem) {
+		dev->dma_mem = mem;
+		INIT_LIST_HEAD(&dev->dma_mem_list);
+	}
+
+	list_add_tail(&mem->node, &dev->dma_mem_list);
 
-	dev->dma_mem = mem;
  	return 0;
  }
 

@@ -187,12 +192,17 @@ static void *__dma_alloc_from_coherent(struct device *dev,
  int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
  		dma_addr_t *dma_handle, void **ret)
  {
-	struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
+	struct dma_coherent_mem *mem_tmp, *mem = dev_get_coherent_memory(dev);
 
  	if (!mem)
  		return 0;
 
-	*ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
+	list_for_each_entry(mem_tmp, &dev->dma_mem_list, node) {
+		*ret = __dma_alloc_from_coherent(dev, mem_tmp, size, dma_handle);
+		if (*ret)
+			break;
+	}
+
  	return 1;
  }
 

@@ -226,9 +236,19 @@ static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
   */
  int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr)
  {
-	struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
+	struct dma_coherent_mem *mem_tmp, *mem = dev_get_coherent_memory(dev);
+	int ret = 0;
 
-	return __dma_release_from_coherent(mem, order, vaddr);
+	if (!mem)
+		return ret;
+
+	list_for_each_entry(mem_tmp, &dev->dma_mem_list, node) {
+		ret = __dma_release_from_coherent(mem_tmp, order, vaddr);
+		if (ret == 1)
+			break;
+	}
+
+	return ret;
  }
 
  static int __dma_mmap_from_coherent(struct dma_coherent_mem *mem,

@@ -271,9 +291,19 @@ static int __dma_mmap_from_coherent(struct dma_coherent_mem *mem,
  int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
  			   void *vaddr, size_t size, int *ret)
  {
-	struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
+	struct dma_coherent_mem *mem_tmp, *mem = dev_get_coherent_memory(dev);
+	int retval = 0;
 
-	return __dma_mmap_from_coherent(mem, vma, vaddr, size, ret);
+	if (!mem)
+		return retval;
+
+	list_for_each_entry(mem_tmp, &dev->dma_mem_list, node) {
+		retval = __dma_mmap_from_coherent(mem_tmp, vma, vaddr, size, ret);
+		if (retval == 1)
+			break;
+	}
+
+	return retval;
  }
 
  #ifdef CONFIG_DMA_GLOBAL_POOL

@@ -351,8 +381,22 @@ static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
  static void rmem_dma_device_release(struct reserved_mem *rmem,
  				    struct device *dev)
  {
-	if (dev)
-		dev->dma_mem = NULL;
+	struct dma_coherent_mem *mem, *mem_tmp, *q;
+
+	if (dev) {
+		mem = dev_get_coherent_memory(dev);
+		if (rmem->priv != mem) {
+			list_for_each_entry_safe(mem_tmp, q, &dev->dma_mem_list, node) {
+				if (mem_tmp == rmem->priv) {
+					list_del_init(&mem_tmp->node);
+					break;
+				}
+			}
+		} else {
+			list_del_init(&mem->node);
+			dev->dma_mem = NULL;
+		}
+	}
  }
 
  static const struct reserved_mem_ops rmem_dma_ops = {

On 2024-02-01 9:35 am, Howard Yen wrote:

include/linux/device.h struct device { · line 754

Quoted 3 lines

struct dma_coherent_mem *dma_mem; /* internal for coherent mem
override */

include/linux/device.h +1 -0

+	struct list_head	dma_mem_list;

I'm not necessarily against the idea, but only if it's implemented sensibly. If we're going to have a list of these it should *replace* the existing pointer, not do this weird thing with both.

Quoted 25 lines · Howard Yen

Add support for multiple coherent rmems per device. This patch addes dma_mem_list to device structure to store multiple rmems.

These multiple rmems can be assigned to the device one by one by of_reserved_mem_device_init_by_idx() with the memory-region declaration in device tree as below and store the rmem to the dma_mem_list.

	device1@0 {
		...
		memory-region = <&reserved_mem0>, <&reserved_mem1>;
		...
	};

When driver tries to allocate memory from the rmems, looks for the first available rmem and allocates the memory from this rmem.

Then if driver removed, of_reserved_mem_device_release() needs to be invoked to release all the rmems assigned to the device.

Signed-off-by Howard Yen <[email protected]>

---
  include/linux/device.h |  1 +
  kernel/dma/coherent.c  | 66 +++++++++++++++++++++++++++++++++++-------
  2 files changed, 56 insertions(+), 11 deletions(-)

include/linux/device.h struct device { · line 754

Quoted 9 lines

include/linux/device.h +1 -0

index 97c4b046c09d..c8682ee507cf 100644
@@ -751,6 +751,7 @@ struct device {
  #ifdef CONFIG_DMA_DECLARE_COHERENT
  	struct dma_coherent_mem	*dma_mem; /* internal for coherent mem
  					     override */
+	struct list_head	dma_mem_list;

I'm not necessarily against the idea, but only if it's implemented sensibly. If we're going to have a list of these it should *replace* the existing pointer, not do this weird thing with both.

Quoted 26 lines · Robin Murphy

Quoted 25 lines · Howard Yen

Add support for multiple coherent rmems per device. This patch addes dma_mem_list to device structure to store multiple rmems.

These multiple rmems can be assigned to the device one by one by of_reserved_mem_device_init_by_idx() with the memory-region declaration in device tree as below and store the rmem to the dma_mem_list.

    device1@0 {
            ...
            memory-region = <&reserved_mem0>, <&reserved_mem1>;
            ...
    };

When driver tries to allocate memory from the rmems, looks for the first available rmem and allocates the memory from this rmem.

Then if driver removed, of_reserved_mem_device_release() needs to be invoked to release all the rmems assigned to the device.

Signed-off-by Howard Yen <[email protected]>

---
  include/linux/device.h |  1 +
  kernel/dma/coherent.c  | 66 +++++++++++++++++++++++++++++++++++-------
  2 files changed, 56 insertions(+), 11 deletions(-)

Quoted 13 lines

include/linux/device.h struct device { · line 754

Quoted 9 lines

include/linux/device.h +1 -0

index 97c4b046c09d..c8682ee507cf 100644
@@ -751,6 +751,7 @@ struct device {
  #ifdef CONFIG_DMA_DECLARE_COHERENT
    struct dma_coherent_mem *dma_mem; /* internal for coherent mem
                                         override */
+   struct list_head        dma_mem_list;

I'm not necessarily against the idea, but only if it's implemented sensibly. If we're going to have a list of these it should *replace* the existing pointer, not do this weird thing with both.

Agreed, it should be one pointer max for this structure for this type of thing. Why not move it into the dma_coherent_mem structure?

thanks,

greg k-h

Quoted 2 lines · Greg KH

Quoted 1 line · Robin Murphy

On 2024-02-01 9:35 am, Howard Yen wrote:

I'm considering to modify the change to

1. Move it into the dma_coherent_mem structure, like

HEAD
mem0->node

This case, if I check list_empty(mem0->node), it would give me the list is empty, but actually there is one rmem.

2. Replace the pointer to a list_head.

HEAD
dma_mems ---> mem0->node

This case, if I check list_empty(dma_mems), it would give me the list is non-empty, it matches the actual status.

So, the 2nd looks reasonable, I'm going to upload a v2 patch with the 2nd approach, does that make sense?