Re: [RFC PATCH 3/6] char: fastrpc: Add support for context Invoke method
On 30/11/18 16:19, Arnd Bergmann wrote:
On Fri, Nov 30, 2018 at 5:03 PM Srinivas Kandagatla
wrote:
On 30/11/18 15:08, Arnd Bergmann wrote:
On Fri, Nov 30, 2018 at 4:01 PM Srinivas Kandagatla
wrote:
Thanks Arnd for the review comments!
On 30/11/18 13:41, Arnd Bergmann wrote:
On Fri, Nov 30, 2018 at 11:48 AM Srinivas Kandagatla
wrote:
+static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
+unsigned long arg)
+{
+ struct fastrpc_user *fl = (struct fastrpc_user *)file->private_data;
+ struct fastrpc_channel_ctx *cctx = fl->cctx;
+ char __user *argp = (char __user *)arg;
+ int err;
+
+ if (!fl->sctx) {
+ fl->sctx = fastrpc_session_alloc(cctx, 0);
+ if (!fl->sctx)
+ return -ENOENT;
+ }
Shouldn't that session be allocated during open()?
Yes, and no, we do not need context in all the cases. In cases like we
just want to allocate dmabuf.
Can you give an example what that would be good for?
Currently the instance which does not need session is used as simple
memory allocator (rpcmem), TBH, this is the side effect of trying to fit
in with downstream application infrastructure which uses ion for andriod
usecases.
That does not sound like enough of a reason then, user space is
easy to change here to just allocate the memory from the device itself.
The only reason that I can see for needing a dmabuf would be if
you have to share a buffer between two instances, and then you
can use either of them.
I agree, I will try rework this and remove the instances that does not
require sessions!
Sharing buffer is also a reason for dmabuf here.
+static void fastrpc_notify_users(struct fastrpc_user *user)
+{
+ struct fastrpc_invoke_ctx *ctx, *n;will go
+
+ spin_lock(&user->lock);
+ list_for_each_entry_safe(ctx, n, &user->pending, node)
+ complete(&ctx->work);
+ spin_unlock(&user->lock);
+}
Can you explain here what it means to have multiple 'users'
a 'fastrpc_user' structure? Why are they all done at the same time?
user is allocated on every open(). Having multiple users means that
there are more than one compute sessions running on a given dsp.
They reason why all the users are notified here is because the dsp is
going down, so all the compute sessions associated with it will not see
any response from dsp, so any pending/waiting compute contexts are
explicitly notified.
I don't get it yet. What are 'compute sessions'? Do you have
multiple threads running on a single instance at the same time?
compute sessions are "compute context-banks" instances in DSP.
DSP supports multiple compute banks, Ideally 12 context banks, 4 which
are reserved for other purposes and 8 of them are used for compute, one
for each process. So ideally we can run 8 parallel computes.
I would have expected to only ever see one thread in the
'wait_for_completion()' above, and others possibly waiting
for a chance to get to but not already running.
struct fastrpc_remote_crc {
__u64 crc;
__u64 reserved1
__u64 reserved2
__u64 reserved3
};
I don't see a need to add extra served fields for structures
that are already naturally aligned here, e.g. in
fastrpc_remote_arg we need the 'reserved1' but not
the 'reserved2'.
Yes, I see, I overdone it!
Other idea, is, may be I can try to combine these into single structure
something like:
struct fastrpc_invoke_arg {
__u64 ptr;
__u64 len;
__u32 fd;
__u32 reserved1
__u64 attr;
__u64 crc;
};
struct fastrpc_ioctl_invoke {
__u32 handle;
__u32 sc;
/* The minimum size is scalar_length * 32*/
struct fastrpc_invoke_args *args;
};
That is still two structure, not one ;-)
struct fastrpc_ioctl_invoke {
__u32 handle;
__u32 sc;
/* The minimum size is scalar_length * 32 */
struct fastrpc_remote_args *rargs;
struct fastrpc_remote_fd *fds;
struct fastrpc_remote_attr *attrs;
struct fastrpc_remote_crc *crc;
};
Do these really have to be indirect then? Are they all
lists of variable length? How do you know how long?
Yes, they are variable length and will be scalar length long.
Scalar length is derived from sc variable in this structure.
Do you mean we have a variable number 'sc', but each array
always has the same length as the other ones? In that
case: yes, combining them seems sensible.
Yes thats what I meant!
The other question this raises is: what is 'handle'?
Why is the file descriptor not enough to identify the
instance we want to talk to?
This is remote handle to opened interface on which this method has to be
invoked.
For example we are running a calculator application, calculator will
have a unique handle on which calculate() method needs to be invoked.
thanks,
srini
Arnd
Re: [RFC PATCH 3/6] char: fastrpc: Add support for context Invoke method
On Fri, Nov 30, 2018 at 5:03 PM Srinivas Kandagatla
wrote:
> On 30/11/18 15:08, Arnd Bergmann wrote:
> > On Fri, Nov 30, 2018 at 4:01 PM Srinivas Kandagatla
> > wrote:
> >> Thanks Arnd for the review comments!
> >> On 30/11/18 13:41, Arnd Bergmann wrote:
> >>> On Fri, Nov 30, 2018 at 11:48 AM Srinivas Kandagatla
> >>> wrote:
> >
> +static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
> +unsigned long arg)
> +{
> + struct fastrpc_user *fl = (struct fastrpc_user
> *)file->private_data;
> + struct fastrpc_channel_ctx *cctx = fl->cctx;
> + char __user *argp = (char __user *)arg;
> + int err;
> +
> + if (!fl->sctx) {
> + fl->sctx = fastrpc_session_alloc(cctx, 0);
> + if (!fl->sctx)
> + return -ENOENT;
> + }
> >>>
> >>> Shouldn't that session be allocated during open()?
> >>>
> >> Yes, and no, we do not need context in all the cases. In cases like we
> >> just want to allocate dmabuf.
> >
> > Can you give an example what that would be good for?
> >
>
> Currently the instance which does not need session is used as simple
> memory allocator (rpcmem), TBH, this is the side effect of trying to fit
> in with downstream application infrastructure which uses ion for andriod
> usecases.
That does not sound like enough of a reason then, user space is
easy to change here to just allocate the memory from the device itself.
The only reason that I can see for needing a dmabuf would be if
you have to share a buffer between two instances, and then you
can use either of them.
> +static void fastrpc_notify_users(struct fastrpc_user *user)
> +{
> + struct fastrpc_invoke_ctx *ctx, *n;will go
> +
> + spin_lock(&user->lock);
> + list_for_each_entry_safe(ctx, n, &user->pending, node)
> + complete(&ctx->work);
> + spin_unlock(&user->lock);
> +}
> >>>
> >>> Can you explain here what it means to have multiple 'users'
> >>> a 'fastrpc_user' structure? Why are they all done at the same time?
>
> user is allocated on every open(). Having multiple users means that
> there are more than one compute sessions running on a given dsp.
>
> They reason why all the users are notified here is because the dsp is
> going down, so all the compute sessions associated with it will not see
> any response from dsp, so any pending/waiting compute contexts are
> explicitly notified.
I don't get it yet. What are 'compute sessions'? Do you have
multiple threads running on a single instance at the same time?
I would have expected to only ever see one thread in the
'wait_for_completion()' above, and others possibly waiting
for a chance to get to but not already running.
> >> struct fastrpc_remote_crc {
> >> __u64 crc;
> >> __u64 reserved1
> >> __u64 reserved2
> >> __u64 reserved3
> >> };
> >
> > I don't see a need to add extra served fields for structures
> > that are already naturally aligned here, e.g. in
> > fastrpc_remote_arg we need the 'reserved1' but not
> > the 'reserved2'.
> Yes, I see, I overdone it!
> Other idea, is, may be I can try to combine these into single structure
> something like:
>
> struct fastrpc_invoke_arg {
> __u64 ptr;
> __u64 len;
> __u32 fd;
> __u32 reserved1
> __u64 attr;
> __u64 crc;
> };
>
> struct fastrpc_ioctl_invoke {
> __u32 handle;
> __u32 sc;
> /* The minimum size is scalar_length * 32*/
> struct fastrpc_invoke_args *args;
> };
That is still two structure, not one ;-)
> >> struct fastrpc_ioctl_invoke {
> >> __u32 handle;
> >> __u32 sc;
> >> /* The minimum size is scalar_length * 32 */
> >> struct fastrpc_remote_args *rargs;
> >> struct fastrpc_remote_fd *fds;
> >> struct fastrpc_remote_attr *attrs;
> >> struct fastrpc_remote_crc *crc;
> >> };
> >
> > Do these really have to be indirect then? Are they all
> > lists of variable length? How do you know how long?
> Yes, they are variable length and will be scalar length long.
> Scalar length is derived from sc variable in this structure.
Do you mean we have a variable number 'sc', but each array
always has the same length as the other ones? In that
case: yes, combining them seems sensible.
The other question this raises is: what is 'handle'?
Why is the file descriptor not enough to identify the
instance we want to talk to?
Arnd
Re: [RFC PATCH 3/6] char: fastrpc: Add support for context Invoke method
On 30/11/18 15:08, Arnd Bergmann wrote:
On Fri, Nov 30, 2018 at 4:01 PM Srinivas Kandagatla
wrote:
Thanks Arnd for the review comments!
On 30/11/18 13:41, Arnd Bergmann wrote:
On Fri, Nov 30, 2018 at 11:48 AM Srinivas Kandagatla
wrote:
+static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
+unsigned long arg)
+{
+ struct fastrpc_user *fl = (struct fastrpc_user *)file->private_data;
+ struct fastrpc_channel_ctx *cctx = fl->cctx;
+ char __user *argp = (char __user *)arg;
+ int err;
+
+ if (!fl->sctx) {
+ fl->sctx = fastrpc_session_alloc(cctx, 0);
+ if (!fl->sctx)
+ return -ENOENT;
+ }
Shouldn't that session be allocated during open()?
Yes, and no, we do not need context in all the cases. In cases like we
just want to allocate dmabuf.
Can you give an example what that would be good for?
Currently the instance which does not need session is used as simple
memory allocator (rpcmem), TBH, this is the side effect of trying to fit
in with downstream application infrastructure which uses ion for andriod
usecases.
+static void fastrpc_notify_users(struct fastrpc_user *user)
+{
+ struct fastrpc_invoke_ctx *ctx, *n;will go
+
+ spin_lock(&user->lock);
+ list_for_each_entry_safe(ctx, n, &user->pending, node)
+ complete(&ctx->work);
+ spin_unlock(&user->lock);
+}
Can you explain here what it means to have multiple 'users'
a 'fastrpc_user' structure? Why are they all done at the same time?
user is allocated on every open(). Having multiple users means that
there are more than one compute sessions running on a given dsp.
They reason why all the users are notified here is because the dsp is
going down, so all the compute sessions associated with it will not see
any response from dsp, so any pending/waiting compute contexts are
explicitly notified.
This is the case where users need to be notified if the dsp goes down
due to crash or shut down!
What is a 'user' then? My point is that it seems to refer to two
different things here. I assume 'fastrpc_user' is whoever
has opened the file descriptor.
+struct fastrpc_ioctl_invoke {
+ uint32_t handle;/* remote handle */
+ uint32_t sc;/* scalars describing the data */
+ remote_arg_t *pra; /* remote arguments list */
+ int *fds; /* fd list */
+ unsigned int *attrs;/* attribute list */
+ unsigned int *crc;
+};
This seems too complex for an ioctl argument, with
multiple levels of pointer indirections. I'd normally
try to make each ioctl argument either a scalar, or a
structure with only fixed-length members.
I totally agree with you and many thanks for your expert inputs,
May be something like below with fixed length members would work?
struct fastrpc_remote_arg {
__u64 ptr; /* buffer ptr */
__u64 len; /* length */
__u32 fd; /* dmabuf fd */
__u32 reserved1
__u64 reserved2
};
struct fastrpc_remote_fd {
__u64 fd;
__u64 reserved1
__u64 reserved2
__u64 reserved3
};
struct fastrpc_remote_attr {
__u64 attr;
__u64 reserved1
__u64 reserved2
__u64 reserved3
};
struct fastrpc_remote_crc {
__u64 crc;
__u64 reserved1
__u64 reserved2
__u64 reserved3
};
I don't see a need to add extra served fields for structures
that are already naturally aligned here, e.g. in
fastrpc_remote_arg we need the 'reserved1' but not
the 'reserved2'.
Yes, I see, I overdone it!
Other idea, is, may be I can try to combine these into single structure
something like:
struct fastrpc_invoke_arg {
__u64 ptr;
__u64 len;
__u32 fd;
__u32 reserved1
__u64 attr;
__u64 crc;
};
struct fastrpc_ioctl_invoke {
__u32 handle;
__u32 sc;
/* The minimum size is scalar_length * 32*/
struct fastrpc_invoke_args *args;
};
struct fastrpc_ioctl_invoke {
__u32 handle;
__u32 sc;
/* The minimum size is scalar_length * 32 */
struct fastrpc_remote_args *rargs;
struct fastrpc_remote_fd *fds;
struct fastrpc_remote_attr *attrs;
struct fastrpc_remote_crc *crc;
};
Do these really have to be indirect then? Are they all
lists of variable length? How do you know how long?
Yes, they are variable length and will be scalar length long.
Scalar length is derived from sc variable in this structure.
--srini
Arnd
Re: [RFC PATCH 3/6] char: fastrpc: Add support for context Invoke method
On Fri, Nov 30, 2018 at 4:01 PM Srinivas Kandagatla
wrote:
> Thanks Arnd for the review comments!
> On 30/11/18 13:41, Arnd Bergmann wrote:
> > On Fri, Nov 30, 2018 at 11:48 AM Srinivas Kandagatla
> > wrote:
> >> +static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
> >> +unsigned long arg)
> >> +{
> >> + struct fastrpc_user *fl = (struct fastrpc_user
> >> *)file->private_data;
> >> + struct fastrpc_channel_ctx *cctx = fl->cctx;
> >> + char __user *argp = (char __user *)arg;
> >> + int err;
> >> +
> >> + if (!fl->sctx) {
> >> + fl->sctx = fastrpc_session_alloc(cctx, 0);
> >> + if (!fl->sctx)
> >> + return -ENOENT;
> >> + }
> >
> > Shouldn't that session be allocated during open()?
> >
> Yes, and no, we do not need context in all the cases. In cases like we
> just want to allocate dmabuf.
Can you give an example what that would be good for?
>
> >> +static void fastrpc_notify_users(struct fastrpc_user *user)
> >> +{
> >> + struct fastrpc_invoke_ctx *ctx, *n;
> >> +
> >> + spin_lock(&user->lock);
> >> + list_for_each_entry_safe(ctx, n, &user->pending, node)
> >> + complete(&ctx->work);
> >> + spin_unlock(&user->lock);
> >> +}
> >
> > Can you explain here what it means to have multiple 'users'
> > a 'fastrpc_user' structure? Why are they all done at the same time?
> >
> This is the case where users need to be notified if the dsp goes down
> due to crash or shut down!
What is a 'user' then? My point is that it seems to refer to two
different things here. I assume 'fastrpc_user' is whoever
has opened the file descriptor.
> >
> >> +struct fastrpc_ioctl_invoke {
> >> + uint32_t handle;/* remote handle */
> >> + uint32_t sc;/* scalars describing the data */
> >> + remote_arg_t *pra; /* remote arguments list */
> >> + int *fds; /* fd list */
> >> + unsigned int *attrs;/* attribute list */
> >> + unsigned int *crc;
> >> +};
> >
> > This seems too complex for an ioctl argument, with
> > multiple levels of pointer indirections. I'd normally
> > try to make each ioctl argument either a scalar, or a
> > structure with only fixed-length members.
> >
> I totally agree with you and many thanks for your expert inputs,
> May be something like below with fixed length members would work?
>
> struct fastrpc_remote_arg {
> __u64 ptr; /* buffer ptr */
> __u64 len; /* length */
> __u32 fd; /* dmabuf fd */
> __u32 reserved1
> __u64 reserved2
> };
>
> struct fastrpc_remote_fd {
> __u64 fd;
> __u64 reserved1
> __u64 reserved2
> __u64 reserved3
> };
>
> struct fastrpc_remote_attr {
> __u64 attr;
> __u64 reserved1
> __u64 reserved2
> __u64 reserved3
> };
>
> struct fastrpc_remote_crc {
> __u64 crc;
> __u64 reserved1
> __u64 reserved2
> __u64 reserved3
> };
I don't see a need to add extra served fields for structures
that are already naturally aligned here, e.g. in
fastrpc_remote_arg we need the 'reserved1' but not
the 'reserved2'.
>
> struct fastrpc_ioctl_invoke {
> __u32 handle;
> __u32 sc;
> /* The minimum size is scalar_length * 32 */
> struct fastrpc_remote_args *rargs;
> struct fastrpc_remote_fd *fds;
> struct fastrpc_remote_attr *attrs;
> struct fastrpc_remote_crc *crc;
> };
Do these really have to be indirect then? Are they all
lists of variable length? How do you know how long?
Arnd
Re: [RFC PATCH 3/6] char: fastrpc: Add support for context Invoke method
Thanks Arnd for the review comments!
On 30/11/18 13:41, Arnd Bergmann wrote:
On Fri, Nov 30, 2018 at 11:48 AM Srinivas Kandagatla
wrote:
This patch adds support to compute context invoke method
on the remote processor (DSP).
This involves setting up the functions input and output arguments,
input and output handles and mapping the dmabuf fd for the
argument/handle buffers.
Most of the work is derived from various downstream Qualcomm kernels.
Credits to various Qualcomm authors who have contributed to this code.
Specially Tharun Kumar Merugu
Signed-off-by: Srinivas Kandagatla
+
+ INIT_LIST_HEAD(&ctx->node);
+ ctx->fl = user;
+ ctx->maps = (struct fastrpc_map **)(&ctx[1]);
+ ctx->lpra = (remote_arg_t *)(&ctx->maps[bufs]);
+ ctx->fds = (int *)(&ctx->lpra[bufs]);
+ ctx->attrs = (unsigned int *)(&ctx->fds[bufs]);
+
+ if (!kernel) {
+ if (copy_from_user(ctx->lpra,
+(void const __user *)inv->pra,
+bufs * sizeof(*ctx->lpra))) {
+ err = -EFAULT;
+ goto err;
+ }
+
+ if (inv->fds) {
+ if (copy_from_user(ctx->fds,
+(void const __user *)inv->fds,
+bufs * sizeof(*ctx->fds))) {
+ err = -EFAULT;
+ goto err;
+ }
+ }
+ if (inv->attrs) {
+ if (copy_from_user(
+ ctx->attrs,
+ (void const __user *)inv->attrs,
+ bufs * sizeof(*ctx->attrs))) {
+ err = -EFAULT;
+ goto err;
+ }
+ }
+ } else {
+ memcpy(ctx->lpra, inv->pra, bufs * sizeof(*ctx->lpra));
+ if (inv->fds)
+ memcpy(ctx->fds, inv->fds,
+ bufs * sizeof(*ctx->fds));
+ if (inv->attrs)
+ memcpy(ctx->attrs, inv->attrs,
+ bufs * sizeof(*ctx->attrs));
+ }
I'd split this function into multiple pieces: the internal one that
just takes kernel pointers, and a wrapper for the ioctl
that copies the user space data into the kernel before calling
the second one.
Sure, will be done in next version!
+static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
+ uint32_t kernel, remote_arg_t *upra)
+{
+ remote_arg64_t *rpra = ctx->rpra;
+ int i, inbufs, outbufs, handles;
+ struct fastrpc_invoke_buf *list;
+ struct fastrpc_phy_page *pages;
+ struct fastrpc_map *mmap;
+ uint32_t sc = ctx->sc;
+ uint64_t *fdlist;
+ uint32_t *crclist;
+ int err = 0;
+
+ inbufs = REMOTE_SCALARS_INBUFS(sc);
+ outbufs = REMOTE_SCALARS_OUTBUFS(sc);
+ handles = REMOTE_SCALARS_INHANDLES(sc) + REMOTE_SCALARS_OUTHANDLES(sc);
+ list = fastrpc_invoke_buf_start(ctx->rpra, sc);
+ pages = fastrpc_phy_page_start(sc, list);
+ fdlist = (uint64_t *)(pages + inbufs + outbufs + handles);
+ crclist = (uint32_t *)(fdlist + FASTRPC_MAX_FDLIST);
+
+ for (i = inbufs; i < inbufs + outbufs; ++i) {
+ if (!ctx->maps[i]) {
+ if (!kernel)
+ err =
+ copy_to_user((void __user *)ctx->lpra[i].buf.pv,
+ (void *)rpra[i].buf.pv, rpra[i].buf.len);
+ else
+ memcpy(ctx->lpra[i].buf.pv,
+ (void *)rpra[i].buf.pv, rpra[i].buf.len);
+
+ if (err)
+ goto bail;
+ } else {
+ fastrpc_map_put(ctx->maps[i]);
+ ctx->maps[i] = NULL;
+ }
+ }
Same here.
+static int fastrpc_internal_invoke(struct fastrpc_user *fl,
+ uint32_t kernel,
+ struct fastrpc_ioctl_invoke *inv)
+{
+ struct fastrpc_invoke_ctx *ctx = NULL;
+ int err = 0;
+
+ if (!fl->sctx)
+ return -EINVAL;
+
+ ctx = fastrpc_context_alloc(fl, kernel, inv);
+ if (IS_ERR(ctx))
+ return PTR_ERR(ctx);
+
+ if (REMOTE_SCALARS_LENGTH(ctx->sc)) {
+ err = fastrpc_get_args(kernel, ctx);
+ if (err)
+ goto bail;
+ }
+
+ err = fastrpc_invoke_send(fl->sctx, ctx, kernel, inv->handle);
+ if (err)
+ goto bail;
+
+ err = wait_for_completion_interruptible(&ctx->work);
+ if (err)
+ goto bail;
Can you add comm
Re: [RFC PATCH 3/6] char: fastrpc: Add support for context Invoke method
On Fri, Nov 30, 2018 at 11:48 AM Srinivas Kandagatla
wrote:
>
> This patch adds support to compute context invoke method
> on the remote processor (DSP).
> This involves setting up the functions input and output arguments,
> input and output handles and mapping the dmabuf fd for the
> argument/handle buffers.
>
> Most of the work is derived from various downstream Qualcomm kernels.
> Credits to various Qualcomm authors who have contributed to this code.
> Specially Tharun Kumar Merugu
>
> Signed-off-by: Srinivas Kandagatla
> +
> + INIT_LIST_HEAD(&ctx->node);
> + ctx->fl = user;
> + ctx->maps = (struct fastrpc_map **)(&ctx[1]);
> + ctx->lpra = (remote_arg_t *)(&ctx->maps[bufs]);
> + ctx->fds = (int *)(&ctx->lpra[bufs]);
> + ctx->attrs = (unsigned int *)(&ctx->fds[bufs]);
> +
> + if (!kernel) {
> + if (copy_from_user(ctx->lpra,
> +(void const __user *)inv->pra,
> +bufs * sizeof(*ctx->lpra))) {
> + err = -EFAULT;
> + goto err;
> + }
> +
> + if (inv->fds) {
> + if (copy_from_user(ctx->fds,
> +(void const __user *)inv->fds,
> +bufs * sizeof(*ctx->fds))) {
> + err = -EFAULT;
> + goto err;
> + }
> + }
> + if (inv->attrs) {
> + if (copy_from_user(
> + ctx->attrs,
> + (void const __user *)inv->attrs,
> + bufs * sizeof(*ctx->attrs))) {
> + err = -EFAULT;
> + goto err;
> + }
> + }
> + } else {
> + memcpy(ctx->lpra, inv->pra, bufs * sizeof(*ctx->lpra));
> + if (inv->fds)
> + memcpy(ctx->fds, inv->fds,
> + bufs * sizeof(*ctx->fds));
> + if (inv->attrs)
> + memcpy(ctx->attrs, inv->attrs,
> + bufs * sizeof(*ctx->attrs));
> + }
I'd split this function into multiple pieces: the internal one that
just takes kernel pointers, and a wrapper for the ioctl
that copies the user space data into the kernel before calling
the second one.
> +static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
> + uint32_t kernel, remote_arg_t *upra)
> +{
> + remote_arg64_t *rpra = ctx->rpra;
> + int i, inbufs, outbufs, handles;
> + struct fastrpc_invoke_buf *list;
> + struct fastrpc_phy_page *pages;
> + struct fastrpc_map *mmap;
> + uint32_t sc = ctx->sc;
> + uint64_t *fdlist;
> + uint32_t *crclist;
> + int err = 0;
> +
> + inbufs = REMOTE_SCALARS_INBUFS(sc);
> + outbufs = REMOTE_SCALARS_OUTBUFS(sc);
> + handles = REMOTE_SCALARS_INHANDLES(sc) +
> REMOTE_SCALARS_OUTHANDLES(sc);
> + list = fastrpc_invoke_buf_start(ctx->rpra, sc);
> + pages = fastrpc_phy_page_start(sc, list);
> + fdlist = (uint64_t *)(pages + inbufs + outbufs + handles);
> + crclist = (uint32_t *)(fdlist + FASTRPC_MAX_FDLIST);
> +
> + for (i = inbufs; i < inbufs + outbufs; ++i) {
> + if (!ctx->maps[i]) {
> + if (!kernel)
> + err =
> + copy_to_user((void __user
> *)ctx->lpra[i].buf.pv,
> + (void *)rpra[i].buf.pv,
> rpra[i].buf.len);
> + else
> + memcpy(ctx->lpra[i].buf.pv,
> + (void *)rpra[i].buf.pv,
> rpra[i].buf.len);
> +
> + if (err)
> + goto bail;
> + } else {
> + fastrpc_map_put(ctx->maps[i]);
> + ctx->maps[i] = NULL;
> + }
> + }
Same here.
> +static int fastrpc_internal_invoke(struct fastrpc_user *fl,
> + uint32_t kernel,
> + struct fastrpc_ioctl_invoke *inv)
> +{
> + struct fastrpc_invoke_ctx *ctx = NULL;
> + int err = 0;
> +
> + if (!fl->sctx)
> + return -EINVAL;
> +
> + ctx = fastrpc_context_alloc(fl, kernel, inv);
> + if (IS_ERR(ctx))
> + return PTR_ERR(ctx);
> +
> + if (REMOTE_SCALARS_LENGTH(ctx->sc)) {
> + err = fastrpc_get_args(kernel, ctx);
> + if (err)
> + goto bail;
> + }
> +
> + err = fastrpc_invoke_send(fl->sctx, ctx, kernel, inv->handle);
> + if (err)
> + goto bail;
> +
> +
[RFC PATCH 3/6] char: fastrpc: Add support for context Invoke method
This patch adds support to compute context invoke method
on the remote processor (DSP).
This involves setting up the functions input and output arguments,
input and output handles and mapping the dmabuf fd for the
argument/handle buffers.
Most of the work is derived from various downstream Qualcomm kernels.
Credits to various Qualcomm authors who have contributed to this code.
Specially Tharun Kumar Merugu
Signed-off-by: Srinivas Kandagatla
---
drivers/char/fastrpc.c | 790 +++
include/uapi/linux/fastrpc.h | 56 +++
2 files changed, 846 insertions(+)
create mode 100644 include/uapi/linux/fastrpc.h
diff --git a/drivers/char/fastrpc.c b/drivers/char/fastrpc.c
index 97d8062eb3e1..5bb224adc24f 100644
--- a/drivers/char/fastrpc.c
+++ b/drivers/char/fastrpc.c
@@ -3,7 +3,9 @@
// Copyright (c) 2018, Linaro Limited
#include
+#include
#include
+#include
#include
#include
#include
@@ -14,6 +16,7 @@
#include
#include
#include
+#include
#define ADSP_DOMAIN_ID (0)
#define MDSP_DOMAIN_ID (1)
@@ -21,10 +24,41 @@
#define CDSP_DOMAIN_ID (3)
#define FASTRPC_DEV_MAX4 /* adsp, mdsp, slpi, cdsp*/
#define FASTRPC_MAX_SESSIONS 9 /*8 compute, 1 cpz*/
+#define FASTRPC_ALIGN 128
+#define FASTRPC_MAX_FDLIST 16
+#define FASTRPC_MAX_CRCLIST64
+#define FASTRPC_PHYS(p)(p & 0x)
#define FASTRPC_CTX_MAX (256)
#define FASTRPC_CTXID_MASK (0xFF0)
#define FASTRPC_DEVICE_NAME"fastrpc"
+/* Retrives number of input buffers from the scalars parameter */
+#define REMOTE_SCALARS_INBUFS(sc)(((sc) >> 16) & 0x0ff)
+
+/* Retrives number of output buffers from the scalars parameter */
+#define REMOTE_SCALARS_OUTBUFS(sc) (((sc) >> 8) & 0x0ff)
+
+/* Retrives number of input handles from the scalars parameter */
+#define REMOTE_SCALARS_INHANDLES(sc) (((sc) >> 4) & 0x0f)
+
+/* Retrives number of output handles from the scalars parameter */
+#define REMOTE_SCALARS_OUTHANDLES(sc)((sc) & 0x0f)
+
+#define REMOTE_SCALARS_LENGTH(sc) (REMOTE_SCALARS_INBUFS(sc) +\
+ REMOTE_SCALARS_OUTBUFS(sc) +\
+ REMOTE_SCALARS_INHANDLES(sc) +\
+ REMOTE_SCALARS_OUTHANDLES(sc))
+
+#define FASTRPC_BUILD_SCALARS(attr, method, in, out, oin, oout) \
+ uint32_t) (attr) & 0x7) << 29) | \
+ (((uint32_t) (method) & 0x1f) << 24) | \
+ (((uint32_t) (in) & 0xff) << 16) | \
+ (((uint32_t)(out) & 0xff) << 8) | \
+ (((uint32_t)(oin) & 0x0f) << 4) | \
+ ((uint32_t) (oout) & 0x0f))
+
+#define FASTRPC_SCALARS(method, in, out) \
+ FASTRPC_BUILD_SCALARS(0, method, in, out, 0, 0)
#define cdev_to_cctx(d) container_of(d, struct fastrpc_channel_ctx, cdev)
static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp",
@@ -32,6 +66,82 @@ static const char *domains[FASTRPC_DEV_MAX] = { "adsp",
"mdsp",
static dev_t fastrpc_major;
static struct class *fastrpc_class;
+struct fastrpc_invoke_header {
+ uint64_t ctx; /* invoke caller context */
+ uint32_t handle;/* handle to invoke */
+ uint32_t sc;/* scalars structure describing the data */
+};
+
+struct fastrpc_phy_page {
+ uint64_t addr; /* physical address */
+ uint64_t size; /* size of contiguous region */
+};
+
+struct fastrpc_invoke_buf {
+ int num;/* number of contiguous regions */
+ int pgidx; /* index to start of contiguous region */
+};
+
+struct fastrpc_invoke {
+ struct fastrpc_invoke_header header;
+ struct fastrpc_phy_page page; /* list of pages address */
+};
+
+struct fastrpc_msg {
+ uint32_t pid; /* process group id */
+ uint32_t tid; /* thread id */
+ struct fastrpc_invoke invoke;
+};
+
+struct fastrpc_invoke_rsp {
+ uint64_t ctx; /* invoke caller context */
+ int retval; /* invoke return value */
+};
+
+struct fastrpc_buf {
+ struct fastrpc_user *fl;
+ struct device *dev;
+ void *virt;
+ uint64_t phys;
+ size_t size;
+};
+
+struct fastrpc_map {
+ struct list_head node;
+ struct fastrpc_user *fl;
+ int fd;
+ struct dma_buf *buf;
+ struct sg_table *table;
+ struct dma_buf_attachment *attach;
+ uint64_t phys;
+ size_t size;
+ uintptr_t va;
+ size_t len;
+ struct kref refcount;
+};
+
+struct fastrpc_invoke_ctx {
+ struct fastrpc_user *fl;
+ struct list_head node; /* list of ctxs */
+ struct completion work;
+ int retval;
+ int pid;
+ int tgid;
+ uint32_t sc;
+ struct fastrpc_msg msg;
+ uint64_t ctxid;
+ size_t used_sz;
+
+ remote_arg_t *lpra;
+ unsigned int *attrs;
+ int *fds;
+
