2019-09-13 16:41:07

by Jorge Ramirez-Ortiz

[permalink] [raw]
Subject: [PATCH 0/5] misc: fastrpc: fixes and map/unmap support

Hi Greg

These patches implement a few fixes identified while working on the
QCS404 ML integration plus we now have support for mmap/unmap of
buffers (so the process can be created with less initial memory
requirements).


Jorge Ramirez-Ortiz (4):
misc: fastrpc: add mmap/unmap support
misc: fastrpc: do not interrupt kernel calls
misc: fastrpc: handle interrupted contexts
misc: fastrpc: revert max init file size back to 2MB

Srinivas Kandagatla (1):
misc: fastrpc: fix memory leak from miscdev->name

drivers/misc/fastrpc.c | 209 ++++++++++++++++++++++++++++++++++--
include/uapi/misc/fastrpc.h | 15 +++
2 files changed, 213 insertions(+), 11 deletions(-)

--
2.23.0


2019-09-13 16:41:07

by Jorge Ramirez-Ortiz

[permalink] [raw]
Subject: [PATCH 5/5] misc: fastrpc: revert max init file size back to 2MB

With the integration of the mmap/unmap functionality, it is no longer
necessary to allow large memory allocations upfront since they can be
handled during runtime.

Tested on QCS404 with CDSP Neural Processing test suite.

Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Reviewed-by: Srinivas Kandagatla <[email protected]>
---
drivers/misc/fastrpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 40b48db032b5..ee6de5d9993d 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -32,7 +32,7 @@
#define FASTRPC_CTX_MAX (256)
#define FASTRPC_INIT_HANDLE 1
#define FASTRPC_CTXID_MASK (0xFF0)
-#define INIT_FILELEN_MAX (64 * 1024 * 1024)
+#define INIT_FILELEN_MAX (2 * 1024 * 1024)
#define INIT_MEMLEN_MAX (8 * 1024 * 1024)
#define FASTRPC_DEVICE_NAME "fastrpc"
#define ADSP_MMAP_ADD_PAGES 0x1000
--
2.23.0

2019-09-13 16:41:34

by Jorge Ramirez-Ortiz

[permalink] [raw]
Subject: [PATCH 1/5] misc: fastrpc: add mmap/unmap support

Support the allocation/deallocation of buffers mapped to the DSP.

When the memory mapped to the DSP at process creation is not enough,
the fastrpc library can extend it at runtime. This avoids having to do
large preallocations by default.

Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Reviewed-by: Srinivas Kandagatla <[email protected]>
---
drivers/misc/fastrpc.c | 181 ++++++++++++++++++++++++++++++++++++
include/uapi/misc/fastrpc.h | 15 +++
2 files changed, 196 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 98603e235cf0..8903388993d3 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -35,6 +35,7 @@
#define INIT_FILELEN_MAX (64 * 1024 * 1024)
#define INIT_MEMLEN_MAX (8 * 1024 * 1024)
#define FASTRPC_DEVICE_NAME "fastrpc"
+#define ADSP_MMAP_ADD_PAGES 0x1000

/* Retrives number of input buffers from the scalars parameter */
#define REMOTE_SCALARS_INBUFS(sc) (((sc) >> 16) & 0x0ff)
@@ -67,6 +68,8 @@
/* Remote Method id table */
#define FASTRPC_RMID_INIT_ATTACH 0
#define FASTRPC_RMID_INIT_RELEASE 1
+#define FASTRPC_RMID_INIT_MMAP 4
+#define FASTRPC_RMID_INIT_MUNMAP 5
#define FASTRPC_RMID_INIT_CREATE 6
#define FASTRPC_RMID_INIT_CREATE_ATTR 7
#define FASTRPC_RMID_INIT_CREATE_STATIC 8
@@ -90,6 +93,23 @@ struct fastrpc_remote_arg {
u64 len;
};

+struct fastrpc_mmap_rsp_msg {
+ u64 vaddr;
+};
+
+struct fastrpc_mmap_req_msg {
+ s32 pgid;
+ u32 flags;
+ u64 vaddr;
+ s32 num;
+};
+
+struct fastrpc_munmap_req_msg {
+ s32 pgid;
+ u64 vaddr;
+ u64 size;
+};
+
struct fastrpc_msg {
int pid; /* process group id */
int tid; /* thread id */
@@ -124,6 +144,9 @@ struct fastrpc_buf {
/* Lock for dma buf attachments */
struct mutex lock;
struct list_head attachments;
+ /* mmap support */
+ struct list_head node; /* list of user requested mmaps */
+ uintptr_t raddr;
};

struct fastrpc_dma_buf_attachment {
@@ -192,6 +215,7 @@ struct fastrpc_user {
struct list_head user;
struct list_head maps;
struct list_head pending;
+ struct list_head mmaps;

struct fastrpc_channel_ctx *cctx;
struct fastrpc_session_ctx *sctx;
@@ -269,6 +293,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
return -ENOMEM;

INIT_LIST_HEAD(&buf->attachments);
+ INIT_LIST_HEAD(&buf->node);
mutex_init(&buf->lock);

buf->fl = fl;
@@ -276,6 +301,7 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
buf->phys = 0;
buf->size = size;
buf->dev = dev;
+ buf->raddr = 0;

buf->virt = dma_alloc_coherent(dev, buf->size, (dma_addr_t *)&buf->phys,
GFP_KERNEL);
@@ -1098,6 +1124,7 @@ static int fastrpc_device_release(struct inode *inode, struct file *file)
struct fastrpc_channel_ctx *cctx = fl->cctx;
struct fastrpc_invoke_ctx *ctx, *n;
struct fastrpc_map *map, *m;
+ struct fastrpc_buf *buf, *b;
unsigned long flags;

fastrpc_release_current_dsp_process(fl);
@@ -1119,6 +1146,11 @@ static int fastrpc_device_release(struct inode *inode, struct file *file)
fastrpc_map_put(map);
}

+ list_for_each_entry_safe(buf, b, &fl->mmaps, node) {
+ list_del(&buf->node);
+ fastrpc_buf_free(buf);
+ }
+
fastrpc_session_free(cctx, fl->sctx);

mutex_destroy(&fl->mutex);
@@ -1143,6 +1175,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
mutex_init(&fl->mutex);
INIT_LIST_HEAD(&fl->pending);
INIT_LIST_HEAD(&fl->maps);
+ INIT_LIST_HEAD(&fl->mmaps);
INIT_LIST_HEAD(&fl->user);
fl->tgid = current->tgid;
fl->cctx = cctx;
@@ -1270,6 +1303,148 @@ static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp)
return err;
}

