public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] misc: fastrpc: Refactor and add userspace buffer support
@ 2025-11-28 10:34 Ekansh Gupta
  2025-11-28 10:34 ` [PATCH v2 1/2] misc: fastrpc: Refactor mmap and munmap logic into helper functions Ekansh Gupta
  2025-11-28 10:34 ` [PATCH v2 2/2] misc: fastrpc: Support mapping userspace-allocated buffers Ekansh Gupta
  0 siblings, 2 replies; 6+ messages in thread
From: Ekansh Gupta @ 2025-11-28 10:34 UTC (permalink / raw)
  To: srini, linux-arm-msm
  Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
	dmitry.baryshkov

This series improves the FastRPC driver by first refactoring mmap and
munmap logic into helper functions, and then adding support for mapping
userspace-allocated buffers to the DSP.

Patch 1 introduces helper functions for DSP-side operations, improving
code readability and preparing for future enhancements. Patch 2 builds
on this by enabling applications to share memory allocated in userspace
(via rpcmem or DMABUF) with the DSP through SMMU, improving flexibility
and performance.

No functional changes are introduced in the first patch; the second
patch adds the new feature.

Patch [v1]: https://lore.kernel.org/all/20250704083726.1901705-1-ekansh.gupta@oss.qualcomm.com/

Changes in v2:
  - Split change into meaningful patches.
  - Replaced uintptr_t with u64.
  - Fixed commit message.

Ekansh Gupta (2):
  misc: fastrpc: Refactor mmap and munmap logic into helper functions
  misc: fastrpc: Support mapping userspace-allocated buffers

 drivers/misc/fastrpc.c | 190 ++++++++++++++++++++++++++++++++---------
 1 file changed, 151 insertions(+), 39 deletions(-)

-- 
2.34.1


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

* [PATCH v2 1/2] misc: fastrpc: Refactor mmap and munmap logic into helper functions
  2025-11-28 10:34 [PATCH v2 0/2] misc: fastrpc: Refactor and add userspace buffer support Ekansh Gupta
