This patchset adds audiopd support to fastrpc.
The first version is here:
https://lore.kernel.org/all/[email protected]/
Changes since v1:
* dropped the patch 13:
"misc: fastrpc: Remove unnecessary if braces in fastrpc_internal_invoke"
* sent patches 1, 2 and 3 as a separate patchset
Abel Vesa (10):
misc: fastrpc: Rename audio protection domain to root
misc: fastrpc: Add reserved mem support
dt-bindings: misc: fastrpc: Document memory-region property
misc: fastrpc: Add fastrpc_remote_heap_alloc
misc: fastrpc: Use fastrpc_map_put in fastrpc_map_create on fail
misc: fastrpc: Rework fastrpc_req_munmap
misc: fastrpc: Add support for audiopd
misc: fastrpc: Safekeep mmaps on interrupted invoke
misc: fastrpc: Add mmap request assigning for static PD pool
misc: fastrpc: Add dma_mask to fastrpc_channel_ctx
.../devicetree/bindings/misc/qcom,fastrpc.txt | 5 +
drivers/misc/fastrpc.c | 267 +++++++++++++++---
include/uapi/misc/fastrpc.h | 7 +
3 files changed, 247 insertions(+), 32 deletions(-)
--
2.34.1
The AUDIO_PD will be done via static pd, so the proper name here is
actually ROOT_PD.
Co-developed-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 58654d394d17..8d803ee33904 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -83,7 +83,7 @@
#define FASTRPC_RMID_INIT_MEM_UNMAP 11
/* Protection Domain(PD) ids */
-#define AUDIO_PD (0) /* also GUEST_OS PD? */
+#define ROOT_PD (0) /* also GUEST_OS PD? */
#define USER_PD (1)
#define SENSORS_PD (2)
@@ -1889,7 +1889,7 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
err = fastrpc_invoke(fl, argp);
break;
case FASTRPC_IOCTL_INIT_ATTACH:
- err = fastrpc_init_attach(fl, AUDIO_PD);
+ err = fastrpc_init_attach(fl, ROOT_PD);
break;
case FASTRPC_IOCTL_INIT_ATTACH_SNS:
err = fastrpc_init_attach(fl, SENSORS_PD);
--
2.34.1
Add memory-region property to the list of optional properties, specify
the value type and a definition
Signed-off-by: Abel Vesa <[email protected]>
---
Documentation/devicetree/bindings/misc/qcom,fastrpc.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt b/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
index 5ec124b138a6..3dd02aaa7ba7 100644
--- a/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
+++ b/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
@@ -17,6 +17,11 @@ other tasks.
Definition: should specify the dsp domain name this fastrpc
corresponds to. must be one of this: "adsp", "mdsp", "sdsp", "cdsp"
+- memory-region:
+ Usage: optional
+ Value type: <phandle>
+ Definition: reference to the reserved-memory for the region
+
- qcom,non-secure-domain:
Usage: required
Value type: <boolean>
--
2.34.1
Split fastrpc_buf_alloc in such a way it allows allocation of remote
heap too and add fastrpc_remote_heap_alloc to do so.
Co-developed-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 52271f51800d..6730aa324e10 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -379,7 +379,7 @@ static void fastrpc_buf_free(struct fastrpc_buf *buf)
kfree(buf);
}
-static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
+static int __fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
u64 size, struct fastrpc_buf **obuf)
{
struct fastrpc_buf *buf;
@@ -407,14 +407,37 @@ static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
return -ENOMEM;
}
+ *obuf = buf;
+
+ return 0;
+}
+
+static int fastrpc_buf_alloc(struct fastrpc_user *fl, struct device *dev,
+ u64 size, struct fastrpc_buf **obuf)
+{
+ int ret;
+ struct fastrpc_buf *buf;
+
+ ret = __fastrpc_buf_alloc(fl, dev, size, obuf);
+ if (ret)
+ return ret;
+
+ buf = *obuf;
+
if (fl->sctx && fl->sctx->sid)
buf->phys += ((u64)fl->sctx->sid << 32);
- *obuf = buf;
-
return 0;
}
+static int fastrpc_remote_heap_alloc(struct fastrpc_user *fl, struct device *dev,
+ u64 size, struct fastrpc_buf **obuf)
+{
+ struct device *rdev = &fl->cctx->rpdev->dev;
+
+ return __fastrpc_buf_alloc(fl, rdev, size, obuf);
+}
+
static void fastrpc_channel_ctx_free(struct kref *ref)
{
struct fastrpc_channel_ctx *cctx;
--
2.34.1
If the mmap request is to add pages and thre are VMIDs associated with
that context, do a call to SCM to reassign that memory. Do not do this
for remote heap allocation, that is done on init create static process
only.
Co-developed-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 41eabdf0a256..66dc71e20e4f 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1849,8 +1849,9 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
if (copy_from_user(&req, argp, sizeof(req)))
return -EFAULT;
- if (req.flags != ADSP_MMAP_ADD_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);
+
return -EINVAL;
}
@@ -1896,6 +1897,22 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
/* let the client know the address to use */
req.vaddrout = rsp_msg.vaddr;
+ /* Add memory to static PD pool, protection thru hypervisor */
+ if (req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
+ struct qcom_scm_vmperm perm;
+ int err = 0;
+
+ perm.vmid = QCOM_SCM_VMID_HLOS;
+ perm.perm = QCOM_SCM_PERM_RWX;
+ err = qcom_scm_assign_mem(buf->phys, buf->size,
+ &(fl->cctx->vmperms[0].vmid), &perm, 1);
+ if (err) {
+ dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
+ buf->phys, buf->size, err);
+ goto err_assign;
+ }
+ }
+
spin_lock(&fl->lock);
list_add_tail(&buf->node, &fl->mmaps);
spin_unlock(&fl->lock);
--
2.34.1
dma_set_mask_and_coherent only updates the mask to which the device
dma_mask pointer points to. Add a dma_mask to the channel ctx and set
the device dma_mask to point to that, otherwise the dma_set_mask will
return an error and the dma_set_coherent_mask will be skipped too.
Co-developed-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 66dc71e20e4f..ce0acc7a2986 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -278,6 +278,7 @@ struct fastrpc_channel_ctx {
struct list_head invoke_interrupted_mmaps;
bool secure;
bool unsigned_support;
+ u64 dma_mask;
};
struct fastrpc_device {
@@ -2309,6 +2310,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
kref_init(&data->refcount);
dev_set_drvdata(&rpdev->dev, data);
+ rdev->dma_mask = &data->dma_mask;
dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32));
INIT_LIST_HEAD(&data->users);
INIT_LIST_HEAD(&data->invoke_interrupted_mmaps);
--
2.34.1
In order to be able to start the adsp listener for audiopd using adsprpcd,
we need to add the corresponding ioctl for creating a static process.
On that ioctl call we need to allocate the heap. Allocating the heap needs
to be happening only once and needs to be kept between different device
open calls, so attach it to the channel context to make sure that remains
until the RPMSG driver is removed. Then, if there are any VMIDs associated
with the static ADSP process, do a call to SCM to assign it.
And then, send all the necessary info related to heap to the DSP.
Co-developed-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 126 ++++++++++++++++++++++++++++++++++++
include/uapi/misc/fastrpc.h | 7 ++
2 files changed, 133 insertions(+)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 7c364c58e379..2c656da4ca5e 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -37,8 +37,20 @@
#define FASTRPC_DSP_UTILITIES_HANDLE 2
#define FASTRPC_CTXID_MASK (0xFF0)
#define INIT_FILELEN_MAX (2 * 1024 * 1024)
+#define INIT_FILE_NAMELEN_MAX (128)
#define FASTRPC_DEVICE_NAME "fastrpc"
+
+/* Add memory to static PD pool, protection thru XPU */
+#define ADSP_MMAP_HEAP_ADDR 4
+/* MAP static DMA buffer on DSP User PD */
+#define ADSP_MMAP_DMA_BUFFER 6
+/* Add memory to static PD pool protection thru hypervisor */
+#define ADSP_MMAP_REMOTE_HEAP_ADDR 8
+/* Add memory to userPD pool, for user heap */
#define ADSP_MMAP_ADD_PAGES 0x1000
+/* Add memory to userPD pool, for LLC heap */
+#define ADSP_MMAP_ADD_PAGES_LLC 0x3000,
+
#define DSP_UNSUPPORTED_API (0x80000414)
/* MAX NUMBER of DSP ATTRIBUTES SUPPORTED */
#define FASTRPC_MAX_DSP_ATTRIBUTES (256)
@@ -72,6 +84,7 @@
FASTRPC_BUILD_SCALARS(0, method, in, out, 0, 0)
#define FASTRPC_CREATE_PROCESS_NARGS 6
+#define FASTRPC_CREATE_STATIC_PROCESS_NARGS 3
/* Remote Method id table */
#define FASTRPC_RMID_INIT_ATTACH 0
#define FASTRPC_RMID_INIT_RELEASE 1
@@ -261,6 +274,7 @@ struct fastrpc_channel_ctx {
u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
struct fastrpc_device *secure_fdevice;
struct fastrpc_device *fdevice;
+ struct fastrpc_buf *remote_heap;
bool secure;
bool unsigned_support;
};
@@ -1167,6 +1181,7 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
spin_unlock(&fl->lock);
fastrpc_context_put(ctx);
}
+
if (err)
dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err);
@@ -1191,6 +1206,111 @@ static bool is_session_rejected(struct fastrpc_user *fl, bool unsigned_pd_reques
return false;
}
+static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
+ char __user *argp)
+{
+ struct fastrpc_init_create_static init;
+ struct fastrpc_invoke_args *args;
+ struct fastrpc_phy_page pages[1];
+ char *name;
+ int err;
+ struct {
+ int pgid;
+ u32 namelen;
+ u32 pageslen;
+ } inbuf;
+ u32 sc;
+
+ args = kcalloc(FASTRPC_CREATE_STATIC_PROCESS_NARGS, sizeof(*args), GFP_KERNEL);
+ if (!args)
+ return -ENOMEM;
+
+ if (copy_from_user(&init, argp, sizeof(init))) {
+ err = -EFAULT;
+ goto err;
+ }
+
+ if (init.namelen > INIT_FILE_NAMELEN_MAX) {
+ err = -EINVAL;
+ goto err;
+ }
+
+ name = kzalloc(init.namelen, GFP_KERNEL);
+ if (!name) {
+ err = -ENOMEM;
+ goto err;
+ }
+
+ if (copy_from_user(name, (void __user *)(uintptr_t)init.name, init.namelen)) {
+ err = -EFAULT;
+ goto err_name;
+ }
+
+ if (!fl->cctx->remote_heap) {
+ err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
+ &fl->cctx->remote_heap);
+ if (err)
+ goto err_name;
+
+ /* Map if we have any heap VMIDs associated with this ADSP Static Process. */
+ if (fl->cctx->vmcount) {
+ unsigned int perms = BIT(QCOM_SCM_VMID_HLOS);
+
+ dev_dbg(fl->sctx->dev, "Assinging memory with phys 0x%llx size 0x%llx perms 0x%x, vmperms %x, vmcount %x\n",
+ fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size,
+ perms, fl->cctx->vmperms, fl->cctx->vmcount);
+ err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
+ (u64)fl->cctx->remote_heap->size, &perms,
+ fl->cctx->vmperms, fl->cctx->vmcount);
+ if (err) {
+ dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d",
+ fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
+ goto err_map;
+ }
+ }
+ }
+
+ inbuf.pgid = fl->tgid;
+ inbuf.namelen = init.namelen;
+ inbuf.pageslen = 0;
+ fl->pd = USER_PD;
+
+ args[0].ptr = (u64)(uintptr_t)&inbuf;
+ args[0].length = sizeof(inbuf);
+ args[0].fd = -1;
+
+ args[1].ptr = (u64)(uintptr_t)name;
+ args[1].length = inbuf.namelen;
+ args[1].fd = -1;
+
+ pages[0].addr = fl->cctx->remote_heap->phys;
+ pages[0].size = fl->cctx->remote_heap->size;
+
+ args[2].ptr = (u64)(uintptr_t) pages;
+ args[2].length = sizeof(*pages);
+ args[2].fd = -1;
+
+ sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0);
+
+ err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
+ sc, args);
+ if (err)
+ goto err_invoke;
+
+ kfree(args);
+
+ return 0;
+err_invoke:
+err_map:
+ fastrpc_buf_free(fl->cctx->remote_heap);
+err_name:
+ kfree(name);
+err:
+ kfree(args);
+
+ return err;
+}
+
static int fastrpc_init_create_process(struct fastrpc_user *fl,
char __user *argp)
{
@@ -1918,6 +2038,9 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
case FASTRPC_IOCTL_INIT_ATTACH_SNS:
err = fastrpc_init_attach(fl, SENSORS_PD);
break;
+ case FASTRPC_IOCTL_INIT_CREATE_STATIC:
+ err = fastrpc_init_create_static_process(fl, argp);
+ break;
case FASTRPC_IOCTL_INIT_CREATE:
err = fastrpc_init_create_process(fl, argp);
break;
@@ -2183,6 +2306,9 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
if (cctx->secure_fdevice)
misc_deregister(&cctx->secure_fdevice->miscdev);
+ if (cctx->remote_heap)
+ fastrpc_buf_free(cctx->remote_heap);
+
of_platform_depopulate(&rpdev->dev);
cctx->rpdev = NULL;
diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
index 5e29f2cfa42d..2cdf2f137d33 100644
--- a/include/uapi/misc/fastrpc.h
+++ b/include/uapi/misc/fastrpc.h
@@ -16,6 +16,7 @@
#define FASTRPC_IOCTL_MEM_MAP _IOWR('R', 10, struct fastrpc_mem_map)
#define FASTRPC_IOCTL_MEM_UNMAP _IOWR('R', 11, struct fastrpc_mem_unmap)
#define FASTRPC_IOCTL_GET_DSP_INFO _IOWR('R', 13, struct fastrpc_ioctl_capability)
+#define FASTRPC_IOCTL_INIT_CREATE_STATIC _IOWR('R', 15, struct fastrpc_init_create_static)
/**
* enum fastrpc_map_flags - control flags for mapping memory on DSP user process
@@ -87,6 +88,12 @@ struct fastrpc_init_create {
__u64 file; /* pointer to elf file */
};
+struct fastrpc_init_create_static {
+ __u32 namelen; /* length of pd process name */
+ __u32 memlen;
+ __u64 name; /* pd process name */
+};
+
struct fastrpc_alloc_dma_buf {
__s32 fd; /* fd */
__u32 flags; /* flags to map with */
--
2.34.1
Move the lookup of the munmap request to the fastrpc_req_munmap and pass
on only the buf to the lower level fastrpc_req_munmap_impl. That way
we can use the lower level fastrpc_req_munmap_impl on error path in
fastrpc_req_mmap to free the buf without searching for the munmap
request it belongs to.
Co-developed-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 47 +++++++++++++++++++++---------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 5eececd9b6bd..7c364c58e379 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1630,30 +1630,14 @@ 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_req_munmap *req)
+static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *buf)
{
struct fastrpc_invoke_args args[1] = { [0] = { 0 } };
- struct fastrpc_buf *buf = NULL, *iter, *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(iter, b, &fl->mmaps, node) {
- if ((iter->raddr == req->vaddrout) && (iter->size == req->size)) {
- buf = iter;
- break;
- }
- }
- 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;
@@ -1679,12 +1663,29 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl,
static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp)
{
+ struct fastrpc_buf *buf = NULL, *iter, *b;
struct fastrpc_req_munmap req;
+ struct device *dev = fl->sctx->dev;
if (copy_from_user(&req, argp, sizeof(req)))
return -EFAULT;
- return fastrpc_req_munmap_impl(fl, &req);
+ spin_lock(&fl->lock);
+ list_for_each_entry_safe(iter, b, &fl->mmaps, node) {
+ if ((iter->raddr == req.vaddrout) && (iter->size == req.size)) {
+ buf = iter;
+ break;
+ }
+ }
+ spin_unlock(&fl->lock);
+
+ if (!buf) {
+ dev_err(dev, "mmap\t\tpt 0x%09llx [len 0x%08llx] not in list\n",
+ req.vaddrout, req.size);
+ return -EINVAL;
+ }
+
+ return fastrpc_req_munmap_impl(fl, buf);
}
static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
@@ -1693,7 +1694,6 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
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;
@@ -1755,11 +1755,8 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
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;
+ err = -EFAULT;
+ goto err_assign;
}
dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
@@ -1767,6 +1764,8 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
return 0;
+err_assign:
+ fastrpc_req_munmap_impl(fl, buf);
err_invoke:
fastrpc_buf_free(buf);
--
2.34.1
Move the kref_init right after the allocation so that we can use
fastrpc_map_put on any following error case.
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 6730aa324e10..5eececd9b6bd 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -745,6 +745,8 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
return -ENOMEM;
INIT_LIST_HEAD(&map->node);
+ kref_init(&map->refcount);
+
map->fl = fl;
map->fd = fd;
map->buf = dma_buf_get(fd);
@@ -771,7 +773,6 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
map->size = len;
map->va = sg_virt(map->table->sgl);
map->len = len;
- kref_init(&map->refcount);
if (attr & FASTRPC_ATTR_SECUREMAP) {
/*
@@ -801,7 +802,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
attach_err:
dma_buf_put(map->buf);
get_err:
- kfree(map);
+ fastrpc_map_put(map);
return err;
}
--
2.34.1
The reserved mem support is needed for CMA heap support, which will be
used by AUDIOPD.
Co-developed-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 8d803ee33904..52271f51800d 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/qcom_scm.h>
#include <uapi/misc/fastrpc.h>
+#include <linux/of_reserved_mem.h>
#define ADSP_DOMAIN_ID (0)
#define MDSP_DOMAIN_ID (1)
@@ -2064,6 +2065,9 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
return -EINVAL;
}
+ if (of_reserved_mem_device_init_by_idx(rdev, rdev->of_node, 0))
+ dev_info(rdev, "no reserved DMA memory for FASTRPC\n");
+
vmcount = of_property_read_variable_u32_array(rdev->of_node,
"qcom,vmids", &vmids[0], 0, FASTRPC_MAX_VMIDS);
if (vmcount < 0)
--
2.34.1
If the userspace daemon is killed in the middle of an invoke (e.g.
audiopd listerner invoke), we need to skip the unmapping on device
release, otherwise the DSP will crash. So lets safekeep all the maps
only if there is in invoke interrupted, by attaching them to the channel
context (which is resident until RPMSG driver is removed), and restore
them back to the fastrpc user on the next device open call.
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/misc/fastrpc.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 2c656da4ca5e..41eabdf0a256 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -275,6 +275,7 @@ struct fastrpc_channel_ctx {
struct fastrpc_device *secure_fdevice;
struct fastrpc_device *fdevice;
struct fastrpc_buf *remote_heap;
+ struct list_head invoke_interrupted_mmaps;
bool secure;
bool unsigned_support;
};
@@ -1114,6 +1115,26 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
}
+static void fastrpc_invoke_interrupted_restore_mmaps(struct fastrpc_user *fl)
+{
+ struct fastrpc_buf *buf, *b;
+
+ list_for_each_entry_safe(buf, b, &fl->cctx->invoke_interrupted_mmaps, node) {
+ list_del(&buf->node);
+ list_add(&buf->node, &fl->mmaps);
+ }
+}
+
+static void fastrpc_invoke_interrupted_save_mmaps(struct fastrpc_user *fl)
+{
+ struct fastrpc_buf *buf, *b;
+
+ list_for_each_entry_safe(buf, b, &fl->mmaps, node) {
+ list_del(&buf->node);
+ list_add_tail(&buf->node, &fl->cctx->invoke_interrupted_mmaps);
+ }
+}
+
static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
u32 handle, u32 sc,
struct fastrpc_invoke_args *args)
@@ -1182,6 +1203,9 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
fastrpc_context_put(ctx);
}
+ if (err == -ERESTARTSYS)
+ fastrpc_invoke_interrupted_save_mmaps(fl);
+
if (err)
dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err);
@@ -1551,6 +1575,8 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
return -EBUSY;
}
+ fastrpc_invoke_interrupted_restore_mmaps(fl);
+
spin_lock_irqsave(&cctx->lock, flags);
list_add_tail(&fl->user, &cctx->users);
spin_unlock_irqrestore(&cctx->lock, flags);
@@ -2268,6 +2294,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
dev_set_drvdata(&rpdev->dev, data);
dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32));
INIT_LIST_HEAD(&data->users);
+ INIT_LIST_HEAD(&data->invoke_interrupted_mmaps);
spin_lock_init(&data->lock);
idr_init(&data->ctx_idr);
data->domain_id = domain_id;
@@ -2292,6 +2319,7 @@ static void fastrpc_notify_users(struct fastrpc_user *user)
static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
{
struct fastrpc_channel_ctx *cctx = dev_get_drvdata(&rpdev->dev);
+ struct fastrpc_buf *buf, *b;
struct fastrpc_user *user;
unsigned long flags;
@@ -2306,6 +2334,9 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
if (cctx->secure_fdevice)
misc_deregister(&cctx->secure_fdevice->miscdev);
+ list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
+ list_del(&buf->node);
+
if (cctx->remote_heap)
fastrpc_buf_free(cctx->remote_heap);
--
2.34.1
Hi Abel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on robh/for-next linus/master v6.0-rc3 next-20220901]
[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/Abel-Vesa/misc-fastrpc-Add-audiopd-support/20220902-235842
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 4ec7ac90ff399b7d9af81cc8afd430a22786c61b
config: mips-randconfig-r035-20220902 (https://download.01.org/0day-ci/archive/20220903/[email protected]/config)
compiler: mipsel-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/7036272dbe486564afd86e31ca0abf48aa1535ed
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Abel-Vesa/misc-fastrpc-Add-audiopd-support/20220902-235842
git checkout 7036272dbe486564afd86e31ca0abf48aa1535ed
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/misc/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:573,
from include/asm-generic/bug.h:22,
from arch/mips/include/asm/bug.h:42,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from include/asm-generic/preempt.h:5,
from ./arch/mips/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:55,
from include/linux/swait.h:7,
from include/linux/completion.h:12,
from drivers/misc/fastrpc.c:5:
drivers/misc/fastrpc.c: In function 'fastrpc_init_create_static_process':
>> drivers/misc/fastrpc.c:1249:48: warning: format '%x' expects argument of type 'unsigned int', but argument 7 has type 'struct qcom_scm_vmperm *' [-Wformat=]
1249 | dev_dbg(fl->sctx->dev, "Assinging memory with phys 0x%llx size 0x%llx perms 0x%x, vmperms %x, vmcount %x\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:134:29: note: in definition of macro '__dynamic_func_call'
134 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:166:9: note: in expansion of macro '_dynamic_func_call'
166 | _dynamic_func_call(fmt,__dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:30: note: in expansion of macro 'dev_fmt'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/misc/fastrpc.c:1249:25: note: in expansion of macro 'dev_dbg'
1249 | dev_dbg(fl->sctx->dev, "Assinging memory with phys 0x%llx size 0x%llx perms 0x%x, vmperms %x, vmcount %x\n",
| ^~~~~~~
drivers/misc/fastrpc.c:1249:116: note: format string is defined here
1249 | dev_dbg(fl->sctx->dev, "Assinging memory with phys 0x%llx size 0x%llx perms 0x%x, vmperms %x, vmcount %x\n",
| ~^
| |
| unsigned int
vim +1249 drivers/misc/fastrpc.c
1198
1199 static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
1200 char __user *argp)
1201 {
1202 struct fastrpc_init_create_static init;
1203 struct fastrpc_invoke_args *args;
1204 struct fastrpc_phy_page pages[1];
1205 char *name;
1206 int err;
1207 struct {
1208 int pgid;
1209 u32 namelen;
1210 u32 pageslen;
1211 } inbuf;
1212 u32 sc;
1213
1214 args = kcalloc(FASTRPC_CREATE_STATIC_PROCESS_NARGS, sizeof(*args), GFP_KERNEL);
1215 if (!args)
1216 return -ENOMEM;
1217
1218 if (copy_from_user(&init, argp, sizeof(init))) {
1219 err = -EFAULT;
1220 goto err;
1221 }
1222
1223 if (init.namelen > INIT_FILE_NAMELEN_MAX) {
1224 err = -EINVAL;
1225 goto err;
1226 }
1227
1228 name = kzalloc(init.namelen, GFP_KERNEL);
1229 if (!name) {
1230 err = -ENOMEM;
1231 goto err;
1232 }
1233
1234 if (copy_from_user(name, (void __user *)(uintptr_t)init.name, init.namelen)) {
1235 err = -EFAULT;
1236 goto err_name;
1237 }
1238
1239 if (!fl->cctx->remote_heap) {
1240 err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
1241 &fl->cctx->remote_heap);
1242 if (err)
1243 goto err_name;
1244
1245 /* Map if we have any heap VMIDs associated with this ADSP Static Process. */
1246 if (fl->cctx->vmcount) {
1247 unsigned int perms = BIT(QCOM_SCM_VMID_HLOS);
1248
> 1249 dev_dbg(fl->sctx->dev, "Assinging memory with phys 0x%llx size 0x%llx perms 0x%x, vmperms %x, vmcount %x\n",
1250 fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size,
1251 perms, fl->cctx->vmperms, fl->cctx->vmcount);
1252 err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
1253 (u64)fl->cctx->remote_heap->size, &perms,
1254 fl->cctx->vmperms, fl->cctx->vmcount);
1255 if (err) {
1256 dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d",
1257 fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
1258 goto err_map;
1259 }
1260 }
1261 }
1262
1263 inbuf.pgid = fl->tgid;
1264 inbuf.namelen = init.namelen;
1265 inbuf.pageslen = 0;
1266 fl->pd = USER_PD;
1267
1268 args[0].ptr = (u64)(uintptr_t)&inbuf;
1269 args[0].length = sizeof(inbuf);
1270 args[0].fd = -1;
1271
1272 args[1].ptr = (u64)(uintptr_t)name;
1273 args[1].length = inbuf.namelen;
1274 args[1].fd = -1;
1275
1276 pages[0].addr = fl->cctx->remote_heap->phys;
1277 pages[0].size = fl->cctx->remote_heap->size;
1278
1279 args[2].ptr = (u64)(uintptr_t) pages;
1280 args[2].length = sizeof(*pages);
1281 args[2].fd = -1;
1282
1283 sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0);
1284
1285 err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
1286 sc, args);
1287 if (err)
1288 goto err_invoke;
1289
1290 kfree(args);
1291
1292 return 0;
1293 err_invoke:
1294 err_map:
1295 fastrpc_buf_free(fl->cctx->remote_heap);
1296 err_name:
1297 kfree(name);
1298 err:
1299 kfree(args);
1300
1301 return err;
1302 }
1303
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Abel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on robh/for-next linus/master v6.0-rc3 next-20220901]
[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/Abel-Vesa/misc-fastrpc-Add-audiopd-support/20220902-235842
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 4ec7ac90ff399b7d9af81cc8afd430a22786c61b
config: s390-randconfig-r033-20220902
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project c55b41d5199d2394dd6cdb8f52180d8b81d809d4)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/7036272dbe486564afd86e31ca0abf48aa1535ed
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Abel-Vesa/misc-fastrpc-Add-audiopd-support/20220902-235842
git checkout 7036272dbe486564afd86e31ca0abf48aa1535ed
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/misc/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from drivers/misc/fastrpc.c:7:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from drivers/misc/fastrpc.c:7:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from drivers/misc/fastrpc.c:7:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/misc/fastrpc.c:1251:12: warning: format specifies type 'unsigned int' but the argument has type 'struct qcom_scm_vmperm *' [-Wformat]
perms, fl->cctx->vmperms, fl->cctx->vmcount);
^~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:163:47: note: expanded from macro 'dev_dbg'
dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:129:34: note: expanded from macro 'dev_printk'
_dev_printk(level, dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
13 warnings generated.
vim +1251 drivers/misc/fastrpc.c
1198
1199 static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
1200 char __user *argp)
1201 {
1202 struct fastrpc_init_create_static init;
1203 struct fastrpc_invoke_args *args;
1204 struct fastrpc_phy_page pages[1];
1205 char *name;
1206 int err;
1207 struct {
1208 int pgid;
1209 u32 namelen;
1210 u32 pageslen;
1211 } inbuf;
1212 u32 sc;
1213
1214 args = kcalloc(FASTRPC_CREATE_STATIC_PROCESS_NARGS, sizeof(*args), GFP_KERNEL);
1215 if (!args)
1216 return -ENOMEM;
1217
1218 if (copy_from_user(&init, argp, sizeof(init))) {
1219 err = -EFAULT;
1220 goto err;
1221 }
1222
1223 if (init.namelen > INIT_FILE_NAMELEN_MAX) {
1224 err = -EINVAL;
1225 goto err;
1226 }
1227
1228 name = kzalloc(init.namelen, GFP_KERNEL);
1229 if (!name) {
1230 err = -ENOMEM;
1231 goto err;
1232 }
1233
1234 if (copy_from_user(name, (void __user *)(uintptr_t)init.name, init.namelen)) {
1235 err = -EFAULT;
1236 goto err_name;
1237 }
1238
1239 if (!fl->cctx->remote_heap) {
1240 err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
1241 &fl->cctx->remote_heap);
1242 if (err)
1243 goto err_name;
1244
1245 /* Map if we have any heap VMIDs associated with this ADSP Static Process. */
1246 if (fl->cctx->vmcount) {
1247 unsigned int perms = BIT(QCOM_SCM_VMID_HLOS);
1248
1249 dev_dbg(fl->sctx->dev, "Assinging memory with phys 0x%llx size 0x%llx perms 0x%x, vmperms %x, vmcount %x\n",
1250 fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size,
> 1251 perms, fl->cctx->vmperms, fl->cctx->vmcount);
1252 err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
1253 (u64)fl->cctx->remote_heap->size, &perms,
1254 fl->cctx->vmperms, fl->cctx->vmcount);
1255 if (err) {
1256 dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d",
1257 fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
1258 goto err_map;
1259 }
1260 }
1261 }
1262
1263 inbuf.pgid = fl->tgid;
1264 inbuf.namelen = init.namelen;
1265 inbuf.pageslen = 0;
1266 fl->pd = USER_PD;
1267
1268 args[0].ptr = (u64)(uintptr_t)&inbuf;
1269 args[0].length = sizeof(inbuf);
1270 args[0].fd = -1;
1271
1272 args[1].ptr = (u64)(uintptr_t)name;
1273 args[1].length = inbuf.namelen;
1274 args[1].fd = -1;
1275
1276 pages[0].addr = fl->cctx->remote_heap->phys;
1277 pages[0].size = fl->cctx->remote_heap->size;
1278
1279 args[2].ptr = (u64)(uintptr_t) pages;
1280 args[2].length = sizeof(*pages);
1281 args[2].fd = -1;
1282
1283 sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0);
1284
1285 err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
1286 sc, args);
1287 if (err)
1288 goto err_invoke;
1289
1290 kfree(args);
1291
1292 return 0;
1293 err_invoke:
1294 err_map:
1295 fastrpc_buf_free(fl->cctx->remote_heap);
1296 err_name:
1297 kfree(name);
1298 err:
1299 kfree(args);
1300
1301 return err;
1302 }
1303
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On 02/09/2022 18:48, Abel Vesa wrote:
> Add memory-region property to the list of optional properties, specify
> the value type and a definition
You should write why adding this property. Is it already used?
New properties can go only to DT schema, so first the conversion [1]
should be finished or started from zero (9 months it's quite a time for
a resend...).
https://lore.kernel.org/all/[email protected]/
Best regards,
Krzysztof
On 22-09-04 22:29:04, Krzysztof Kozlowski wrote:
> On 02/09/2022 18:48, Abel Vesa wrote:
> > Add memory-region property to the list of optional properties, specify
> > the value type and a definition
>
> You should write why adding this property. Is it already used?
It is not already used, will be used for cma remote heap. Will add it in
the next version.
>
> New properties can go only to DT schema, so first the conversion [1]
> should be finished or started from zero (9 months it's quite a time for
> a resend...).
>
> https://lore.kernel.org/all/[email protected]/
Right, will try to resend David's patch after addressing all the
remaining comments on that thread. Will be a separate patch.
>
>
> Best regards,
> Krzysztof
Thanks Abel,
Some minor comments.
On 02/09/2022 16:48, Abel Vesa wrote:
> In order to be able to start the adsp listener for audiopd using adsprpcd,
> we need to add the corresponding ioctl for creating a static process.
> On that ioctl call we need to allocate the heap. Allocating the heap needs
> to be happening only once and needs to be kept between different device
> open calls, so attach it to the channel context to make sure that remains
> until the RPMSG driver is removed. Then, if there are any VMIDs associated
> with the static ADSP process, do a call to SCM to assign it.
> And then, send all the necessary info related to heap to the DSP.
>
> Co-developed-by: Srinivas Kandagatla <[email protected]>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> drivers/misc/fastrpc.c | 126 ++++++++++++++++++++++++++++++++++++
> include/uapi/misc/fastrpc.h | 7 ++
> 2 files changed, 133 insertions(+)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 7c364c58e379..2c656da4ca5e 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -37,8 +37,20 @@
> #define FASTRPC_DSP_UTILITIES_HANDLE 2
> #define FASTRPC_CTXID_MASK (0xFF0)
> #define INIT_FILELEN_MAX (2 * 1024 * 1024)
> +#define INIT_FILE_NAMELEN_MAX (128)
> #define FASTRPC_DEVICE_NAME "fastrpc"
> +
> +/* Add memory to static PD pool, protection thru XPU */
> +#define ADSP_MMAP_HEAP_ADDR 4
> +/* MAP static DMA buffer on DSP User PD */
> +#define ADSP_MMAP_DMA_BUFFER 6
> +/* Add memory to static PD pool protection thru hypervisor */
> +#define ADSP_MMAP_REMOTE_HEAP_ADDR 8
> +/* Add memory to userPD pool, for user heap */
> #define ADSP_MMAP_ADD_PAGES 0x1000
> +/* Add memory to userPD pool, for LLC heap */
> +#define ADSP_MMAP_ADD_PAGES_LLC 0x3000,
> +
> #define DSP_UNSUPPORTED_API (0x80000414)
> /* MAX NUMBER of DSP ATTRIBUTES SUPPORTED */
> #define FASTRPC_MAX_DSP_ATTRIBUTES (256)
> @@ -72,6 +84,7 @@
> FASTRPC_BUILD_SCALARS(0, method, in, out, 0, 0)
>
> #define FASTRPC_CREATE_PROCESS_NARGS 6
> +#define FASTRPC_CREATE_STATIC_PROCESS_NARGS 3
> /* Remote Method id table */
> #define FASTRPC_RMID_INIT_ATTACH 0
> #define FASTRPC_RMID_INIT_RELEASE 1
> @@ -261,6 +274,7 @@ struct fastrpc_channel_ctx {
> u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
> struct fastrpc_device *secure_fdevice;
> struct fastrpc_device *fdevice;
> + struct fastrpc_buf *remote_heap;
> bool secure;
> bool unsigned_support;
> };
> @@ -1167,6 +1181,7 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
> spin_unlock(&fl->lock);
> fastrpc_context_put(ctx);
> }
> +
new line not relevant to this patch.
> if (err)
> dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err);
>
> @@ -1191,6 +1206,111 @@ static bool is_session_rejected(struct fastrpc_user *fl, bool unsigned_pd_reques
> return false;
> }
>
> +static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> + char __user *argp)
> +{
> + struct fastrpc_init_create_static init;
> + struct fastrpc_invoke_args *args;
> + struct fastrpc_phy_page pages[1];
> + char *name;
> + int err;
> + struct {
> + int pgid;
> + u32 namelen;
> + u32 pageslen;
> + } inbuf;
> + u32 sc;
> +
> + args = kcalloc(FASTRPC_CREATE_STATIC_PROCESS_NARGS, sizeof(*args), GFP_KERNEL);
> + if (!args)
> + return -ENOMEM;
> +
> + if (copy_from_user(&init, argp, sizeof(init))) {
> + err = -EFAULT;
> + goto err;
> + }
> +
> + if (init.namelen > INIT_FILE_NAMELEN_MAX) {
> + err = -EINVAL;
> + goto err;
> + }
> +
> + name = kzalloc(init.namelen, GFP_KERNEL);
> + if (!name) {
> + err = -ENOMEM;
> + goto err;
> + }
> +
> + if (copy_from_user(name, (void __user *)(uintptr_t)init.name, init.namelen)) {
> + err = -EFAULT;
> + goto err_name;
> + }
> +
> + if (!fl->cctx->remote_heap) {
> + err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
> + &fl->cctx->remote_heap);
> + if (err)
> + goto err_name;
> +
> + /* Map if we have any heap VMIDs associated with this ADSP Static Process. */
> + if (fl->cctx->vmcount) {
> + unsigned int perms = BIT(QCOM_SCM_VMID_HLOS);
> +
> + dev_dbg(fl->sctx->dev, "Assinging memory with phys 0x%llx size 0x%llx perms 0x%x, vmperms %x, vmcount %x\n",
> + fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size,
> + perms, fl->cctx->vmperms, fl->cctx->vmcount);
Please remove this debug, do we really need this?
> + err = qcom_scm_assign_mem(fl->cctx->remote_heap->phys,
> + (u64)fl->cctx->remote_heap->size, &perms,
> + fl->cctx->vmperms, fl->cctx->vmcount);
> + if (err) {
> + dev_err(fl->sctx->dev, "Failed to assign memory with phys 0x%llx size 0x%llx err %d",
> + fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> + goto err_map;
> + }
> + }
> + }
> +
> + inbuf.pgid = fl->tgid;
> + inbuf.namelen = init.namelen;
> + inbuf.pageslen = 0;
> + fl->pd = USER_PD;
> +
> + args[0].ptr = (u64)(uintptr_t)&inbuf;
> + args[0].length = sizeof(inbuf);
> + args[0].fd = -1;
> +
> + args[1].ptr = (u64)(uintptr_t)name;
> + args[1].length = inbuf.namelen;
> + args[1].fd = -1;
> +
> + pages[0].addr = fl->cctx->remote_heap->phys;
> + pages[0].size = fl->cctx->remote_heap->size;
> +
> + args[2].ptr = (u64)(uintptr_t) pages;
> + args[2].length = sizeof(*pages);
> + args[2].fd = -1;
> +
> + sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0);
> +
> + err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
> + sc, args);
> + if (err)
> + goto err_invoke;
> +
> + kfree(args);
> +
> + return 0;
> +err_invoke:
What happens to the secure mappings here, if invoke failed should we not
cleanup that part?
Or may be a not saying that we will be reusing this would be nice.
> +err_map:
> + fastrpc_buf_free(fl->cctx->remote_heap);
> +err_name:
> + kfree(name);
> +err:
> + kfree(args);
> +
> + return err;
> +}
> +
> static int fastrpc_init_create_process(struct fastrpc_user *fl,
> char __user *argp)
> {
> @@ -1918,6 +2038,9 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
> case FASTRPC_IOCTL_INIT_ATTACH_SNS:
> err = fastrpc_init_attach(fl, SENSORS_PD);
> break;
> + case FASTRPC_IOCTL_INIT_CREATE_STATIC:
> + err = fastrpc_init_create_static_process(fl, argp);
> + break;
> case FASTRPC_IOCTL_INIT_CREATE:
> err = fastrpc_init_create_process(fl, argp);
> break;
> @@ -2183,6 +2306,9 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
> if (cctx->secure_fdevice)
> misc_deregister(&cctx->secure_fdevice->miscdev);
>
> + if (cctx->remote_heap)
> + fastrpc_buf_free(cctx->remote_heap);
> +
> of_platform_depopulate(&rpdev->dev);
>
> cctx->rpdev = NULL;
> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
> index 5e29f2cfa42d..2cdf2f137d33 100644
> --- a/include/uapi/misc/fastrpc.h
> +++ b/include/uapi/misc/fastrpc.h
> @@ -16,6 +16,7 @@
> #define FASTRPC_IOCTL_MEM_MAP _IOWR('R', 10, struct fastrpc_mem_map)
> #define FASTRPC_IOCTL_MEM_UNMAP _IOWR('R', 11, struct fastrpc_mem_unmap)
> #define FASTRPC_IOCTL_GET_DSP_INFO _IOWR('R', 13, struct fastrpc_ioctl_capability)
> +#define FASTRPC_IOCTL_INIT_CREATE_STATIC _IOWR('R', 15, struct fastrpc_init_create_static)
>
> /**
> * enum fastrpc_map_flags - control flags for mapping memory on DSP user process
> @@ -87,6 +88,12 @@ struct fastrpc_init_create {
> __u64 file; /* pointer to elf file */
> };
>
> +struct fastrpc_init_create_static {
> + __u32 namelen; /* length of pd process name */
> + __u32 memlen;
> + __u64 name; /* pd process name */
> +};
> +
> struct fastrpc_alloc_dma_buf {
> __s32 fd; /* fd */
> __u32 flags; /* flags to map with */
On 02/09/2022 16:48, Abel Vesa wrote:
> If the userspace daemon is killed in the middle of an invoke (e.g.
> audiopd listerner invoke), we need to skip the unmapping on device
> release, otherwise the DSP will crash. So lets safekeep all the maps
> only if there is in invoke interrupted, by attaching them to the channel
> context (which is resident until RPMSG driver is removed), and restore
> them back to the fastrpc user on the next device open call.
>
Am bit confused with this patch.
Intention is to support interrupted invoke calls, but resorting the maps
on next open is totally something different.
if we move maps from interrupted list to compute context list on open
then these will be freed in the close.
AFAIU, to support interrupted invoke we need to check the fd and sc and
restore.
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> drivers/misc/fastrpc.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 2c656da4ca5e..41eabdf0a256 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -275,6 +275,7 @@ struct fastrpc_channel_ctx {
> struct fastrpc_device *secure_fdevice;
> struct fastrpc_device *fdevice;
> struct fastrpc_buf *remote_heap;
> + struct list_head invoke_interrupted_mmaps;
> bool secure;
> bool unsigned_support;
> };
> @@ -1114,6 +1115,26 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
>
> }
>
> +static void fastrpc_invoke_interrupted_restore_mmaps(struct fastrpc_user *fl)
> +{
> + struct fastrpc_buf *buf, *b;
> +
> + list_for_each_entry_safe(buf, b, &fl->cctx->invoke_interrupted_mmaps, node) {
> + list_del(&buf->node);
> + list_add(&buf->node, &fl->mmaps);
> + }
> +}
> +
> +static void fastrpc_invoke_interrupted_save_mmaps(struct fastrpc_user *fl)
> +{
> + struct fastrpc_buf *buf, *b;
> +
> + list_for_each_entry_safe(buf, b, &fl->mmaps, node) {
> + list_del(&buf->node);
> + list_add_tail(&buf->node, &fl->cctx->invoke_interrupted_mmaps);
> + }
> +}
> +
> static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
> u32 handle, u32 sc,
> struct fastrpc_invoke_args *args)
> @@ -1182,6 +1203,9 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
> fastrpc_context_put(ctx);
> }
>
> + if (err == -ERESTARTSYS)
> + fastrpc_invoke_interrupted_save_mmaps(fl);
> +
> if (err)
> dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err);
>
> @@ -1551,6 +1575,8 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
> return -EBUSY;
> }
>
> + fastrpc_invoke_interrupted_restore_mmaps(fl);
> +
> spin_lock_irqsave(&cctx->lock, flags);
> list_add_tail(&fl->user, &cctx->users);
> spin_unlock_irqrestore(&cctx->lock, flags);
> @@ -2268,6 +2294,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> dev_set_drvdata(&rpdev->dev, data);
> dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32));
> INIT_LIST_HEAD(&data->users);
> + INIT_LIST_HEAD(&data->invoke_interrupted_mmaps);
> spin_lock_init(&data->lock);
> idr_init(&data->ctx_idr);
> data->domain_id = domain_id;
> @@ -2292,6 +2319,7 @@ static void fastrpc_notify_users(struct fastrpc_user *user)
> static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
> {
> struct fastrpc_channel_ctx *cctx = dev_get_drvdata(&rpdev->dev);
> + struct fastrpc_buf *buf, *b;
> struct fastrpc_user *user;
> unsigned long flags;
>
> @@ -2306,6 +2334,9 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
> if (cctx->secure_fdevice)
> misc_deregister(&cctx->secure_fdevice->miscdev);
>
> + list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
> + list_del(&buf->node);
AFAIU, This list will be empty all the time, as you are moving them to
compute context list on open.
> +
> if (cctx->remote_heap)
> fastrpc_buf_free(cctx->remote_heap);
>
On 02/09/2022 16:48, Abel Vesa wrote:
> Move the kref_init right after the allocation so that we can use
> fastrpc_map_put on any following error case.
>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
Reviewed-by: Srinivas Kandagatla <[email protected]>
> drivers/misc/fastrpc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 6730aa324e10..5eececd9b6bd 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -745,6 +745,8 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
> return -ENOMEM;
>
> INIT_LIST_HEAD(&map->node);
> + kref_init(&map->refcount);
> +
> map->fl = fl;
> map->fd = fd;
> map->buf = dma_buf_get(fd);
> @@ -771,7 +773,6 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
> map->size = len;
> map->va = sg_virt(map->table->sgl);
> map->len = len;
> - kref_init(&map->refcount);
>
> if (attr & FASTRPC_ATTR_SECUREMAP) {
> /*
> @@ -801,7 +802,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
> attach_err:
> dma_buf_put(map->buf);
> get_err:
> - kfree(map);
> + fastrpc_map_put(map);
>
> return err;
> }
Hi Abel,
Thanks for picking up these patches and reworking.
On 02/09/2022 16:48, Abel Vesa wrote:
> This patchset adds audiopd support to fastrpc.
>
> The first version is here:
> https://lore.kernel.org/all/[email protected]/
>
I have tested this on sm8450 with audiopd and loading Single MIC ECNS
module to adsp.
Which platforms did you test these patches on?
Tested-by: Srinivas Kandagatla <[email protected]>
--srini
> Changes since v1:
> * dropped the patch 13:
> "misc: fastrpc: Remove unnecessary if braces in fastrpc_internal_invoke"
> * sent patches 1, 2 and 3 as a separate patchset
>
> Abel Vesa (10):
> misc: fastrpc: Rename audio protection domain to root
> misc: fastrpc: Add reserved mem support
> dt-bindings: misc: fastrpc: Document memory-region property
> misc: fastrpc: Add fastrpc_remote_heap_alloc
> misc: fastrpc: Use fastrpc_map_put in fastrpc_map_create on fail
> misc: fastrpc: Rework fastrpc_req_munmap
> misc: fastrpc: Add support for audiopd
> misc: fastrpc: Safekeep mmaps on interrupted invoke
> misc: fastrpc: Add mmap request assigning for static PD pool
> misc: fastrpc: Add dma_mask to fastrpc_channel_ctx
>
> .../devicetree/bindings/misc/qcom,fastrpc.txt | 5 +
> drivers/misc/fastrpc.c | 267 +++++++++++++++---
> include/uapi/misc/fastrpc.h | 7 +
> 3 files changed, 247 insertions(+), 32 deletions(-)
>
On 22-09-06 15:12:39, Srinivas Kandagatla wrote:
> Hi Abel,
> Thanks for picking up these patches and reworking.
>
> On 02/09/2022 16:48, Abel Vesa wrote:
> > This patchset adds audiopd support to fastrpc.
> >
> > The first version is here:
> > https://lore.kernel.org/all/[email protected]/
> >
>
> I have tested this on sm8450 with audiopd and loading Single MIC ECNS module
> to adsp.
>
> Which platforms did you test these patches on?
I have tested on sm8250, with memory-region property and vmids added
locally to the adsp fastrpc devicetree node.
>
>
> Tested-by: Srinivas Kandagatla <[email protected]>
Thanks for testing it on sm8450 too.
>
>
> --srini
>
>
> > Changes since v1:
> > * dropped the patch 13:
> > "misc: fastrpc: Remove unnecessary if braces in fastrpc_internal_invoke"
> > * sent patches 1, 2 and 3 as a separate patchset
> >
> > Abel Vesa (10):
> > misc: fastrpc: Rename audio protection domain to root
> > misc: fastrpc: Add reserved mem support
> > dt-bindings: misc: fastrpc: Document memory-region property
> > misc: fastrpc: Add fastrpc_remote_heap_alloc
> > misc: fastrpc: Use fastrpc_map_put in fastrpc_map_create on fail
> > misc: fastrpc: Rework fastrpc_req_munmap
> > misc: fastrpc: Add support for audiopd
> > misc: fastrpc: Safekeep mmaps on interrupted invoke
> > misc: fastrpc: Add mmap request assigning for static PD pool
> > misc: fastrpc: Add dma_mask to fastrpc_channel_ctx
> >
> > .../devicetree/bindings/misc/qcom,fastrpc.txt | 5 +
> > drivers/misc/fastrpc.c | 267 +++++++++++++++---
> > include/uapi/misc/fastrpc.h | 7 +
> > 3 files changed, 247 insertions(+), 32 deletions(-)
> >