[v1,02/16] misc: fastrpc: Add support for unsigned PD

Message ID 20240202064039.15505-3-quic_ekangupt@quicinc.com
State New
Headers
Series Add missing features to FastRPC driver |

Commit Message

Ekansh Gupta Feb. 2, 2024, 6:40 a.m. UTC
  Unsigned PD requires more initial memory to spawn. Also most of
the memory request are allocated from userspace. Add support for
unsigned PD by increasing init memory size and handling mapping
request for cases other than DSP heap grow requests.

Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
---
 drivers/misc/fastrpc.c | 184 ++++++++++++++++++++++++++---------------
 1 file changed, 118 insertions(+), 66 deletions(-)
  

Comments

Greg KH Feb. 2, 2024, 2:43 p.m. UTC | #1
On Fri, Feb 02, 2024 at 12:10:25PM +0530, Ekansh Gupta wrote:
> Unsigned PD requires more initial memory to spawn.

What exactly is "Unsigned PD"?

And where are all of the userspace changes for this series so we can
verify they work properly?

thanks,

greg k-h
  
Ekansh Gupta Feb. 7, 2024, 8:57 a.m. UTC | #2
On 2/2/2024 8:13 PM, Greg KH wrote:
> On Fri, Feb 02, 2024 at 12:10:25PM +0530, Ekansh Gupta wrote:
>> Unsigned PD requires more initial memory to spawn.
> What exactly is "Unsigned PD"?
>
> And where are all of the userspace changes for this series so we can
> verify they work properly?
>
> thanks,
>
> greg k-h

Hi Greg,
Unsigned PDs are sandboxed DSP processes used to offload computation
workloads to the DSP. Unsigned PD have less privileges in terms of
DSP resource access as compared to Signed PD.

Unsigned PD can be enabled using userspace API:
https://git.codelinaro.org/linaro/qcomlt/fastrpc/-/blob/master/src/fastrpc_apps_user.c?ref_type=heads#L1173

For multimode invoke request(other feature) also I've shared the
userspace changes with Srini which he will be pulling to userspace
project.

Please let me know if you have any other queries.

Thanks
--ekansh
  
Dmitry Baryshkov Feb. 7, 2024, 9:52 a.m. UTC | #3
On Wed, 7 Feb 2024 at 10:57, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
>
>
> On 2/2/2024 8:13 PM, Greg KH wrote:
> > On Fri, Feb 02, 2024 at 12:10:25PM +0530, Ekansh Gupta wrote:
> >> Unsigned PD requires more initial memory to spawn.
> > What exactly is "Unsigned PD"?
> >
> > And where are all of the userspace changes for this series so we can
> > verify they work properly?
> >
> > thanks,
> >
> > greg k-h
>
> Hi Greg,
> Unsigned PDs are sandboxed DSP processes used to offload computation
> workloads to the DSP. Unsigned PD have less privileges in terms of
> DSP resource access as compared to Signed PD.

All such details must be described in commit messages for the patches.

>
> Unsigned PD can be enabled using userspace API:
> https://git.codelinaro.org/linaro/qcomlt/fastrpc/-/blob/master/src/fastrpc_apps_user.c?ref_type=heads#L1173
>
> For multimode invoke request(other feature) also I've shared the
> userspace changes with Srini which he will be pulling to userspace
> project.

And this should be a part of the cover letter.

>
> Please let me know if you have any other queries.
>
> Thanks
> --ekansh
>
>
  

Patch

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index f3ef5e194f11..bdce5469de19 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -38,7 +38,7 @@ 
 #define FASTRPC_INIT_HANDLE	1
 #define FASTRPC_DSP_UTILITIES_HANDLE	2
 #define FASTRPC_CTXID_MASK (0xFF0)
-#define INIT_FILELEN_MAX (2 * 1024 * 1024)
+#define INIT_FILELEN_MAX (5 * 1024 * 1024)
 #define INIT_FILE_NAMELEN_MAX (128)
 #define FASTRPC_DEVICE_NAME	"fastrpc"
 