@ 2025-11-28 10:34 ` Ekansh Gupta
  2025-11-29 11:10   ` kernel test robot
  2025-11-28 10:34 ` [PATCH v2 2/2] misc: fastrpc: Support mapping userspace-allocated buffers Ekansh Gupta
  1 sibling, 1 reply; 6+ messages in thread
From: Ekansh Gupta @ 2025-11-28 10:34 UTC (permalink / raw)
  To: srini, linux-arm-msm
  Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
	dmitry.baryshkov

Refactor FastRPC mmap and munmap handling by introducing dedicated
helper functions for DSP-side operations. This change improves code
readability and separates DSP invocation logic from buffer allocation
and cleanup.

Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 110 +++++++++++++++++++++++++++--------------
 1 file changed, 74 insertions(+), 36 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index ee652ef01534..9bf76e224852 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1811,24 +1811,33 @@ static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp)
 	return 0;
 }
 
-static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *buf)
+static int fastrpc_req_munmap_dsp(struct fastrpc_user *fl, u64 raddr, u64 size)
 {
 	struct fastrpc_invoke_args args[1] = { [0] = { 0 } };
 	struct fastrpc_munmap_req_msg req_msg;
-	struct device *dev = fl->sctx->dev;
 	int err;
 	u32 sc;
 
 	req_msg.client_id = fl->client_id;
-	req_msg.size = buf->size;
-	req_msg.vaddr = buf->raddr;
+	req_msg.size = size;
+	req_msg.vaddr = raddr;
 
-	args[0].ptr = (u64) (uintptr_t) &req_msg;
+	args[0].ptr = (u64) &req_msg;
 	args[0].length = sizeof(req_msg);
 
 	sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MUNMAP, 1, 0);
 	err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
 				      &args[0]);
+
+	return err;
+}
+
+static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *buf)
+{
+	struct device *dev = fl->sctx->dev;
+	int err;
+
+	err = fastrpc_req_munmap_dsp(fl, buf->raddr, buf->size);
 	if (!err) {
 		dev_dbg(dev, "unmmap\tpt 0x%09lx OK\n", buf->raddr);
 		spin_lock(&fl->lock);
@@ -1869,26 +1878,54 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 	return fastrpc_req_munmap_impl(fl, buf);
 }
 
-static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
+static int fastrpc_req_map_dsp(struct fastrpc_user *fl, u64 phys,
+			       u64 size, u32 flag, u64 vaddrin,
+			       u64 *raddr)
 {
 	struct fastrpc_invoke_args args[3] = { [0 ... 2] = { 0 } };
-	struct fastrpc_buf *buf = NULL;
 	struct fastrpc_mmap_req_msg req_msg;
 	struct fastrpc_mmap_rsp_msg rsp_msg;
 	struct fastrpc_phy_page pages;
-	struct fastrpc_req_mmap req;
-	struct device *dev = fl->sctx->dev;
 	int err;
 	u32 sc;
 
-	if (copy_from_user(&req, argp, sizeof(req)))
-		return -EFAULT;
+	req_msg.client_id = fl->client_id;
+	req_msg.flags = flag;
+	req_msg.vaddr = vaddrin;
+	req_msg.num = sizeof(pages);
 
-	if (req.flags != ADSP_MMAP_ADD_PAGES && req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR) {
-		dev_err(dev, "flag not supported 0x%x\n", req.flags);
+	args[0].ptr = (u64)&req_msg;
+	args[0].length = sizeof(req_msg);
 
-		return -EINVAL;
+	pages.addr = phys;
+	pages.size = size;
+
+	args[1].ptr = (u64)&pages;
+	args[1].length = sizeof(pages);
+
+	args[2].ptr = (u64)&rsp_msg;
+	args[2].length = sizeof(rsp_msg);
+	sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1);
+	err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
+				      &args[0]);
+
+	if (err) {
+		dev_err(fl->sctx->dev, "mmap error (len 0x%08llx)\n", size);
+		return err;
 	}
+	*raddr = rsp_msg.vaddr;
+
+	return 0;
+}
+
+static int fastrpc_req_buf_alloc(struct fastrpc_user *fl,
+				 struct fastrpc_req_mmap req,
+				 char __user *argp)
+{
+	struct device *dev = fl->sctx->dev;
+	struct fastrpc_buf *buf = NULL;
+	u64 raddr = 0;
+	int err;
 
 	if (req.vaddrin) {
 		dev_err(dev, "adding user allocated pages is not supported\n");
@@ -1905,26 +1942,8 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
 		return err;
 	}
 
-	req_msg.client_id = fl->client_id;
-	req_msg.flags = req.flags;
-	req_msg.vaddr = req.vaddrin;
-	req_msg.num = sizeof(pages);
-
-	args[0].ptr = (u64) (uintptr_t) &req_msg;
-	args[0].length = sizeof(req_msg);
-
-	pages.addr = buf->phys;
-	pages.size = buf->size;
-
-	args[1].ptr = (u64) (uintptr_t) &pages;
-	args[1].length = sizeof(pages);
-
-	args[2].ptr = (u64) (uintptr_t) &rsp_msg;
-	args[2].length = sizeof(rsp_msg);
-
-	sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1);
-	err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
-				      &args[0]);
+	err = fastrpc_req_map_dsp(fl, buf->phys, buf->size, req.flags,
+				  req.vaddrin, &raddr);
 	if (err) {
 		dev_err(dev, "mmap error (len 0x%08llx)\n", buf->size);
 		fastrpc_buf_free(buf);
@@ -1932,10 +1951,10 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
 	}
 
 	/* update the buffer to be able to deallocate the memory on the DSP */
-	buf->raddr = (uintptr_t) rsp_msg.vaddr;
+	buf->raddr = (uintptr_t)raddr;
 
 	/* let the client know the address to use */
-	req.vaddrout = rsp_msg.vaddr;
+	req.vaddrout = raddr;
 
 	/* Add memory to static PD pool, protection thru hypervisor */
 	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
@@ -1970,6 +1989,25 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
 	return err;
 }
 
+static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
+{
+	struct fastrpc_req_mmap req;
+	int err;
+
+	if (copy_from_user(&req, argp, sizeof(req)))
+		return -EFAULT;
+
+	if (req.flags != ADSP_MMAP_ADD_PAGES && req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR) {
+		dev_err(fl->sctx->dev, "flag not supported 0x%x\n", req.flags);
+
+		return -EINVAL;
+	}
+
+	err = fastrpc_req_buf_alloc(fl, req, argp);
+
+	return err;
+}
+
 static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_mem_unmap *req)
 {
 	struct fastrpc_invoke_args args[1] = { [0] = { 0 } };
-- 
2.34.1


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

* [PATCH v2 2/2] misc: fastrpc: Support mapping userspace-allocated buffers
  2025-11-28 10:34 [PATCH v2 0/2] misc: fastrpc: Refactor and add userspace buffer support Ekansh Gupta
  2025-11-28 10:34 ` [PATCH v2 1/2] misc: fastrpc: Refactor mmap and munmap logic into helper functions Ekansh Gupta