+static int fastrpc_req_munmap_impl(struct fastrpc_user *fl,
+ struct fastrpc_req_munmap *req)
+{
+ struct fastrpc_invoke_args args[1] = { [0] = { 0 } };
+ struct fastrpc_buf *buf, *b;
+ struct fastrpc_munmap_req_msg req_msg;
+ struct device *dev = fl->sctx->dev;
+ int err;
+ u32 sc;
+
+ spin_lock(&fl->lock);
+ list_for_each_entry_safe(buf, b, &fl->mmaps, node) {
+ if ((buf->raddr == req->vaddrout) && (buf->size == req->size))
+ break;
+ buf = NULL;
+ }
+ spin_unlock(&fl->lock);
+
+ if (!buf) {
+ dev_err(dev, "mmap not in list\n");
+ return -EINVAL;
+ }
+
+ req_msg.pgid = fl->tgid;
+ req_msg.size = buf->size;
+ req_msg.vaddr = buf->raddr;
+
+ args[0].ptr = (u64) (uintptr_t) &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]);
+ if (!err) {
+ dev_dbg(dev, "unmmap\tpt 0x%09lx OK\n", buf->raddr);
+ spin_lock(&fl->lock);
+ list_del(&buf->node);
+ spin_unlock(&fl->lock);
+ fastrpc_buf_free(buf);
+ } else {
+ dev_err(dev, "unmmap\tpt 0x%09lx ERROR\n", buf->raddr);
+ }
+
+ return err;
+}
+
+static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
+{
+ struct fastrpc_req_munmap req;
+
+ if (copy_from_user(&req, argp, sizeof(req)))
+ return -EFAULT;
+
+ return fastrpc_req_munmap_impl(fl, &req);
+}
+
+static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
+{
+ 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_req_munmap req_unmap;
+ 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;
+
+ if (req.flags != ADSP_MMAP_ADD_PAGES) {
+ dev_err(dev, "flag not supported 0x%x\n", req.flags);
+ return -EINVAL;
+ }
+
+ if (req.vaddrin) {
+ dev_err(dev, "adding user allocated pages is not supported\n");
+ return -EINVAL;
+ }
+
+ err = fastrpc_buf_alloc(fl, fl->sctx->dev, req.size, &buf);
+ if (err) {
+ dev_err(dev, "failed to allocate buffer\n");
+ return err;
+ }
+
+ req_msg.pgid = fl->tgid;
+ req_msg.flags = req.flags;
+ req_msg.vaddr = req.vaddrin;
+ req_msg.num = sizeof(pages);
+
+ args[0].ptr = (u64) &req_msg;
+ args[0].length = sizeof(req_msg);
+
+ pages.addr = buf->phys;
+ pages.size = buf->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(dev, "mmap error (len 0x%08llx)\n", buf->size);
+ goto err_invoke;
+ }
+
+ /* update the buffer to be able to deallocate the memory on the DSP */
+ buf->raddr = (uintptr_t) rsp_msg.vaddr;
+
+ /* let the client know the address to use */
+ req.vaddrout = rsp_msg.vaddr;
+
+ spin_lock(&fl->lock);
+ list_add_tail(&buf->node, &fl->mmaps);
+ spin_unlock(&fl->lock);
+
+ if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
+ /* unmap the memory and release the buffer */
+ req_unmap.vaddrout = buf->raddr;
+ req_unmap.size = buf->size;
+ fastrpc_req_munmap_impl(fl, &req_unmap);
+ return -EFAULT;
+ }
+
+ dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
+ buf->raddr, buf->size);
+
+ return 0;
+
+err_invoke:
+ fastrpc_buf_free(buf);
+
+ return err;
+}
+
static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -1293,6 +1468,12 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
case FASTRPC_IOCTL_ALLOC_DMA_BUFF:
err = fastrpc_dmabuf_alloc(fl, argp);
break;
+ case FASTRPC_IOCTL_MMAP:
+ err = fastrpc_req_mmap(fl, argp);
+ break;
+ case FASTRPC_IOCTL_MUNMAP:
+ err = fastrpc_req_munmap(fl, argp);
+ break;
default:
err = -ENOTTY;
break;
diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
index fb792e882cef..07de2b7aac85 100644
--- a/include/uapi/misc/fastrpc.h
+++ b/include/uapi/misc/fastrpc.h
@@ -10,6 +10,8 @@
#define FASTRPC_IOCTL_INVOKE _IOWR('R', 3, struct fastrpc_invoke)
#define FASTRPC_IOCTL_INIT_ATTACH _IO('R', 4)
#define FASTRPC_IOCTL_INIT_CREATE _IOWR('R', 5, struct fastrpc_init_create)
+#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct fastrpc_req_mmap)
+#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct fastrpc_req_munmap)

struct fastrpc_invoke_args {
__u64 ptr;
@@ -38,4 +40,17 @@ struct fastrpc_alloc_dma_buf {
__u64 size; /* size */
};

+struct fastrpc_req_mmap {
+ __s32 fd;
+ __u32 flags; /* flags for dsp to map with */
+ __u64 vaddrin; /* optional virtual address */
+ __u64 size; /* size */
+ __u64 vaddrout; /* dsp virtual address */
+};
+
+struct fastrpc_req_munmap {
+ __u64 vaddrout; /* address to unmap */
+ __u64 size; /* size */
+};
+
#endif /* __QCOM_FASTRPC_H__ */
--
2.23.0

2019-09-13 16:42:10

by Jorge Ramirez-Ortiz

[permalink] [raw]
Subject: [PATCH 3/5] misc: fastrpc: do not interrupt kernel calls

the DSP firmware requires some calls to be held until processing has
completed: this is to guarantee that memory continues to be
accessible.

Nevertheless, the fastrpc driver chooses not support the case were
requests need to be held for unbounded amounts of time. If such a
use-case becomes necessary, this timeout will need to be revisited.

Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Reviewed-by: Srinivas Kandagatla <[email protected]>
---
drivers/misc/fastrpc.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index bc03500bfe60..d2b639dfc461 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -927,8 +927,13 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
if (err)
goto bail;

- /* Wait for remote dsp to respond or time out */
- err = wait_for_completion_interruptible(&ctx->work);
+ if (kernel) {
+ if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
+ err = -ETIMEDOUT;
+ } else {
+ err = wait_for_completion_interruptible(&ctx->work);
+ }
+
if (err)
goto bail;

--
2.23.0

2019-09-13 19:12:25

by Jorge Ramirez-Ortiz

[permalink] [raw]
Subject: [PATCH 4/5] misc: fastrpc: handle interrupted contexts

Buffers owned by a context that has been interrupted either by a
signal or a timeout might still be being accessed by the DSP.

delegate returning the associated memory to a later time when the
device is released.

Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
Reviewed-by: Srinivas Kandagatla <[email protected]>
---
drivers/misc/fastrpc.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index d2b639dfc461..40b48db032b5 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -952,12 +952,13 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
}