@@ -303,6 +303,7 @@  struct fastrpc_user {
 	int tgid;
 	int pd;
 	bool is_secure_dev;
+	bool is_unsigned_pd;
 	/* Lock for lists */
 	spinlock_t lock;
 	/* lock for allocations */
@@ -1403,7 +1404,6 @@  static int fastrpc_init_create_process(struct fastrpc_user *fl,
 		u32 siglen;
 	} inbuf;
 	u32 sc;
-	bool unsigned_module = false;
 
 	args = kcalloc(FASTRPC_CREATE_PROCESS_NARGS, sizeof(*args), GFP_KERNEL);
 	if (!args)
@@ -1415,9 +1415,9 @@  static int fastrpc_init_create_process(struct fastrpc_user *fl,
 	}
 
 	if (init.attrs & FASTRPC_MODE_UNSIGNED_MODULE)
-		unsigned_module = true;
+		fl->is_unsigned_pd = true;
 
-	if (is_session_rejected(fl, unsigned_module)) {
+	if (is_session_rejected(fl, fl->is_unsigned_pd)) {
 		err = -ECONNREFUSED;
 		goto err;
 	}
@@ -1486,6 +1486,7 @@  static int fastrpc_init_create_process(struct fastrpc_user *fl,
 		goto err_invoke;
 
 	kfree(args);
+	fastrpc_map_put(map);
 
 	return 0;
 
@@ -1953,98 +1954,149 @@  static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
 	struct fastrpc_mmap_rsp_msg rsp_msg;
 	struct fastrpc_phy_page pages;
 	struct fastrpc_req_mmap req;
+	struct fastrpc_map *map = NULL;
 	struct device *dev = fl->sctx->dev;
 	int err;
 	u32 sc;
+	unsigned long flags;
 
 	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(dev, "flag not supported 0x%x\n", req.flags);
+	if (req.flags == ADSP_MMAP_ADD_PAGES || req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR) {
+		if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->is_unsigned_pd) {
+			dev_err(dev, "secure memory allocation is not supported in unsigned PD\n");
+			return -EINVAL;
+		}
+		if (req.vaddrin && !fl->is_unsigned_pd) {
+			dev_err(dev, "adding user allocated pages is not supported\n");
+			return -EINVAL;
+		}
 
-		return -EINVAL;
-	}
+		if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR)
+			err = fastrpc_remote_heap_alloc(fl, dev, req.size, &buf);
+		else
+			err = fastrpc_buf_alloc(fl, dev, req.size, &buf);
 
-	if (req.vaddrin) {
-		dev_err(dev, "adding user allocated pages is not supported\n");
-		return -EINVAL;
-	}
+		if (err) {
+			dev_err(dev, "failed to allocate buffer\n");
+			return err;
+		}
 
-	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR)
-		err = fastrpc_remote_heap_alloc(fl, dev, req.size, &buf);
-	else
-		err = fastrpc_buf_alloc(fl, dev, req.size, &buf);
+		req_msg.pgid = fl->tgid;
+		req_msg.flags = req.flags;
+		req_msg.vaddr = req.vaddrin;
+		req_msg.num = sizeof(pages);
 
-	if (err) {
-		dev_err(dev, "failed to allocate buffer\n");
-		return err;
-	}
+		args[0].ptr = (u64) (uintptr_t) &req_msg;
+		args[0].length = sizeof(req_msg);
 
-	req_msg.pgid = fl->tgid;
-	req_msg.flags = req.flags;
-	req_msg.vaddr = req.vaddrin;
-	req_msg.num = sizeof(pages);
+		pages.addr = buf->phys;
+		pages.size = buf->size;
 
-	args[0].ptr = (u64) (uintptr_t) &req_msg;
-	args[0].length = sizeof(req_msg);
+		args[1].ptr = (u64) (uintptr_t) &pages;
+		args[1].length = sizeof(pages);
 
-	pages.addr = buf->phys;
-	pages.size = buf->size;
+		args[2].ptr = (u64) (uintptr_t) &rsp_msg;
+		args[2].length = sizeof(rsp_msg);
 