@ 2025-11-28 10:34 ` Ekansh Gupta
  2025-11-28 15:32   ` Greg KH
  1 sibling, 1 reply; 6+ messages in thread
From: Ekansh Gupta @ 2025-11-28 10:34 UTC (permalink / raw)
  To: srini, linux-arm-msm
  Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
	dmitry.baryshkov

Currently, FastRPC only supports mapping buffers allocated by the
kernel. This limits flexibility for applications that allocate memory
in userspace using rpcmem or DMABUF and need to share it with the DSP.
Add support for mapping and unmapping userspace-allocated buffers to
the DSP through SMMU. This includes handling map requests for rpcmem
and DMABUF-backed memory and providing corresponding unmap
functionality.

Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 96 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 85 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 9bf76e224852..feba79913763 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1854,8 +1854,10 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *
 static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 {
 	struct fastrpc_buf *buf = NULL, *iter, *b;
+	struct fastrpc_map *map = NULL, *iterm, *m;
 	struct fastrpc_req_munmap req;
 	struct device *dev = fl->sctx->dev;
+	int err;
 
 	if (copy_from_user(&req, argp, sizeof(req)))
 		return -EFAULT;
@@ -1869,13 +1871,41 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
 	}
 	spin_unlock(&fl->lock);
 
-	if (!buf) {
-		dev_err(dev, "mmap\t\tpt 0x%09llx [len 0x%08llx] not in list\n",
+	if (buf) {
+		err = fastrpc_req_munmap_impl(fl, buf);
+		if (err) {
+			spin_lock(&fl->lock);
+			list_add_tail(&buf->node, &fl->mmaps);
+			spin_unlock(&fl->lock);
+		}
+		return err;
+	}
+
+	spin_lock(&fl->lock);
+	list_for_each_entry_safe(iterm, m, &fl->maps, node) {
+		if (iterm->raddr == req.vaddrout) {
+			map = iterm;
+			list_del(&iterm->node);
+			break;
+		}
+	}
+	spin_unlock(&fl->lock);
+	if (!map) {
+		dev_dbg(dev, "buffer/map not found addr 0x%09llx, len 0x%08llx\n",
 			req.vaddrout, req.size);
 		return -EINVAL;
 	}
 
-	return fastrpc_req_munmap_impl(fl, buf);
+	err = fastrpc_req_munmap_dsp(fl, map->raddr, map->size);
+	if (err) {
+		dev_dbg(dev, "unmmap\tpt fd = %d, 0x%09llx error\n",  map->fd, map->raddr);
+		spin_lock(&fl->lock);
+		list_add_tail(&map->node, &fl->maps);
+		spin_unlock(&fl->lock);
+	} else {
+		fastrpc_map_put(map);
+	}
+	return err;
 }
 
 static int fastrpc_req_map_dsp(struct fastrpc_user *fl, u64 phys,
@@ -1989,25 +2019,69 @@ static int fastrpc_req_buf_alloc(struct fastrpc_user *fl,
 	return err;
 }
 
-static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
+static int fastrpc_req_map_create(struct fastrpc_user *fl,
+				  struct fastrpc_req_mmap req,
+				  char __user *argp)
 {
-	struct fastrpc_req_mmap req;
+	struct fastrpc_map *map = NULL;
+	struct device *dev = fl->sctx->dev;
+	u64 raddr = 0;
 	int err;
 
-	if (copy_from_user(&req, argp, sizeof(req)))
-		return -EFAULT;
+	err = fastrpc_map_create(fl, req.fd, req.size, 0, &map);
+	if (err) {
+		dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
+		return err;
+	}
 
-	if (req.flags != ADSP_MMAP_ADD_PAGES && req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR) {
-		dev_err(fl->sctx->dev, "flag not supported 0x%x\n", req.flags);
+	err = fastrpc_req_map_dsp(fl, map->phys, map->size, req.flags,
+				  req.vaddrin, &raddr);
+	if (err)
+		goto err_invoke;
 
-		return -EINVAL;
+	/* update the buffer to be able to deallocate the memory on the DSP */
+	map->raddr = (u64)raddr;
+
+	/* let the client know the address to use */
+	req.vaddrout = raddr;
+	dev_dbg(dev, "mmap\t\tpt 0x%09llx OK [len 0x%08llx]\n",
+		map->raddr, map->size);
+
+	if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
+		err = -EFAULT;
+		goto err_copy;
 	}
 
-	err = fastrpc_req_buf_alloc(fl, req, argp);
+	return 0;
+err_copy:
+	fastrpc_req_munmap_dsp(fl, map->raddr, map->size);
+err_invoke:
+	fastrpc_map_put(map);
 
 	return err;
 }
 
+static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
+{
+	struct fastrpc_req_mmap req;
+	int err;
+
+	if (copy_from_user(&req, argp, sizeof(req)))
+		return -EFAULT;
+
+	if ((req.flags == ADSP_MMAP_ADD_PAGES ||
+	     req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR)) {
+		err = fastrpc_req_buf_alloc(fl, req, argp);
+		if (err)
+			return err;
+	} else {
+		err = fastrpc_req_map_create(fl, req, argp);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
 static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_mem_unmap *req)
 {
 	struct fastrpc_invoke_args args[1] = { [0] = { 0 } };
-- 
2.34.1


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

* Re: [PATCH v2 2/2] misc: fastrpc: Support mapping userspace-allocated buffers
  2025-11-28 10:34 ` [PATCH v2 2/2] misc: fastrpc: Support mapping userspace-allocated buffers Ekansh Gupta
@ 2025-11-28 15:32   ` Greg KH
  2025-12-02  6:07     ` Ekansh Gupta
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2025-11-28 15:32 UTC (permalink / raw)
  To: Ekansh Gupta
  Cc: srini, linux-arm-msm, quic_bkumar, linux-kernel, quic_chennak,
	dri-devel, arnd, dmitry.baryshkov

On Fri, Nov 28, 2025 at 04:04:28PM +0530, Ekansh Gupta wrote:
> Currently, FastRPC only supports mapping buffers allocated by the
> kernel. This limits flexibility for applications that allocate memory
> in userspace using rpcmem or DMABUF and need to share it with the DSP.
> Add support for mapping and unmapping userspace-allocated buffers to
> the DSP through SMMU. This includes handling map requests for rpcmem
> and DMABUF-backed memory and providing corresponding unmap
> functionality.
> 
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 96 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 85 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 9bf76e224852..feba79913763 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1854,8 +1854,10 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *
>  static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
>  {
>  	struct fastrpc_buf *buf = NULL, *iter, *b;
> +	struct fastrpc_map *map = NULL, *iterm, *m;
>  	struct fastrpc_req_munmap req;
>  	struct device *dev = fl->sctx->dev;
> +	int err;
>  
>  	if (copy_from_user(&req, argp, sizeof(req)))
>  		return -EFAULT;
> @@ -1869,13 +1871,41 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
>  	}
>  	spin_unlock(&fl->lock);
>  
> -	if (!buf) {
> -		dev_err(dev, "mmap\t\tpt 0x%09llx [len 0x%08llx] not in list\n",
> +	if (buf) {
> +		err = fastrpc_req_munmap_impl(fl, buf);
> +		if (err) {
> +			spin_lock(&fl->lock);
> +			list_add_tail(&buf->node, &fl->mmaps);
> +			spin_unlock(&fl->lock);
> +		}
> +		return err;
> +	}
> +
> +	spin_lock(&fl->lock);
> +	list_for_each_entry_safe(iterm, m, &fl->maps, node) {
> +		if (iterm->raddr == req.vaddrout) {
> +			map = iterm;
> +			list_del(&iterm->node);
> +			break;
> +		}
> +	}
> +	spin_unlock(&fl->lock);
> +	if (!map) {
> +		dev_dbg(dev, "buffer/map not found addr 0x%09llx, len 0x%08llx\n",
>  			req.vaddrout, req.size);

Never print out kernel pointers "raw" like this, use the real %p tags
instead.  Odd that the current code does this, that is not good and is
probably a "information leak" somehow.

Can you fix that up first so it can be backported properly?

>  		return -EINVAL;
>  	}
>  
> -	return fastrpc_req_munmap_impl(fl, buf);
> +	err = fastrpc_req_munmap_dsp(fl, map->raddr, map->size);
> +	if (err) {
> +		dev_dbg(dev, "unmmap\tpt fd = %d, 0x%09llx error\n",  map->fd, map->raddr);

Same here.  Also, no need for a \t in a kernel log message.

> +		spin_lock(&fl->lock);
> +		list_add_tail(&map->node, &fl->maps);
> +		spin_unlock(&fl->lock);
> +	} else {
> +		fastrpc_map_put(map);
> +	}
> +	return err;
>  }
>  
>  static int fastrpc_req_map_dsp(struct fastrpc_user *fl, u64 phys,
> @@ -1989,25 +2019,69 @@ static int fastrpc_req_buf_alloc(struct fastrpc_user *fl,
>  	return err;
>  }
>  
> -static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> +static int fastrpc_req_map_create(struct fastrpc_user *fl,
> +				  struct fastrpc_req_mmap req,
> +				  char __user *argp)
>  {
> -	struct fastrpc_req_mmap req;
> +	struct fastrpc_map *map = NULL;
> +	struct device *dev = fl->sctx->dev;
> +	u64 raddr = 0;
>  	int err;
>  
> -	if (copy_from_user(&req, argp, sizeof(req)))
> -		return -EFAULT;
> +	err = fastrpc_map_create(fl, req.fd, req.size, 0, &map);
> +	if (err) {
> +		dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
> +		return err;
> +	}
>  
> -	if (req.flags != ADSP_MMAP_ADD_PAGES && req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR) {
> -		dev_err(fl->sctx->dev, "flag not supported 0x%x\n", req.flags);
> +	err = fastrpc_req_map_dsp(fl, map->phys, map->size, req.flags,
> +				  req.vaddrin, &raddr);
> +	if (err)
> +		goto err_invoke;
>  
> -		return -EINVAL;
> +	/* update the buffer to be able to deallocate the memory on the DSP */
> +	map->raddr = (u64)raddr;
> +
> +	/* let the client know the address to use */
> +	req.vaddrout = raddr;
> +	dev_dbg(dev, "mmap\t\tpt 0x%09llx OK [len 0x%08llx]\n",
> +		map->raddr, map->size);
> +
> +	if (copy_to_user((void __user *)argp, &req, sizeof(req))) {

argp is already a user pointer, no need to cast it again, right?

> +		err = -EFAULT;
> +		goto err_copy;
>  	}
>  
> -	err = fastrpc_req_buf_alloc(fl, req, argp);
> +	return 0;
> +err_copy:
> +	fastrpc_req_munmap_dsp(fl, map->raddr, map->size);
> +err_invoke:
> +	fastrpc_map_put(map);
>  
>  	return err;
>  }
>  
> +static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
> +{
> +	struct fastrpc_req_mmap req;
> +	int err;
> +
> +	if (copy_from_user(&req, argp, sizeof(req)))
> +		return -EFAULT;
> +
> +	if ((req.flags == ADSP_MMAP_ADD_PAGES ||
> +	     req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR)) {
> +		err = fastrpc_req_buf_alloc(fl, req, argp);
> +		if (err)
> +			return err;
> +	} else {
> +		err = fastrpc_req_map_create(fl, req, argp);

You changed the logic here from what used to happen if req.flags was not
set to those two values.  Are you _sure_ you mean to do that?  If so,
how does userspace know?  Why don't you have a new flag for the new
type of memory you want to map?

thanks,

greg k-h

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

* Re: [PATCH v2 1/2] misc: fastrpc: Refactor mmap and munmap logic into helper functions
  2025-11-28 10:34 ` [PATCH v2 1/2] misc: fastrpc: Refactor mmap and munmap logic into helper functions Ekansh Gupta
@ 2025-11-29 11:10   ` kernel test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-11-29 11:10 UTC (permalink / raw)
  To: Ekansh Gupta, srini, linux-arm-msm
  Cc: oe-kbuild-all, gregkh, quic_bkumar, linux-kernel, quic_chennak,
	dri-devel, arnd, dmitry.baryshkov

Hi Ekansh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.18-rc7 next-20251128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ekansh-Gupta/misc-fastrpc-Refactor-mmap-and-munmap-logic-into-helper-functions/20251128-183620
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20251128103428.1119696-2-ekansh.gupta%40oss.qualcomm.com
patch subject: [PATCH v2 1/2] misc: fastrpc: Refactor mmap and munmap logic into helper functions
config: arm-allyesconfig (https://download.01.org/0day-ci/archive/20251129/202511291834.zu81Ud77-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251129/202511291834.zu81Ud77-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511291834.zu81Ud77-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/misc/fastrpc.c: In function 'fastrpc_req_munmap_dsp':
>> drivers/misc/fastrpc.c:1825:23: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
    1825 |         args[0].ptr = (u64) &req_msg;
         |                       ^
   drivers/misc/fastrpc.c: In function 'fastrpc_req_map_dsp':
   drivers/misc/fastrpc.c:1897:23: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
    1897 |         args[0].ptr = (u64)&req_msg;
         |                       ^
   drivers/misc/fastrpc.c:1903:23: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
    1903 |         args[1].ptr = (u64)&pages;
         |                       ^
   drivers/misc/fastrpc.c:1906:23: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
    1906 |         args[2].ptr = (u64)&rsp_msg;
         |                       ^


vim +1825 drivers/misc/fastrpc.c

  1813	
  1814	static int fastrpc_req_munmap_dsp(struct fastrpc_user *fl, u64 raddr, u64 size)
  1815	{
  1816		struct fastrpc_invoke_args args[1] = { [0] = { 0 } };
  1817		struct fastrpc_munmap_req_msg req_msg;
  1818		int err;
  1819		u32 sc;
  1820	
  1821		req_msg.client_id = fl->client_id;
  1822		req_msg.size = size;
  1823		req_msg.vaddr = raddr;
  1824	
> 1825		args[0].ptr = (u64) &req_msg;
  1826		args[0].length = sizeof(req_msg);
  1827	
  1828		sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MUNMAP, 1, 0);
  1829		err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
  1830					      &args[0]);
  1831	
  1832		return err;
  1833	}
  1834	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v2 2/2] misc: fastrpc: Support mapping userspace-allocated buffers
  2025-11-28 15:32   ` Greg KH
@ 2025-12-02  6:07     ` Ekansh Gupta
  0 siblings, 0 replies; 6+ messages in thread
From: Ekansh Gupta @ 2025-12-02  6:07 UTC (permalink / raw)
  To: Greg KH
  Cc: srini, linux-arm-msm, quic_bkumar, linux-kernel, quic_chennak,
	dri-devel, arnd, dmitry.baryshkov



On 11/28/2025 9:02 PM, Greg KH wrote:
> On Fri, Nov 28, 2025 at 04:04:28PM +0530, Ekansh Gupta wrote:
>> Currently, FastRPC only supports mapping buffers allocated by the
>> kernel. This limits flexibility for applications that allocate memory
>> in userspace using rpcmem or DMABUF and need to share it with the DSP.
>> Add support for mapping and unmapping userspace-allocated buffers to
>> the DSP through SMMU. This includes handling map requests for rpcmem
>> and DMABUF-backed memory and providing corresponding unmap
>> functionality.
>>
>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>> ---
>>  drivers/misc/fastrpc.c | 96 +++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 85 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 9bf76e224852..feba79913763 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -1854,8 +1854,10 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *
>>  static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
>>  {
>>  	struct fastrpc_buf *buf = NULL, *iter, *b;
>> +	struct fastrpc_map *map = NULL, *iterm, *m;
>>  	struct fastrpc_req_munmap req;
>>  	struct device *dev = fl->sctx->dev;
>> +	int err;
>>  
>>  	if (copy_from_user(&req, argp, sizeof(req)))
>>  		return -EFAULT;
>> @@ -1869,13 +1871,41 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
>>  	}
>>  	spin_unlock(&fl->lock);
>>  
>> -	if (!buf) {
>> -		dev_err(dev, "mmap\t\tpt 0x%09llx [len 0x%08llx] not in list\n",
>> +	if (buf) {
>> +		err = fastrpc_req_munmap_impl(fl, buf);
>> +		if (err) {
>> +			spin_lock(&fl->lock);
>> +			list_add_tail(&buf->node, &fl->mmaps);
>> +			spin_unlock(&fl->lock);
>> +		}
>> +		return err;
>> +	}
>> +
>> +	spin_lock(&fl->lock);
>> +	list_for_each_entry_safe(iterm, m, &fl->maps, node) {
>> +		if (iterm->raddr == req.vaddrout) {
>> +			map = iterm;
>> +			list_del(&iterm->node);
>> +			break;
>> +		}
>> +	}
>> +	spin_unlock(&fl->lock);
>> +	if (!map) {
>> +		dev_dbg(dev, "buffer/map not found addr 0x%09llx, len 0x%08llx\n",
>>  			req.vaddrout, req.size);
> Never print out kernel pointers "raw" like this, use the real %p tags
> instead.  Odd that the current code does this, that is not good and is
> probably a "information leak" somehow.
>
> Can you fix that up first so it can be backported properly?
Thanks for pointing this out, I'll fix this before refactoring.
>
>>  		return -EINVAL;
>>  	}
>>  
>> -	return fastrpc_req_munmap_impl(fl, buf);
>> +	err = fastrpc_req_munmap_dsp(fl, map->raddr, map->size);
>> +	if (err) {
>> +		dev_dbg(dev, "unmmap\tpt fd = %d, 0x%09llx error\n",  map->fd, map->raddr);
> Same here.  Also, no need for a \t in a kernel log message.
Ack.
>
>> +		spin_lock(&fl->lock);
>> +		list_add_tail(&map->node, &fl->maps);
>> +		spin_unlock(&fl->lock);
>> +	} else {
>> +		fastrpc_map_put(map);
>> +	}
>> +	return err;
>>  }
>>  
>>  static int fastrpc_req_map_dsp(struct fastrpc_user *fl, u64 phys,
>> @@ -1989,25 +2019,69 @@ static int fastrpc_req_buf_alloc(struct fastrpc_user *fl,
>>  	return err;
>>  }
>>  
>> -static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>> +static int fastrpc_req_map_create(struct fastrpc_user *fl,
>> +				  struct fastrpc_req_mmap req,
>> +				  char __user *argp)
>>  {
>> -	struct fastrpc_req_mmap req;
>> +	struct fastrpc_map *map = NULL;
>> +	struct device *dev = fl->sctx->dev;
>> +	u64 raddr = 0;
>>  	int err;
>>  
>> -	if (copy_from_user(&req, argp, sizeof(req)))
>> -		return -EFAULT;
>> +	err = fastrpc_map_create(fl, req.fd, req.size, 0, &map);
>> +	if (err) {
>> +		dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
>> +		return err;
>> +	}
>>  
>> -	if (req.flags != ADSP_MMAP_ADD_PAGES && req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR) {
>> -		dev_err(fl->sctx->dev, "flag not supported 0x%x\n", req.flags);
>> +	err = fastrpc_req_map_dsp(fl, map->phys, map->size, req.flags,
>> +				  req.vaddrin, &raddr);
>> +	if (err)
>> +		goto err_invoke;
>>  
>> -		return -EINVAL;
>> +	/* update the buffer to be able to deallocate the memory on the DSP */
>> +	map->raddr = (u64)raddr;
>> +
>> +	/* let the client know the address to use */
>> +	req.vaddrout = raddr;
>> +	dev_dbg(dev, "mmap\t\tpt 0x%09llx OK [len 0x%08llx]\n",
>> +		map->raddr, map->size);
>> +
>> +	if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
> argp is already a user pointer, no need to cast it again, right?
yes, right.
>
>> +		err = -EFAULT;
>> +		goto err_copy;
>>  	}
>>  
>> -	err = fastrpc_req_buf_alloc(fl, req, argp);
>> +	return 0;
>> +err_copy:
>> +	fastrpc_req_munmap_dsp(fl, map->raddr, map->size);
>> +err_invoke:
>> +	fastrpc_map_put(map);
>>  
>>  	return err;
>>  }
>>  
>> +static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>> +{
>> +	struct fastrpc_req_mmap req;
>> +	int err;
>> +
>> +	if (copy_from_user(&req, argp, sizeof(req)))
>> +		return -EFAULT;
>> +
>> +	if ((req.flags == ADSP_MMAP_ADD_PAGES ||
>> +	     req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR)) {
>> +		err = fastrpc_req_buf_alloc(fl, req, argp);
>> +		if (err)
>> +			return err;
>> +	} else {
>> +		err = fastrpc_req_map_create(fl, req, argp);
> You changed the logic here from what used to happen if req.flags was not
> set to those two values.  Are you _sure_ you mean to do that?  If so,
> how does userspace know?  Why don't you have a new flag for the new
> type of memory you want to map?
The userspace follows the same logic for all the flags other than the defined flags
where a buffer is allocated using DMA-BUF and then mapped onto DSP using this
IOCTL call.

Do you see any concerns with this?

Thanks,
Ekansh
>
> thanks,
>
> greg k-h


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

end of thread, other threads:[~2025-12-02  6:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-28 10:34 [PATCH v2 0/2] misc: fastrpc: Refactor and add userspace buffer support Ekansh Gupta
2025-11-28 10:34 ` [PATCH v2 1/2] misc: fastrpc: Refactor mmap and munmap logic into helper functions Ekansh Gupta
2025-11-29 11:10   ` kernel test robot
2025-11-28 10:34 ` [PATCH v2 2/2] misc: fastrpc: Support mapping userspace-allocated buffers Ekansh Gupta
2025-11-28 15:32   ` Greg KH
2025-12-02  6:07     ` Ekansh Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox