[PATCH 0/3] staging: vchiq_core: Consolidate bulk xfer helper

[PATCH 0/3] staging: vchiq_core: Consolidate bulk xfer helper

[Umang Jain]

Few functions were identified were their declaration, definition
and usage were spread (incorrectly) across vchiq_core and vchiq_arm.

These are being consolidated into vchiq_core in this series.
Please look at individual patches for details.

2/3 is in particular a big one. The code is just a move, but
seems big - as it was diffcult to split it into smaller, compilable
hunks of individual patches.

Umang Jain (3):
  staging: vchiq_core: Move remote_event_signal() vchiq_core
  staging: vchiq_core: Move bulk data functions in vchiq_core
  staging: vchiq_core: Drop vchiq_pagelist.h

 .../interface/vchiq_arm/vchiq_arm.c           | 356 ------------------
 .../interface/vchiq_arm/vchiq_core.c          | 350 +++++++++++++++++
 .../interface/vchiq_arm/vchiq_core.h          |  30 +-
 .../interface/vchiq_arm/vchiq_pagelist.h      |  21 --
 4 files changed, 373 insertions(+), 384 deletions(-)
 delete mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h

-- 
2.45.2

[PATCH 1/3] staging: vchiq_core: Move remote_event_signal() vchiq_core

[Umang Jain]

The function remote_event_signal() is declared in vchiq_core.h while
defined in vchiq_arm.c and used only in vchiq_core.c. Move the
definition to vchiq_core.c as it is only used in this file.

Also convert it to static and drop the function signature from
vchiq_core.h header. BELL2 doorbell macro is also moved from vchiq_arm
to vchiq_core as part of this change.

No functional changes intended in this patch.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 19 ----------------
 .../interface/vchiq_arm/vchiq_core.c          | 22 +++++++++++++++++++
 .../interface/vchiq_arm/vchiq_core.h          |  2 --
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index c44b4dc79c03..8e9a5c67a6de 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -48,7 +48,6 @@
 #define VCHIQ_PLATFORM_FRAGMENTS_COUNT_IDX  1
 
 #define BELL0	0x00
-#define BELL2	0x08
 
 #define ARM_DS_ACTIVE	BIT(2)
 
@@ -616,24 +615,6 @@ static struct vchiq_arm_state *vchiq_platform_get_arm_state(struct vchiq_state *
 	return (struct vchiq_arm_state *)state->platform_state;
 }
 
-void
-remote_event_signal(struct vchiq_state *state, struct remote_event *event)
-{
-	struct vchiq_drv_mgmt *mgmt = dev_get_drvdata(state->dev);
-
-	/*
-	 * Ensure that all writes to shared data structures have completed
-	 * before signalling the peer.
-	 */
-	wmb();
-
-	event->fired = 1;
-
-	dsb(sy);         /* data barrier operation */
-
-	if (event->armed)
-		writel(0, mgmt->regs + BELL2); /* trigger vc interrupt */
-}
 
 int
 vchiq_prepare_bulk_data(struct vchiq_instance *instance, struct vchiq_bulk *bulk, void *offset,
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 7ad43a3d1bab..88d510c6793a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -5,6 +5,7 @@
 #include <linux/completion.h>
 #include <linux/mutex.h>
 #include <linux/bitops.h>
+#include <linux/io.h>
 #include <linux/kthread.h>
 #include <linux/wait.h>
 #include <linux/delay.h>
@@ -61,6 +62,8 @@
 #define MAKE_REMOTE_USE			(VCHIQ_MSG_REMOTE_USE << TYPE_SHIFT)
 #define MAKE_REMOTE_USE_ACTIVE		(VCHIQ_MSG_REMOTE_USE_ACTIVE << TYPE_SHIFT)
 
+#define BELL2	0x08
+
 /* Ensure the fields are wide enough */
 static_assert(VCHIQ_MSG_SRCPORT(VCHIQ_MAKE_MSG(0, 0, VCHIQ_PORT_MAX))
 	== 0);
@@ -526,6 +529,25 @@ remote_event_wait(wait_queue_head_t *wq, struct remote_event *event)
 	return ret;
 }
 
+static void
+remote_event_signal(struct vchiq_state *state, struct remote_event *event)
+{
+	struct vchiq_drv_mgmt *mgmt = dev_get_drvdata(state->dev);
+
+	/*
+	 * Ensure that all writes to shared data structures have completed
+	 * before signalling the peer.
+	 */
+	wmb();
+
+	event->fired = 1;
+
+	dsb(sy);         /* data barrier operation */
+
+	if (event->armed)
+		writel(0, mgmt->regs + BELL2); /* trigger vc interrupt */
+}
+
 /*
  * Acknowledge that the event has been signalled, and wake any waiters. Usually
  * called as a result of the doorbell being rung.
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 5bf543dfc9c7..32b0521aa036 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -534,8 +534,6 @@ int vchiq_prepare_bulk_data(struct vchiq_instance *instance, struct vchiq_bulk *
 
 void vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bulk);
 
-void remote_event_signal(struct vchiq_state *state, struct remote_event *event);
-
 void vchiq_dump_platform_state(struct seq_file *f);
 
 void vchiq_dump_platform_instances(struct vchiq_state *state, struct seq_file *f);
-- 
2.45.2

[PATCH 2/3] staging: vchiq_core: Move bulk data functions in vchiq_core

[Umang Jain]

Bulk transfers core logic lives in vchiq_core.c, hence move all
the preparatory bulk data allocation helpers to vchiq_core.c (from
vchiq_arm).

The discrepancy was noticed when vchiq_prepare_bulk_data() and
vchiq_complete_bulk() are being used vchiq_core.c but are defined
in vchiq_arm. Now that they are now confined to vchiq_core.c,
they can be made static and their signatures from vchiq_core header
can be dropped.

vchiq_prepare_bulk_data() and vchiq_complete_bulk() depends on
struct vchiq_pagelist_info, cleanup_pagelist(), free_pagelist() and
create_pagelist() hence they are pulled in from vchiq_arm as well,
as part of this commit.

No functional changes intended in this patch.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 337 ------------------
 .../interface/vchiq_arm/vchiq_core.c          | 324 +++++++++++++++++
 .../interface/vchiq_arm/vchiq_core.h          |  19 +-
 3 files changed, 338 insertions(+), 342 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 8e9a5c67a6de..cc0b3f2506a4 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -14,7 +14,6 @@
 #include <linux/device.h>
 #include <linux/device/bus.h>
 #include <linux/mm.h>
-#include <linux/highmem.h>
 #include <linux/pagemap.h>
 #include <linux/bug.h>
 #include <linux/completion.h>
@@ -36,7 +35,6 @@
 #include "vchiq_arm.h"
 #include "vchiq_bus.h"
 #include "vchiq_debugfs.h"
-#include "vchiq_pagelist.h"
 
 #define DEVICE_NAME "vchiq"
 
@@ -108,17 +106,6 @@ struct vchiq_arm_state {
 	int first_connect;
 };
 
-struct vchiq_pagelist_info {
-	struct pagelist *pagelist;
-	size_t pagelist_buffer_size;
-	dma_addr_t dma_addr;
-	enum dma_data_direction dma_dir;
-	unsigned int num_pages;
-	unsigned int pages_need_release;
-	struct page **pages;
-	struct scatterlist *scatterlist;
-	unsigned int scatterlist_mapped;
-};
 
 static int
 vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *data,
@@ -145,35 +132,6 @@ vchiq_doorbell_irq(int irq, void *dev_id)
 	return ret;
 }
 
-static void
-cleanup_pagelistinfo(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagelistinfo)
-{
-	if (pagelistinfo->scatterlist_mapped) {
-		dma_unmap_sg(instance->state->dev, pagelistinfo->scatterlist,
-			     pagelistinfo->num_pages, pagelistinfo->dma_dir);
-	}
-
-	if (pagelistinfo->pages_need_release)
-		unpin_user_pages(pagelistinfo->pages, pagelistinfo->num_pages);
-
-	dma_free_coherent(instance->state->dev, pagelistinfo->pagelist_buffer_size,
-			  pagelistinfo->pagelist, pagelistinfo->dma_addr);
-}
-
-static inline bool
-is_adjacent_block(u32 *addrs, dma_addr_t addr, unsigned int k)
-{
-	u32 tmp;
-
-	if (!k)
-		return false;
-
-	tmp = (addrs[k - 1] & PAGE_MASK) +
-	      (((addrs[k - 1] & ~PAGE_MASK) + 1) << PAGE_SHIFT);
-
-	return tmp == (addr & PAGE_MASK);
-}
-
 /*
  * This function is called by the vchiq stack once it has been connected to
  * the videocore and clients can start to use the stack.
@@ -224,270 +182,6 @@ void vchiq_add_connected_callback(struct vchiq_device *device, void (*callback)(
 }
 EXPORT_SYMBOL(vchiq_add_connected_callback);
 
-/* There is a potential problem with partial cache lines (pages?)
- * at the ends of the block when reading. If the CPU accessed anything in
- * the same line (page?) then it may have pulled old data into the cache,
- * obscuring the new data underneath. We can solve this by transferring the
- * partial cache lines separately, and allowing the ARM to copy into the
- * cached area.
- */
-
-static struct vchiq_pagelist_info *
-create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
-		size_t count, unsigned short type)
-{
-	struct vchiq_drv_mgmt *drv_mgmt;
-	struct pagelist *pagelist;
-	struct vchiq_pagelist_info *pagelistinfo;
-	struct page **pages;
-	u32 *addrs;
-	unsigned int num_pages, offset, i, k;
-	int actual_pages;
-	size_t pagelist_size;
-	struct scatterlist *scatterlist, *sg;
-	int dma_buffers;
-	dma_addr_t dma_addr;
-
-	if (count >= INT_MAX - PAGE_SIZE)
-		return NULL;
-
-	drv_mgmt = dev_get_drvdata(instance->state->dev);
-
-	if (buf)
-		offset = (uintptr_t)buf & (PAGE_SIZE - 1);
-	else
-		offset = (uintptr_t)ubuf & (PAGE_SIZE - 1);
-	num_pages = DIV_ROUND_UP(count + offset, PAGE_SIZE);
-
-	if ((size_t)num_pages > (SIZE_MAX - sizeof(struct pagelist) -
-			 sizeof(struct vchiq_pagelist_info)) /
-			(sizeof(u32) + sizeof(pages[0]) +
-			 sizeof(struct scatterlist)))
-		return NULL;
-
-	pagelist_size = sizeof(struct pagelist) +
-			(num_pages * sizeof(u32)) +
-			(num_pages * sizeof(pages[0]) +
-			(num_pages * sizeof(struct scatterlist))) +
-			sizeof(struct vchiq_pagelist_info);
-
-	/* Allocate enough storage to hold the page pointers and the page
-	 * list
-	 */
-	pagelist = dma_alloc_coherent(instance->state->dev, pagelist_size, &dma_addr,
-				      GFP_KERNEL);
-
-	dev_dbg(instance->state->dev, "arm: %pK\n", pagelist);
-
-	if (!pagelist)
-		return NULL;
-
-	addrs		= pagelist->addrs;
-	pages		= (struct page **)(addrs + num_pages);
-	scatterlist	= (struct scatterlist *)(pages + num_pages);
-	pagelistinfo	= (struct vchiq_pagelist_info *)
-			  (scatterlist + num_pages);
-
-	pagelist->length = count;
-	pagelist->type = type;
-	pagelist->offset = offset;
-
-	/* Populate the fields of the pagelistinfo structure */
-	pagelistinfo->pagelist = pagelist;
-	pagelistinfo->pagelist_buffer_size = pagelist_size;
-	pagelistinfo->dma_addr = dma_addr;
-	pagelistinfo->dma_dir =  (type == PAGELIST_WRITE) ?
-				  DMA_TO_DEVICE : DMA_FROM_DEVICE;
-	pagelistinfo->num_pages = num_pages;
-	pagelistinfo->pages_need_release = 0;
-	pagelistinfo->pages = pages;
-	pagelistinfo->scatterlist = scatterlist;
-	pagelistinfo->scatterlist_mapped = 0;
-
-	if (buf) {
-		unsigned long length = count;
-		unsigned int off = offset;
-
-		for (actual_pages = 0; actual_pages < num_pages;
-		     actual_pages++) {
-			struct page *pg =
-				vmalloc_to_page((buf +
-						 (actual_pages * PAGE_SIZE)));
-			size_t bytes = PAGE_SIZE - off;
-
-			if (!pg) {
-				cleanup_pagelistinfo(instance, pagelistinfo);
-				return NULL;
-			}
-
-			if (bytes > length)
-				bytes = length;
-			pages[actual_pages] = pg;
-			length -= bytes;
-			off = 0;
-		}
-		/* do not try and release vmalloc pages */
-	} else {
-		actual_pages = pin_user_pages_fast((unsigned long)ubuf & PAGE_MASK, num_pages,
-						   type == PAGELIST_READ, pages);
-
-		if (actual_pages != num_pages) {
-			dev_dbg(instance->state->dev, "arm: Only %d/%d pages locked\n",
-				actual_pages, num_pages);
-
-			/* This is probably due to the process being killed */
-			if (actual_pages > 0)
-				unpin_user_pages(pages, actual_pages);
-			cleanup_pagelistinfo(instance, pagelistinfo);
-			return NULL;
-		}
-		 /* release user pages */
-		pagelistinfo->pages_need_release = 1;
-	}
-
-	/*
-	 * Initialize the scatterlist so that the magic cookie
-	 *  is filled if debugging is enabled
-	 */
-	sg_init_table(scatterlist, num_pages);
-	/* Now set the pages for each scatterlist */
-	for (i = 0; i < num_pages; i++)	{
-		unsigned int len = PAGE_SIZE - offset;
-
-		if (len > count)
-			len = count;
-		sg_set_page(scatterlist + i, pages[i], len, offset);
-		offset = 0;
-		count -= len;
-	}
-
-	dma_buffers = dma_map_sg(instance->state->dev,
-				 scatterlist,
-				 num_pages,
-				 pagelistinfo->dma_dir);
-
-	if (dma_buffers == 0) {
-		cleanup_pagelistinfo(instance, pagelistinfo);
-		return NULL;
-	}
-
-	pagelistinfo->scatterlist_mapped = 1;
-
-	/* Combine adjacent blocks for performance */
-	k = 0;
-	for_each_sg(scatterlist, sg, dma_buffers, i) {
-		unsigned int len = sg_dma_len(sg);
-		dma_addr_t addr = sg_dma_address(sg);
-
-		/* Note: addrs is the address + page_count - 1
-		 * The firmware expects blocks after the first to be page-
-		 * aligned and a multiple of the page size
-		 */
-		WARN_ON(len == 0);
-		WARN_ON(i && (i != (dma_buffers - 1)) && (len & ~PAGE_MASK));
-		WARN_ON(i && (addr & ~PAGE_MASK));
-		if (is_adjacent_block(addrs, addr, k))
-			addrs[k - 1] += ((len + PAGE_SIZE - 1) >> PAGE_SHIFT);
-		else
-			addrs[k++] = (addr & PAGE_MASK) |
-				(((len + PAGE_SIZE - 1) >> PAGE_SHIFT) - 1);
-	}
-
-	/* Partial cache lines (fragments) require special measures */
-	if ((type == PAGELIST_READ) &&
-	    ((pagelist->offset & (drv_mgmt->info->cache_line_size - 1)) ||
-	    ((pagelist->offset + pagelist->length) &
-	    (drv_mgmt->info->cache_line_size - 1)))) {
-		char *fragments;
-
-		if (down_interruptible(&drv_mgmt->free_fragments_sema)) {
-			cleanup_pagelistinfo(instance, pagelistinfo);
-			return NULL;
-		}
-
-		WARN_ON(!drv_mgmt->free_fragments);
-
-		down(&drv_mgmt->free_fragments_mutex);
-		fragments = drv_mgmt->free_fragments;
-		WARN_ON(!fragments);
-		drv_mgmt->free_fragments = *(char **)drv_mgmt->free_fragments;
-		up(&drv_mgmt->free_fragments_mutex);
-		pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
-			(fragments - drv_mgmt->fragments_base) / drv_mgmt->fragments_size;
-	}
-
-	return pagelistinfo;
-}
-
-static void
-free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagelistinfo,
-	      int actual)
-{
-	struct vchiq_drv_mgmt *drv_mgmt;
-	struct pagelist *pagelist = pagelistinfo->pagelist;
-	struct page **pages = pagelistinfo->pages;
-	unsigned int num_pages = pagelistinfo->num_pages;
-
-	dev_dbg(instance->state->dev, "arm: %pK, %d\n", pagelistinfo->pagelist, actual);
-
-	drv_mgmt = dev_get_drvdata(instance->state->dev);
-
-	/*
-	 * NOTE: dma_unmap_sg must be called before the
-	 * cpu can touch any of the data/pages.
-	 */
-	dma_unmap_sg(instance->state->dev, pagelistinfo->scatterlist,
-		     pagelistinfo->num_pages, pagelistinfo->dma_dir);
-	pagelistinfo->scatterlist_mapped = 0;
-
-	/* Deal with any partial cache lines (fragments) */
-	if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS && drv_mgmt->fragments_base) {
-		char *fragments = drv_mgmt->fragments_base +
-			(pagelist->type - PAGELIST_READ_WITH_FRAGMENTS) *
-			drv_mgmt->fragments_size;
-		int head_bytes, tail_bytes;
-
-		head_bytes = (drv_mgmt->info->cache_line_size - pagelist->offset) &
-			(drv_mgmt->info->cache_line_size - 1);
-		tail_bytes = (pagelist->offset + actual) &
-			(drv_mgmt->info->cache_line_size - 1);
-
-		if ((actual >= 0) && (head_bytes != 0)) {
-			if (head_bytes > actual)
-				head_bytes = actual;
-
-			memcpy_to_page(pages[0],
-				pagelist->offset,
-				fragments,
-				head_bytes);
-		}
-		if ((actual >= 0) && (head_bytes < actual) &&
-		    (tail_bytes != 0))
-			memcpy_to_page(pages[num_pages - 1],
-				(pagelist->offset + actual) &
-				(PAGE_SIZE - 1) & ~(drv_mgmt->info->cache_line_size - 1),
-				fragments + drv_mgmt->info->cache_line_size,
-				tail_bytes);
-
-		down(&drv_mgmt->free_fragments_mutex);
-		*(char **)fragments = drv_mgmt->free_fragments;
-		drv_mgmt->free_fragments = fragments;
-		up(&drv_mgmt->free_fragments_mutex);
-		up(&drv_mgmt->free_fragments_sema);
-	}
-
-	/* Need to mark all the pages dirty. */
-	if (pagelist->type != PAGELIST_WRITE &&
-	    pagelistinfo->pages_need_release) {
-		unsigned int i;
-
-		for (i = 0; i < num_pages; i++)
-			set_page_dirty(pages[i]);
-	}
-
-	cleanup_pagelistinfo(instance, pagelistinfo);
-}
-
 static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
 {
 	struct device *dev = &pdev->dev;
@@ -616,38 +310,7 @@ static struct vchiq_arm_state *vchiq_platform_get_arm_state(struct vchiq_state *
 }
 
 
-int
-vchiq_prepare_bulk_data(struct vchiq_instance *instance, struct vchiq_bulk *bulk, void *offset,
-			void __user *uoffset, int size, int dir)
-{
-	struct vchiq_pagelist_info *pagelistinfo;
-
-	pagelistinfo = create_pagelist(instance, offset, uoffset, size,
-				       (dir == VCHIQ_BULK_RECEIVE)
-				       ? PAGELIST_READ
-				       : PAGELIST_WRITE);
-
-	if (!pagelistinfo)
-		return -ENOMEM;
-
-	bulk->data = pagelistinfo->dma_addr;
-
-	/*
-	 * Store the pagelistinfo address in remote_data,
-	 * which isn't used by the slave.
-	 */
-	bulk->remote_data = pagelistinfo;
 
-	return 0;
-}
-
-void
-vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bulk)
-{
-	if (bulk && bulk->remote_data && bulk->actual)
-		free_pagelist(instance, (struct vchiq_pagelist_info *)bulk->remote_data,
-			      bulk->actual);
-}
 
 void vchiq_dump_platform_state(struct seq_file *f)
 {
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 88d510c6793a..586c41cd1ed5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -6,6 +6,7 @@
 #include <linux/mutex.h>
 #include <linux/bitops.h>
 #include <linux/io.h>
+#include <linux/highmem.h>
 #include <linux/kthread.h>
 #include <linux/wait.h>
 #include <linux/delay.h>
@@ -1437,6 +1438,329 @@ poll_services(struct vchiq_state *state)
 		poll_services_of_group(state, group);
 }
 
+static void
+cleanup_pagelistinfo(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagelistinfo)
+{
+	if (pagelistinfo->scatterlist_mapped) {
+		dma_unmap_sg(instance->state->dev, pagelistinfo->scatterlist,
+			     pagelistinfo->num_pages, pagelistinfo->dma_dir);
+	}
+
+	if (pagelistinfo->pages_need_release)
+		unpin_user_pages(pagelistinfo->pages, pagelistinfo->num_pages);
+
+	dma_free_coherent(instance->state->dev, pagelistinfo->pagelist_buffer_size,
+			  pagelistinfo->pagelist, pagelistinfo->dma_addr);
+}
+
+static inline bool
+is_adjacent_block(u32 *addrs, dma_addr_t addr, unsigned int k)
+{
+	u32 tmp;
+
+	if (!k)
+		return false;
+
+	tmp = (addrs[k - 1] & PAGE_MASK) +
+	      (((addrs[k - 1] & ~PAGE_MASK) + 1) << PAGE_SHIFT);
+
+	return tmp == (addr & PAGE_MASK);
+}
+
+/* There is a potential problem with partial cache lines (pages?)
+ * at the ends of the block when reading. If the CPU accessed anything in
+ * the same line (page?) then it may have pulled old data into the cache,
+ * obscuring the new data underneath. We can solve this by transferring the
+ * partial cache lines separately, and allowing the ARM to copy into the
+ * cached area.
+ */
+static struct vchiq_pagelist_info *
+create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
+		size_t count, unsigned short type)
+{
+	struct vchiq_drv_mgmt *drv_mgmt;
+	struct pagelist *pagelist;
+	struct vchiq_pagelist_info *pagelistinfo;
+	struct page **pages;
+	u32 *addrs;
+	unsigned int num_pages, offset, i, k;
+	int actual_pages;
+	size_t pagelist_size;
+	struct scatterlist *scatterlist, *sg;
+	int dma_buffers;
+	dma_addr_t dma_addr;
+
+	if (count >= INT_MAX - PAGE_SIZE)
+		return NULL;
+
+	drv_mgmt = dev_get_drvdata(instance->state->dev);
+
+	if (buf)
+		offset = (uintptr_t)buf & (PAGE_SIZE - 1);
+	else
+		offset = (uintptr_t)ubuf & (PAGE_SIZE - 1);
+	num_pages = DIV_ROUND_UP(count + offset, PAGE_SIZE);
+
+	if ((size_t)num_pages > (SIZE_MAX - sizeof(struct pagelist) -
+			 sizeof(struct vchiq_pagelist_info)) /
+			(sizeof(u32) + sizeof(pages[0]) +
+			 sizeof(struct scatterlist)))
+		return NULL;
+
+	pagelist_size = sizeof(struct pagelist) +
+			(num_pages * sizeof(u32)) +
+			(num_pages * sizeof(pages[0]) +
+			(num_pages * sizeof(struct scatterlist))) +
+			sizeof(struct vchiq_pagelist_info);
+
+	/* Allocate enough storage to hold the page pointers and the page
+	 * list
+	 */
+	pagelist = dma_alloc_coherent(instance->state->dev, pagelist_size, &dma_addr,
+				      GFP_KERNEL);
+
+	dev_dbg(instance->state->dev, "arm: %pK\n", pagelist);
+
+	if (!pagelist)
+		return NULL;
+
+	addrs		= pagelist->addrs;
+	pages		= (struct page **)(addrs + num_pages);
+	scatterlist	= (struct scatterlist *)(pages + num_pages);
+	pagelistinfo	= (struct vchiq_pagelist_info *)
+			  (scatterlist + num_pages);
+
+	pagelist->length = count;
+	pagelist->type = type;
+	pagelist->offset = offset;
+
+	/* Populate the fields of the pagelistinfo structure */
+	pagelistinfo->pagelist = pagelist;
+	pagelistinfo->pagelist_buffer_size = pagelist_size;
+	pagelistinfo->dma_addr = dma_addr;
+	pagelistinfo->dma_dir =  (type == PAGELIST_WRITE) ?
+				  DMA_TO_DEVICE : DMA_FROM_DEVICE;
+	pagelistinfo->num_pages = num_pages;
+	pagelistinfo->pages_need_release = 0;
+	pagelistinfo->pages = pages;
+	pagelistinfo->scatterlist = scatterlist;
+	pagelistinfo->scatterlist_mapped = 0;
+
+	if (buf) {
+		unsigned long length = count;
+		unsigned int off = offset;
+
+		for (actual_pages = 0; actual_pages < num_pages;
+		     actual_pages++) {
+			struct page *pg =
+				vmalloc_to_page((buf +
+						 (actual_pages * PAGE_SIZE)));
+			size_t bytes = PAGE_SIZE - off;
+
+			if (!pg) {
+				cleanup_pagelistinfo(instance, pagelistinfo);
+				return NULL;
+			}
+
+			if (bytes > length)
+				bytes = length;
+			pages[actual_pages] = pg;
+			length -= bytes;
+			off = 0;
+		}
+		/* do not try and release vmalloc pages */
+	} else {
+		actual_pages = pin_user_pages_fast((unsigned long)ubuf & PAGE_MASK, num_pages,
+						   type == PAGELIST_READ, pages);
+
+		if (actual_pages != num_pages) {
+			dev_dbg(instance->state->dev, "arm: Only %d/%d pages locked\n",
+				actual_pages, num_pages);
+
+			/* This is probably due to the process being killed */
+			if (actual_pages > 0)
+				unpin_user_pages(pages, actual_pages);
+			cleanup_pagelistinfo(instance, pagelistinfo);
+			return NULL;
+		}
+		 /* release user pages */
+		pagelistinfo->pages_need_release = 1;
+	}
+
+	/*
+	 * Initialize the scatterlist so that the magic cookie
+	 *  is filled if debugging is enabled
+	 */
+	sg_init_table(scatterlist, num_pages);
+	/* Now set the pages for each scatterlist */
+	for (i = 0; i < num_pages; i++)	{
+		unsigned int len = PAGE_SIZE - offset;
+
+		if (len > count)
+			len = count;
+		sg_set_page(scatterlist + i, pages[i], len, offset);
+		offset = 0;
+		count -= len;
+	}
+
+	dma_buffers = dma_map_sg(instance->state->dev,
+				 scatterlist,
+				 num_pages,
+				 pagelistinfo->dma_dir);
+
+	if (dma_buffers == 0) {
+		cleanup_pagelistinfo(instance, pagelistinfo);
+		return NULL;
+	}
+
+	pagelistinfo->scatterlist_mapped = 1;
+
+	/* Combine adjacent blocks for performance */
+	k = 0;
+	for_each_sg(scatterlist, sg, dma_buffers, i) {
+		unsigned int len = sg_dma_len(sg);
+		dma_addr_t addr = sg_dma_address(sg);
+
+		/* Note: addrs is the address + page_count - 1
+		 * The firmware expects blocks after the first to be page-
+		 * aligned and a multiple of the page size
+		 */
+		WARN_ON(len == 0);
+		WARN_ON(i && (i != (dma_buffers - 1)) && (len & ~PAGE_MASK));
+		WARN_ON(i && (addr & ~PAGE_MASK));
+		if (is_adjacent_block(addrs, addr, k))
+			addrs[k - 1] += ((len + PAGE_SIZE - 1) >> PAGE_SHIFT);
+		else
+			addrs[k++] = (addr & PAGE_MASK) |
+				(((len + PAGE_SIZE - 1) >> PAGE_SHIFT) - 1);
+	}
+
+	/* Partial cache lines (fragments) require special measures */
+	if ((type == PAGELIST_READ) &&
+	    ((pagelist->offset & (drv_mgmt->info->cache_line_size - 1)) ||
+	    ((pagelist->offset + pagelist->length) &
+	    (drv_mgmt->info->cache_line_size - 1)))) {
+		char *fragments;
+
+		if (down_interruptible(&drv_mgmt->free_fragments_sema)) {
+			cleanup_pagelistinfo(instance, pagelistinfo);
+			return NULL;
+		}
+
+		WARN_ON(!drv_mgmt->free_fragments);
+
+		down(&drv_mgmt->free_fragments_mutex);
+		fragments = drv_mgmt->free_fragments;
+		WARN_ON(!fragments);
+		drv_mgmt->free_fragments = *(char **)drv_mgmt->free_fragments;
+		up(&drv_mgmt->free_fragments_mutex);
+		pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
+			(fragments - drv_mgmt->fragments_base) / drv_mgmt->fragments_size;
+	}
+
+	return pagelistinfo;
+}
+
+static void
+free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagelistinfo,
+	      int actual)
+{
+	struct vchiq_drv_mgmt *drv_mgmt;
+	struct pagelist *pagelist = pagelistinfo->pagelist;
+	struct page **pages = pagelistinfo->pages;
+	unsigned int num_pages = pagelistinfo->num_pages;
+
+	dev_dbg(instance->state->dev, "arm: %pK, %d\n", pagelistinfo->pagelist, actual);
+
+	drv_mgmt = dev_get_drvdata(instance->state->dev);
+
+	/*
+	 * NOTE: dma_unmap_sg must be called before the
+	 * cpu can touch any of the data/pages.
+	 */
+	dma_unmap_sg(instance->state->dev, pagelistinfo->scatterlist,
+		     pagelistinfo->num_pages, pagelistinfo->dma_dir);
+	pagelistinfo->scatterlist_mapped = 0;
+
+	/* Deal with any partial cache lines (fragments) */
+	if (pagelist->type >= PAGELIST_READ_WITH_FRAGMENTS && drv_mgmt->fragments_base) {
+		char *fragments = drv_mgmt->fragments_base +
+			(pagelist->type - PAGELIST_READ_WITH_FRAGMENTS) *
+			drv_mgmt->fragments_size;
+		int head_bytes, tail_bytes;
+
+		head_bytes = (drv_mgmt->info->cache_line_size - pagelist->offset) &
+			(drv_mgmt->info->cache_line_size - 1);
+		tail_bytes = (pagelist->offset + actual) &
+			(drv_mgmt->info->cache_line_size - 1);
+
+		if ((actual >= 0) && (head_bytes != 0)) {
+			if (head_bytes > actual)
+				head_bytes = actual;
+
+			memcpy_to_page(pages[0], pagelist->offset,
+				       fragments, head_bytes);
+		}
+		if ((actual >= 0) && (head_bytes < actual) &&
+		    (tail_bytes != 0))
+			memcpy_to_page(pages[num_pages - 1],
+				       (pagelist->offset + actual) &
+				       (PAGE_SIZE - 1) & ~(drv_mgmt->info->cache_line_size - 1),
+				       fragments + drv_mgmt->info->cache_line_size,
+				       tail_bytes);
+
+		down(&drv_mgmt->free_fragments_mutex);
+		*(char **)fragments = drv_mgmt->free_fragments;
+		drv_mgmt->free_fragments = fragments;
+		up(&drv_mgmt->free_fragments_mutex);
+		up(&drv_mgmt->free_fragments_sema);
+	}
+
+	/* Need to mark all the pages dirty. */
+	if (pagelist->type != PAGELIST_WRITE &&
+	    pagelistinfo->pages_need_release) {
+		unsigned int i;
+
+		for (i = 0; i < num_pages; i++)
+			set_page_dirty(pages[i]);
+	}
+
+	cleanup_pagelistinfo(instance, pagelistinfo);
+}
+
+static int
+vchiq_prepare_bulk_data(struct vchiq_instance *instance, struct vchiq_bulk *bulk, void *offset,
+			void __user *uoffset, int size, int dir)
+{
+	struct vchiq_pagelist_info *pagelistinfo;
+
+	pagelistinfo = create_pagelist(instance, offset, uoffset, size,
+				       (dir == VCHIQ_BULK_RECEIVE)
+				       ? PAGELIST_READ
+				       : PAGELIST_WRITE);
+
+	if (!pagelistinfo)
+		return -ENOMEM;
+
+	bulk->data = pagelistinfo->dma_addr;
+
+	/*
+	 * Store the pagelistinfo address in remote_data,
+	 * which isn't used by the slave.
+	 */
+	bulk->remote_data = pagelistinfo;
+
+	return 0;
+}
+
+static void
+vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bulk)
+{
+	if (bulk && bulk->remote_data && bulk->actual)
+		free_pagelist(instance, (struct vchiq_pagelist_info *)bulk->remote_data,
+			      bulk->actual);
+}
+
 /* Called with the bulk_mutex held */
 static void
 abort_outstanding_bulks(struct vchiq_service *service,
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 32b0521aa036..6662dd21e827 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -6,6 +6,7 @@
 
 #include <linux/mutex.h>
 #include <linux/completion.h>
+#include <linux/dma-mapping.h>
 #include <linux/debugfs.h>
 #include <linux/dev_printk.h>
 #include <linux/kthread.h>
@@ -16,6 +17,7 @@
 
 #include "../../include/linux/raspberrypi/vchiq.h"
 #include "vchiq_cfg.h"
+#include "vchiq_pagelist.h"
 
 /* Do this so that we can test-build the code on non-rpi systems */
 #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
@@ -409,6 +411,18 @@ struct vchiq_state {
 	struct opaque_platform_state *platform_state;
 };
 
+struct vchiq_pagelist_info {
+	struct pagelist *pagelist;
+	size_t pagelist_buffer_size;
+	dma_addr_t dma_addr;
+	enum dma_data_direction dma_dir;
+	unsigned int num_pages;
+	unsigned int pages_need_release;
+	struct page **pages;
+	struct scatterlist *scatterlist;
+	unsigned int scatterlist_mapped;
+};
+
 static inline bool vchiq_remote_initialised(const struct vchiq_state *state)
 {
 	return state->remote && state->remote->initialised;
@@ -529,11 +543,6 @@ vchiq_queue_message(struct vchiq_instance *instance, unsigned int handle,
 		    void *context,
 		    size_t size);
 
-int vchiq_prepare_bulk_data(struct vchiq_instance *instance, struct vchiq_bulk *bulk, void *offset,
-			    void __user *uoffset, int size, int dir);
-
-void vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bulk);
-
 void vchiq_dump_platform_state(struct seq_file *f);
 
 void vchiq_dump_platform_instances(struct vchiq_state *state, struct seq_file *f);
-- 
2.45.2

[PATCH 3/3] staging: vchiq_core: Drop vchiq_pagelist.h

[Umang Jain]

vchiq_pagelist.h only defines one struct and a couple of macros.
It can be merged with vchiq_core since all the pagelist related
function helpers are now in vchiq_core for bulk transfers.

Move the struct and related macros to vchiq_core header and drop
vchiq_pagelist.h.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 .../interface/vchiq_arm/vchiq_core.c          |  4 ++++
 .../interface/vchiq_arm/vchiq_core.h          | 11 +++++++++-
 .../interface/vchiq_arm/vchiq_pagelist.h      | 21 -------------------
 3 files changed, 14 insertions(+), 22 deletions(-)
 delete mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 586c41cd1ed5..1281f3bc5548 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -63,6 +63,10 @@
 #define MAKE_REMOTE_USE			(VCHIQ_MSG_REMOTE_USE << TYPE_SHIFT)
 #define MAKE_REMOTE_USE_ACTIVE		(VCHIQ_MSG_REMOTE_USE_ACTIVE << TYPE_SHIFT)
 
+#define PAGELIST_WRITE			0
+#define PAGELIST_READ			1
+#define PAGELIST_READ_WITH_FRAGMENTS	2
+
 #define BELL2	0x08
 
 /* Ensure the fields are wide enough */
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 6662dd21e827..400472f1aa06 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -17,7 +17,6 @@
 
 #include "../../include/linux/raspberrypi/vchiq.h"
 #include "vchiq_cfg.h"
-#include "vchiq_pagelist.h"
 
 /* Do this so that we can test-build the code on non-rpi systems */
 #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
@@ -411,6 +410,16 @@ struct vchiq_state {
 	struct opaque_platform_state *platform_state;
 };
 
+struct pagelist {
+	u32 length;
+	u16 type;
+	u16 offset;
+	u32 addrs[1];	/* N.B. 12 LSBs hold the number
+			 * of following pages at consecutive
+			 * addresses.
+			 */
+};
+
 struct vchiq_pagelist_info {
 	struct pagelist *pagelist;
 	size_t pagelist_buffer_size;
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h
deleted file mode 100644
index ebd12bfabb63..000000000000
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
-/* Copyright (c) 2010-2012 Broadcom. All rights reserved. */
-
-#ifndef VCHIQ_PAGELIST_H
-#define VCHIQ_PAGELIST_H
-
-#define PAGELIST_WRITE 0
-#define PAGELIST_READ 1
-#define PAGELIST_READ_WITH_FRAGMENTS 2
-
-struct pagelist {
-	u32 length;
-	u16 type;
-	u16 offset;
-	u32 addrs[1];	/* N.B. 12 LSBs hold the number
-			 * of following pages at consecutive
-			 * addresses.
-			 */
-};
-
-#endif /* VCHIQ_PAGELIST_H */
-- 
2.45.2

Re: [PATCH 0/3] staging: vchiq_core: Consolidate bulk xfer helper

[Umang Jain]

Hi,

forgot to mention, series to is built and tested on top of

"[PATCH 0/6] staging: vchiq_core: bulk xfer killable() completions"

https://lore.kernel.org/linux-staging/20240918163100.870596-1-umang.jain@ideasonboard.com/T/#t

On 19/09/24 7:51 pm, Umang Jain wrote:
> Few functions were identified were their declaration, definition
> and usage were spread (incorrectly) across vchiq_core and vchiq_arm.
>
> These are being consolidated into vchiq_core in this series.
> Please look at individual patches for details.
>
> 2/3 is in particular a big one. The code is just a move, but
> seems big - as it was diffcult to split it into smaller, compilable
> hunks of individual patches.
>
> Umang Jain (3):
>    staging: vchiq_core: Move remote_event_signal() vchiq_core
>    staging: vchiq_core: Move bulk data functions in vchiq_core
>    staging: vchiq_core: Drop vchiq_pagelist.h
>
>   .../interface/vchiq_arm/vchiq_arm.c           | 356 ------------------
>   .../interface/vchiq_arm/vchiq_core.c          | 350 +++++++++++++++++
>   .../interface/vchiq_arm/vchiq_core.h          |  30 +-
>   .../interface/vchiq_arm/vchiq_pagelist.h      |  21 --
>   4 files changed, 373 insertions(+), 384 deletions(-)
>   delete mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_pagelist.h
>

Re: [PATCH 0/3] staging: vchiq_core: Consolidate bulk xfer helper

[Dan Carpenter]

On Thu, Sep 19, 2024 at 07:51:27PM +0530, Umang Jain wrote:
> Few functions were identified were their declaration, definition
> and usage were spread (incorrectly) across vchiq_core and vchiq_arm.
> 
> These are being consolidated into vchiq_core in this series.
> Please look at individual patches for details.
> 
> 2/3 is in particular a big one. The code is just a move, but
> seems big - as it was diffcult to split it into smaller, compilable
> hunks of individual patches.
> 
> Umang Jain (3):
>   staging: vchiq_core: Move remote_event_signal() vchiq_core
>   staging: vchiq_core: Move bulk data functions in vchiq_core
>   staging: vchiq_core: Drop vchiq_pagelist.h
> 

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

But next time, if you're just moving code around, just ignore checkpatch.  Do
the white space changes as a separate patch.

regards,
dan carpenter

+-                      memcpy_to_page(pages[0],
+-                              pagelist->offset,
+-                              fragments,
+-                              head_bytes);
++                      memcpy_to_page(pages[0], pagelist->offset,
++                                     fragments, head_bytes);
 }
+               if ((actual >= 0) && (head_bytes < actual) &&
+                   (tail_bytes != 0))
+                       memcpy_to_page(pages[num_pages - 1],
+-                              (pagelist->offset + actual) &
+-                              (PAGE_SIZE - 1) & ~(drv_mgmt->info->cache_line_size - 1),
+-                              fragments + drv_mgmt->info->cache_line_size,
+-                              tail_bytes);
++                                     (pagelist->offset + actual) &
++                                     (PAGE_SIZE - 1) & ~(drv_mgmt->info->cache_line_size - 1),
++                                     fragments + drv_mgmt->info->cache_line_size,
++                                     tail_bytes);

Re: [PATCH 0/3] staging: vchiq_core: Consolidate bulk xfer helper

[Umang Jain]

Hi Dan,

On 25/09/24 2:53 pm, Dan Carpenter wrote:
> On Thu, Sep 19, 2024 at 07:51:27PM +0530, Umang Jain wrote:
>> Few functions were identified were their declaration, definition
>> and usage were spread (incorrectly) across vchiq_core and vchiq_arm.
>>
>> These are being consolidated into vchiq_core in this series.
>> Please look at individual patches for details.
>>
>> 2/3 is in particular a big one. The code is just a move, but
>> seems big - as it was diffcult to split it into smaller, compilable
>> hunks of individual patches.
>>
>> Umang Jain (3):
>>    staging: vchiq_core: Move remote_event_signal() vchiq_core
>>    staging: vchiq_core: Move bulk data functions in vchiq_core
>>    staging: vchiq_core: Drop vchiq_pagelist.h
>>
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

Thanks for reviewing.

>
> But next time, if you're just moving code around, just ignore checkpatch.  Do
> the white space changes as a separate patch.

Thanks for the tip, will surely keep this mind.
>
> regards,
> dan carpenter
>
> +-                      memcpy_to_page(pages[0],
> +-                              pagelist->offset,
> +-                              fragments,
> +-                              head_bytes);
> ++                      memcpy_to_page(pages[0], pagelist->offset,
> ++                                     fragments, head_bytes);
>   }
> +               if ((actual >= 0) && (head_bytes < actual) &&
> +                   (tail_bytes != 0))
> +                       memcpy_to_page(pages[num_pages - 1],
> +-                              (pagelist->offset + actual) &
> +-                              (PAGE_SIZE - 1) & ~(drv_mgmt->info->cache_line_size - 1),
> +-                              fragments + drv_mgmt->info->cache_line_size,
> +-                              tail_bytes);
> ++                                     (pagelist->offset + actual) &
> ++                                     (PAGE_SIZE - 1) & ~(drv_mgmt->info->cache_line_size - 1),
> ++                                     fragments + drv_mgmt->info->cache_line_size,
> ++                                     tail_bytes);
>