bail:
- /* We are done with this compute context, remove it from pending list */
- spin_lock(&fl->lock);
- list_del(&ctx->node);
- spin_unlock(&fl->lock);
- fastrpc_context_put(ctx);
-
+ if (err != -ERESTARTSYS && err != -ETIMEDOUT) {
+ /* We are done with this compute context */
+ spin_lock(&fl->lock);
+ list_del(&ctx->node);
+ spin_unlock(&fl->lock);
+ fastrpc_context_put(ctx);
+ }
if (err)
dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err);

--
2.23.0

2019-09-16 15:52:23

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/5] misc: fastrpc: add mmap/unmap support

Hi Jorge,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3 next-20190915]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Jorge-Ramirez-Ortiz/misc-fastrpc-fixes-and-map-unmap-support/20190916-150416
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=arm

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/misc/fastrpc.c: In function 'fastrpc_req_mmap':
>> drivers/misc/fastrpc.c:1399:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
args[0].ptr = (u64) &req_msg;
^
drivers/misc/fastrpc.c:1405:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
args[1].ptr = (u64) &pages;
^
drivers/misc/fastrpc.c:1408:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
args[2].ptr = (u64) &rsp_msg;
^

vim +1399 drivers/misc/fastrpc.c

1361
1362 static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
1363 {
1364 struct fastrpc_invoke_args args[3] = { [0 ... 2] = { 0 } };
1365 struct fastrpc_buf *buf = NULL;
1366 struct fastrpc_mmap_req_msg req_msg;
1367 struct fastrpc_mmap_rsp_msg rsp_msg;
1368 struct fastrpc_req_munmap req_unmap;
1369 struct fastrpc_phy_page pages;
1370 struct fastrpc_req_mmap req;
1371 struct device *dev = fl->sctx->dev;
1372 int err;
1373 u32 sc;
1374
1375 if (copy_from_user(&req, argp, sizeof(req)))
1376 return -EFAULT;
1377
1378 if (req.flags != ADSP_MMAP_ADD_PAGES) {
1379 dev_err(dev, "flag not supported 0x%x\n", req.flags);
1380 return -EINVAL;
1381 }
1382
1383 if (req.vaddrin) {
1384 dev_err(dev, "adding user allocated pages is not supported\n");
1385 return -EINVAL;
1386 }
1387
1388 err = fastrpc_buf_alloc(fl, fl->sctx->dev, req.size, &buf);
1389 if (err) {
1390 dev_err(dev, "failed to allocate buffer\n");
1391 return err;
1392 }
1393
1394 req_msg.pgid = fl->tgid;
1395 req_msg.flags = req.flags;
1396 req_msg.vaddr = req.vaddrin;
1397 req_msg.num = sizeof(pages);
1398
> 1399 args[0].ptr = (u64) &req_msg;
1400 args[0].length = sizeof(req_msg);
1401
1402 pages.addr = buf->phys;
1403 pages.size = buf->size;
1404
1405 args[1].ptr = (u64) &pages;
1406 args[1].length = sizeof(pages);
1407
1408 args[2].ptr = (u64) &rsp_msg;
1409 args[2].length = sizeof(rsp_msg);
1410
1411 sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1);
1412 err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
1413 &args[0]);
1414 if (err) {
1415 dev_err(dev, "mmap error (len 0x%08llx)\n", buf->size);
1416 goto err_invoke;
1417 }
1418
1419 /* update the buffer to be able to deallocate the memory on the DSP */
1420 buf->raddr = (uintptr_t) rsp_msg.vaddr;
1421
1422 /* let the client know the address to use */
1423 req.vaddrout = rsp_msg.vaddr;
1424
1425 spin_lock(&fl->lock);
1426 list_add_tail(&buf->node, &fl->mmaps);
1427 spin_unlock(&fl->lock);
1428
1429 if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
1430 /* unmap the memory and release the buffer */
1431 req_unmap.vaddrout = buf->raddr;
1432 req_unmap.size = buf->size;
1433 fastrpc_req_munmap_impl(fl, &req_unmap);
1434 return -EFAULT;
1435 }
1436
1437 dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
1438 buf->raddr, buf->size);
1439
1440 return 0;
1441
1442 err_invoke:
1443 fastrpc_buf_free(buf);
1444
1445 return err;
1446 }
1447

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (4.35 kB)
.config.gz (69.71 kB)
Download all attachments