From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
To: Abel Vesa <abel.vesa@linaro.org>,
Amol Maheshwari <amahesh@qti.qualcomm.com>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konrad.dybcio@somainline.org>,
Ekansh Gupta <quic_ekagupt@quicinc.com>
Cc: Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-arm-msm@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
devicetree@vger.kernel.org
Subject: Re: [PATCH v2 07/10] misc: fastrpc: Add support for audiopd
Date: Tue, 6 Sep 2022 15:10:41 +0100 [thread overview]
Message-ID: <200f2cc9-1747-95a3-82b1-600650cff1df@linaro.org> (raw)
In-Reply-To: <20220902154900.3404524-8-abel.vesa@linaro.org>
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 <srinivas.kandagatla@linaro.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> 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 */
next prev parent reply other threads:[~2022-09-06 14:56 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-02 15:48 [PATCH v2 00/10] misc: fastrpc: Add audiopd support Abel Vesa
2022-09-02 15:48 ` [PATCH v2 01/10] misc: fastrpc: Rename audio protection domain to root Abel Vesa
2022-09-02 15:48 ` [PATCH v2 02/10] misc: fastrpc: Add reserved mem support Abel Vesa
2022-09-02 15:48 ` [PATCH v2 03/10] dt-bindings: misc: fastrpc: Document memory-region property Abel Vesa
2022-09-04 19:29 ` Krzysztof Kozlowski
2022-09-05 7:53 ` Abel Vesa
2022-09-02 15:48 ` [PATCH v2 04/10] misc: fastrpc: Add fastrpc_remote_heap_alloc Abel Vesa
2022-09-02 15:48 ` [PATCH v2 05/10] misc: fastrpc: Use fastrpc_map_put in fastrpc_map_create on fail Abel Vesa
2022-09-06 14:10 ` Srinivas Kandagatla
2022-09-02 15:48 ` [PATCH v2 06/10] misc: fastrpc: Rework fastrpc_req_munmap Abel Vesa
2022-09-02 15:48 ` [PATCH v2 07/10] misc: fastrpc: Add support for audiopd Abel Vesa
2022-09-03 2:36 ` kernel test robot
2022-09-03 3:18 ` kernel test robot
2022-09-06 14:10 ` Srinivas Kandagatla [this message]
2022-09-02 15:48 ` [PATCH v2 08/10] misc: fastrpc: Safekeep mmaps on interrupted invoke Abel Vesa
2022-09-06 14:10 ` Srinivas Kandagatla
2022-09-02 15:48 ` [PATCH v2 09/10] misc: fastrpc: Add mmap request assigning for static PD pool Abel Vesa
2022-09-02 15:49 ` [PATCH v2 10/10] misc: fastrpc: Add dma_mask to fastrpc_channel_ctx Abel Vesa
2022-09-06 14:12 ` [PATCH v2 00/10] misc: fastrpc: Add audiopd support Srinivas Kandagatla
2022-09-07 10:01 ` Abel Vesa
2022-11-25 7:13 [PATCH v2 00/10] misc: fastrpc: patches for 6.2 Srinivas Kandagatla
2022-11-25 7:14 ` [PATCH v2 07/10] misc: fastrpc: Add support for audiopd Srinivas Kandagatla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200f2cc9-1747-95a3-82b1-600650cff1df@linaro.org \
--to=srinivas.kandagatla@linaro.org \
--cc=abel.vesa@linaro.org \
--cc=agross@kernel.org \
--cc=amahesh@qti.qualcomm.com \
--cc=andersson@kernel.org \
--cc=arnd@arndb.de \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=konrad.dybcio@somainline.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=quic_ekagupt@quicinc.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).