[Linaro-mm-sig] Re: [PATCH RFC 14/18] accel/qda: Add FastRPC dynamic invocation support

On Tue, Feb 24, 2026 at 12:39:08AM +0530, Ekansh Gupta wrote:

Extend the QDA FastRPC implementation to support dynamic remote procedure calls from userspace. A new DRM_QDA_INVOKE ioctl is added, which accepts a qda_invoke_args structure containing a remote handle, FastRPC scalars value and a pointer to an array of fastrpc_invoke_args describing the individual arguments. The driver copies the scalar and argument array into a fastrpc_invoke_context and reuses the existing buffer overlap and packing logic to build a GEM-backed message buffer for transport.

The FastRPC core gains a FASTRPC_RMID_INVOKE_DYNAMIC method type and a fastrpc_prepare_args_invoke() helper that reads the qda_invoke_args header and argument descriptors from user or kernel memory using a copy_from_user_or_kernel() helper. The generic fastrpc_prepare_args() path is updated to handle the dynamic method alongside the existing INIT_ATTACH and INIT_RELEASE control calls, deriving the number of buffers and scalars from the provided FastRPC scalars encoding.

On the transport side qda_ioctl_invoke() simply forwards the request to fastrpc_invoke() with the dynamic method id, allowing the RPMsg transport and context lookup to treat dynamic calls in the same way as the existing control methods. This patch establishes the basic FastRPC invoke mechanism on top of the QDA GEM and RPMsg infrastructure so that future patches can wire up more complex DSP APIs.

Signed-off-by: Ekansh Gupta ekansh.gupta@oss.qualcomm.com

drivers/accel/qda/qda_drv.c | 1 + drivers/accel/qda/qda_fastrpc.c | 48 +++++++++++++++++++++++++++++++++++++++++ drivers/accel/qda/qda_fastrpc.h | 1 + drivers/accel/qda/qda_ioctl.c | 5 +++++ drivers/accel/qda/qda_ioctl.h | 13 +++++++++++ include/uapi/drm/qda_accel.h | 21 ++++++++++++++++++ 6 files changed, 89 insertions(+)

diff --git a/drivers/accel/qda/qda_drv.c b/drivers/accel/qda/qda_drv.c index 3034ea660924..f94f780ea50a 100644 --- a/drivers/accel/qda/qda_drv.c +++ b/drivers/accel/qda/qda_drv.c @@ -162,6 +162,7 @@ static const struct drm_ioctl_desc qda_ioctls[] = { DRM_IOCTL_DEF_DRV(QDA_GEM_CREATE, qda_ioctl_gem_create, 0), DRM_IOCTL_DEF_DRV(QDA_GEM_MMAP_OFFSET, qda_ioctl_gem_mmap_offset, 0), DRM_IOCTL_DEF_DRV(QDA_INIT_ATTACH, qda_ioctl_attach, 0),

• DRM_IOCTL_DEF_DRV(QDA_INVOKE, qda_ioctl_invoke, 0),

}; static struct drm_driver qda_drm_driver = { diff --git a/drivers/accel/qda/qda_fastrpc.c b/drivers/accel/qda/qda_fastrpc.c index eda7c90070ee..a48b255ffb1b 100644 --- a/drivers/accel/qda/qda_fastrpc.c +++ b/drivers/accel/qda/qda_fastrpc.c @@ -12,6 +12,16 @@ #include "qda_gem.h" #include "qda_memory_manager.h" +static int copy_from_user_or_kernel(void *dst, const void __user *src, size_t size) +{

• if ((unsigned long)src >= PAGE_OFFSET) {

• memcpy(dst, src, size);
  

• return 0;
  

• } else {

• return copy_from_user(dst, src, size) ? -EFAULT : 0;
  

• }

Nah, it's a direct route to failure. __user is for user pointers, it can't be a kernel data. Define separate functions and be 100% sure whether the data is coming from the user (and thus needs to be sanitized) or of it is coming from the kernel. Otherwise a funny user can pass kernel pointer and get away with your code copying data from or writing data to the kernel buffer.

+}

static int copy_to_user_or_kernel(void __user *dst, const void *src, size_t size) { if ((unsigned long)dst >= PAGE_OFFSET) { @@ -509,6 +519,41 @@ static int fastrpc_prepare_args_release_process(struct fastrpc_invoke_context *c return 0; } +static int fastrpc_prepare_args_invoke(struct fastrpc_invoke_context *ctx, char __user *argp) +{

• struct fastrpc_invoke_args *args = NULL;
• struct qda_invoke_args inv;
• int err = 0;
• int nscalars;
• if (!argp)

• return -EINVAL;
  

• err = copy_from_user_or_kernel(&inv, argp, sizeof(inv));
• if (err)

• return err;
  

• nscalars = REMOTE_SCALARS_LENGTH(inv.sc);
• if (nscalars) {

• args = kcalloc(nscalars, sizeof(*args), GFP_KERNEL);
  

• if (!args)
  

• 	return -ENOMEM;
  

• err = copy_from_user_or_kernel(args, (const void __user *)(uintptr_t)inv.args,
  

• 			       nscalars * sizeof(*args));
  

So... You are allowing users to specify the address in the kernel address space? Are you... sure?

• if (err) {
  

• 	kfree(args);
  

• 	return err;
  

• }
  

• }
• ctx->sc = inv.sc;
• ctx->args = args;
• ctx->handle = inv.handle;
• return 0;

+}

int fastrpc_prepare_args(struct fastrpc_invoke_context *ctx, char __user *argp) { int err; @@ -521,6 +566,9 @@ int fastrpc_prepare_args(struct fastrpc_invoke_context *ctx, char __user *argp) case FASTRPC_RMID_INIT_RELEASE: err = fastrpc_prepare_args_release_process(ctx); break;

• case FASTRPC_RMID_INVOKE_DYNAMIC:

• err = fastrpc_prepare_args_invoke(ctx, argp);
  

• break;
  

  default: return -EINVAL; }

diff --git a/drivers/accel/qda/qda_fastrpc.h b/drivers/accel/qda/qda_fastrpc.h index 744421382079..bcadf9437a36 100644 --- a/drivers/accel/qda/qda_fastrpc.h +++ b/drivers/accel/qda/qda_fastrpc.h @@ -237,6 +237,7 @@ struct fastrpc_invoke_context { /* Remote Method ID table - identifies initialization and control operations */ #define FASTRPC_RMID_INIT_ATTACH 0 /* Attach to DSP session */ #define FASTRPC_RMID_INIT_RELEASE 1 /* Release DSP session */ +#define FASTRPC_RMID_INVOKE_DYNAMIC 0xFFFFFFFF /* Dynamic method invocation */ /* Common handle for initialization operations */ #define FASTRPC_INIT_HANDLE 0x1 diff --git a/drivers/accel/qda/qda_ioctl.c b/drivers/accel/qda/qda_ioctl.c index 1066ab6ddc7b..e90aceabd30d 100644 --- a/drivers/accel/qda/qda_ioctl.c +++ b/drivers/accel/qda/qda_ioctl.c @@ -192,3 +192,8 @@ int fastrpc_release_current_dsp_process(struct qda_dev *qdev, struct drm_file *f { return fastrpc_invoke(FASTRPC_RMID_INIT_RELEASE, qdev->drm_dev, NULL, file_priv); }

+int qda_ioctl_invoke(struct drm_device *dev, void *data, struct drm_file *file_priv) +{

• return fastrpc_invoke(FASTRPC_RMID_INVOKE_DYNAMIC, dev, data, file_priv);

+} diff --git a/drivers/accel/qda/qda_ioctl.h b/drivers/accel/qda/qda_ioctl.h index 044c616a51c6..e186c5183171 100644 --- a/drivers/accel/qda/qda_ioctl.h +++ b/drivers/accel/qda/qda_ioctl.h @@ -63,4 +63,17 @@ int qda_ioctl_attach(struct drm_device *dev, void *data, struct drm_file *file_p */ int fastrpc_release_current_dsp_process(struct qda_dev *qdev, struct drm_file *file_priv); +/**

• • qda_ioctl_invoke - Invoke a remote procedure on the DSP
• • @dev: DRM device structure
• • @data: User-space data containing invocation parameters
• • @file_priv: DRM file private data
• • This IOCTL handler initiates a remote procedure call on the DSP,
• • marshalling arguments, executing the call, and returning results.
• • Return: 0 on success, negative error code on failure
• */

+int qda_ioctl_invoke(struct drm_device *dev, void *data, struct drm_file *file_priv);

#endif /* _QDA_IOCTL_H */ diff --git a/include/uapi/drm/qda_accel.h b/include/uapi/drm/qda_accel.h index 4d3666c5b998..01072a9d0a91 100644 --- a/include/uapi/drm/qda_accel.h +++ b/include/uapi/drm/qda_accel.h @@ -22,6 +22,9 @@ extern "C" { #define DRM_QDA_GEM_CREATE 0x01 #define DRM_QDA_GEM_MMAP_OFFSET 0x02 #define DRM_QDA_INIT_ATTACH 0x03 +/* Indexes 0x04 to 0x06 are reserved for other requests */ +#define DRM_QDA_INVOKE 0x07

/*

• QDA IOCTL definitions

@@ -35,6 +38,8 @@ extern "C" { #define DRM_IOCTL_QDA_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_QDA_GEM_MMAP_OFFSET, \ struct drm_qda_gem_mmap_offset) #define DRM_IOCTL_QDA_INIT_ATTACH DRM_IO(DRM_COMMAND_BASE + DRM_QDA_INIT_ATTACH) +#define DRM_IOCTL_QDA_INVOKE DRM_IOWR(DRM_COMMAND_BASE + DRM_QDA_INVOKE, \

• 				 struct qda_invoke_args)
  

/**

• struct drm_qda_query - Device information query structure

@@ -95,6 +100,22 @@ struct fastrpc_invoke_args { __u32 attr; }; +/**

• • struct qda_invoke_args - User-space IOCTL arguments for invoking a function
• • @handle: Handle identifying the remote function to invoke
• • @sc: Scalars parameter encoding buffer counts and attributes

Encoding... how?

• • @args: User-space pointer to the argument array

Which is defined at...?

Can you actually write the user code by looking at your uapi header?

• • This structure is passed from user-space to invoke a remote function
• • on the DSP. The scalars parameter encodes the number and types of
• • input/output buffers.
• */

+struct qda_invoke_args {

• __u32 handle;
• __u32 sc;
• __u64 args;

+};

#if defined(__cplusplus) } #endif

-- 2.34.1