* [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
* 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
* [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 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