-	args[1].ptr = (u64) (uintptr_t) &pages;
-	args[1].length = sizeof(pages);
+		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;
+		}
 
-	args[2].ptr = (u64) (uintptr_t) &rsp_msg;
-	args[2].length = sizeof(rsp_msg);
+		/* update the buffer to be able to deallocate the memory on the DSP */
+		buf->raddr = (uintptr_t) rsp_msg.vaddr;
+		buf->flag = req.flags;
 
-	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;
-	}
+		/* let the client know the address to use */
+		req.vaddrout = rsp_msg.vaddr;
 
-	/* update the buffer to be able to deallocate the memory on the DSP */
-	buf->raddr = (uintptr_t) rsp_msg.vaddr;
-	buf->flag = req.flags;
+		/* Add memory to static PD pool, protection thru hypervisor */
+		if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
+			err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
+				&fl->cctx->perms, fl->cctx->vmperms, fl->cctx->vmcount);
+			if (err) {
+				dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
+						buf->phys, buf->size, err);
+				goto err_assign;
+			}
+		}
 
-	/* let the client know the address to use */
-	req.vaddrout = rsp_msg.vaddr;
+		if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR) {
+			spin_lock_irqsave(&fl->cctx->lock, flags);
+			list_add_tail(&buf->node, &fl->cctx->rmaps);
+			spin_unlock_irqrestore(&fl->cctx->lock, flags);
+		} else {
+			spin_lock(&fl->lock);
+			list_add_tail(&buf->node, &fl->mmaps);
+			spin_unlock(&fl->lock);
+		}
 
-	/* Add memory to static PD pool, protection thru hypervisor */
-	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR && fl->cctx->vmcount) {
-		err = qcom_scm_assign_mem(buf->phys, (u64)buf->size,
-			&fl->cctx->perms, fl->cctx->vmperms, fl->cctx->vmcount);
-		if (err) {
-			dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d\n",
-					buf->phys, buf->size, err);
+		if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
+			err = -EFAULT;
 			goto err_assign;
 		}
-	}
+	} else {
+		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;
+		}
 
-	spin_lock(&fl->lock);
-	if (req.flags == ADSP_MMAP_REMOTE_HEAP_ADDR)
-		list_add_tail(&buf->node, &fl->cctx->rmaps);
-	else
-		list_add_tail(&buf->node, &fl->mmaps);
-	spin_unlock(&fl->lock);
+		req_msg.pgid = fl->tgid;
+		req_msg.flags = req.flags;
+		req_msg.vaddr = req.vaddrin;
+		req_msg.num = sizeof(pages);
 
-	if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
-		err = -EFAULT;
-		goto err_assign;
-	}
+		args[0].ptr = (u64) (uintptr_t) &req_msg;
+		args[0].length = sizeof(req_msg);
 
-	dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
-		buf->raddr, buf->size);
+		pages.addr = map->phys;
+		pages.size = map->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]);
+		if (err) {
+			dev_err(dev, "mmap error (len 0x%08llx)\n", map->size);
+			goto err_invoke;
+		}
+
+		/* update the buffer to be able to deallocate the memory on the DSP */
+		map->raddr = (uintptr_t) rsp_msg.vaddr;
+
+		/* let the client know the address to use */
+		req.vaddrout = rsp_msg.vaddr;
+
+		if (copy_to_user((void __user *)argp, &req, sizeof(req))) {
+			err = -EFAULT;
+			goto err_assign;
+		}
+	}
 	return 0;
 
 err_assign:
-	fastrpc_req_munmap_impl(fl, buf);
+	if (req.flags != ADSP_MMAP_ADD_PAGES && req.flags != ADSP_MMAP_REMOTE_HEAP_ADDR)
+		fastrpc_map_put(map);
+	else
+		fastrpc_req_munmap_impl(fl, buf);
+
 err_invoke:
-	fastrpc_buf_free(buf);
+	if (map)
+		fastrpc_map_put(map);
+	if (buf)
+		fastrpc_buf_free(buf);
 
 	return err;
 }