| Message ID | 1685974276-23435-1-git-send-email-quic_ekangupt@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] misc: fastrpc: Fix remote heap allocation request | expand |
On 05/06/2023 15:11, Ekansh Gupta wrote: > Remote heap is used by DSP audioPD on need basis. This memory is > allocated from reserved CMA memory region and is then shared with > audioPD to use it for it's functionality. > > Current implementation of remote heap is not allocating the memory > from CMA region, instead it is allocating the memory from SMMU > context bank. The arguments passed to scm call for the reassignment > of ownership is also not correct. Added changes to allocate CMA > memory and have a proper ownership reassignment. > > Fixes: 532ad70c6d44 ("misc: fastrpc: Add mmap request assigning for static PD pool") > Cc: stable <stable@kernel.org> > Tested-by: Ekansh Gupta <quic_ekangupt@quicinc.com> > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> > --- > drivers/misc/fastrpc.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index 30d4d04..f5fc2de 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -1866,7 +1866,11 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) > return -EINVAL; > } > > - err = fastrpc_buf_alloc(fl, fl->sctx->dev, req.size, &buf); > + 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 (err) { > dev_err(dev, "failed to allocate buffer\n"); > return err; > @@ -1905,12 +1909,22 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) > > /* 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; > + u64 src_perms = BIT(QCOM_SCM_VMID_HLOS); > + struct qcom_scm_vmperm *dst_perms; > + u32 i; > > - perm.vmid = QCOM_SCM_VMID_HLOS; > - perm.perm = QCOM_SCM_PERM_RWX; > - err = qcom_scm_assign_mem(buf->phys, buf->size, > - &fl->cctx->perms, &perm, 1); > + dst_perms = kcalloc(fl->cctx->vmcount, > + sizeof(struct qcom_scm_vmperm), GFP_KERNEL); > + if (!dst_perms) > + return -ENOMEM; > + for (i = 0; i < fl->cctx->vmcount; i++) { > + dst_perms[i].vmid = fl->cctx->vmperms[i].vmid; > + dst_perms[i].perm = fl->cctx->vmperms[i].perm; > + } > + Why not use fl->cctx->vmperms instead of allocating a new dst_perms? --srini > + err = qcom_scm_assign_mem(buf->phys,(u64)buf->size, > + &src_perms, dst_perms, fl->cctx->vmcount); > + kfree(dst_perms); > if (err) { > dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d", > buf->phys, buf->size, err);
On 6/12/2023 5:34 PM, Srinivas Kandagatla wrote: > > > On 05/06/2023 15:11, Ekansh Gupta wrote: >> Remote heap is used by DSP audioPD on need basis. This memory is >> allocated from reserved CMA memory region and is then shared with >> audioPD to use it for it's functionality. >> >> Current implementation of remote heap is not allocating the memory >> from CMA region, instead it is allocating the memory from SMMU >> context bank. The arguments passed to scm call for the reassignment >> of ownership is also not correct. Added changes to allocate CMA >> memory and have a proper ownership reassignment. >> >> Fixes: 532ad70c6d44 ("misc: fastrpc: Add mmap request assigning for >> static PD pool") >> Cc: stable <stable@kernel.org> >> Tested-by: Ekansh Gupta <quic_ekangupt@quicinc.com> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> >> --- >> drivers/misc/fastrpc.c | 26 ++++++++++++++++++++------ >> 1 file changed, 20 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c >> index 30d4d04..f5fc2de 100644 >> --- a/drivers/misc/fastrpc.c >> +++ b/drivers/misc/fastrpc.c >> @@ -1866,7 +1866,11 @@ static int fastrpc_req_mmap(struct fastrpc_user >> *fl, char __user *argp) >> return -EINVAL; >> } >> - err = fastrpc_buf_alloc(fl, fl->sctx->dev, req.size, &buf); >> + 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 (err) { >> dev_err(dev, "failed to allocate buffer\n"); >> return err; >> @@ -1905,12 +1909,22 @@ static int fastrpc_req_mmap(struct >> fastrpc_user *fl, char __user *argp) >> /* 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; >> + u64 src_perms = BIT(QCOM_SCM_VMID_HLOS); >> + struct qcom_scm_vmperm *dst_perms; >> + u32 i; >> - perm.vmid = QCOM_SCM_VMID_HLOS; >> - perm.perm = QCOM_SCM_PERM_RWX; >> - err = qcom_scm_assign_mem(buf->phys, buf->size, >> - &fl->cctx->perms, &perm, 1); >> + dst_perms = kcalloc(fl->cctx->vmcount, >> + sizeof(struct qcom_scm_vmperm), GFP_KERNEL); >> + if (!dst_perms) >> + return -ENOMEM; >> + for (i = 0; i < fl->cctx->vmcount; i++) { >> + dst_perms[i].vmid = fl->cctx->vmperms[i].vmid; >> + dst_perms[i].perm = fl->cctx->vmperms[i].perm; >> + } >> + > Why not use fl->cctx->vmperms instead of allocating a new dst_perms? > > --srini > Thanks Srini for your comments. I'll address them in next patch. >> + err = qcom_scm_assign_mem(buf->phys,(u64)buf->size, >> + &src_perms, dst_perms, fl->cctx->vmcount); >> + kfree(dst_perms); >> if (err) { >> dev_err(fl->sctx->dev, "Failed to assign memory phys >> 0x%llx size 0x%llx err %d", >> buf->phys, buf->size, err);
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 30d4d04..f5fc2de 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -1866,7 +1866,11 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) return -EINVAL; } - err = fastrpc_buf_alloc(fl, fl->sctx->dev, req.size, &buf); + 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 (err) { dev_err(dev, "failed to allocate buffer\n"); return err; @@ -1905,12 +1909,22 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) /* 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; + u64 src_perms = BIT(QCOM_SCM_VMID_HLOS); + struct qcom_scm_vmperm *dst_perms; + u32 i; - perm.vmid = QCOM_SCM_VMID_HLOS; - perm.perm = QCOM_SCM_PERM_RWX; - err = qcom_scm_assign_mem(buf->phys, buf->size, - &fl->cctx->perms, &perm, 1); + dst_perms = kcalloc(fl->cctx->vmcount, + sizeof(struct qcom_scm_vmperm), GFP_KERNEL); + if (!dst_perms) + return -ENOMEM; + for (i = 0; i < fl->cctx->vmcount; i++) { + dst_perms[i].vmid = fl->cctx->vmperms[i].vmid; + dst_perms[i].perm = fl->cctx->vmperms[i].perm; + } + + err = qcom_scm_assign_mem(buf->phys,(u64)buf->size, + &src_perms, dst_perms, fl->cctx->vmcount); + kfree(dst_perms); if (err) { dev_err(fl->sctx->dev, "Failed to assign memory phys 0x%llx size 0x%llx err %d", buf->phys, buf->size, err);
Remote heap is used by DSP audioPD on need basis. This memory is allocated from reserved CMA memory region and is then shared with audioPD to use it for it's functionality. Current implementation of remote heap is not allocating the memory from CMA region, instead it is allocating the memory from SMMU context bank. The arguments passed to scm call for the reassignment of ownership is also not correct. Added changes to allocate CMA memory and have a proper ownership reassignment. Fixes: 532ad70c6d44 ("misc: fastrpc: Add mmap request assigning for static PD pool") Cc: stable <stable@kernel.org> Tested-by: Ekansh Gupta <quic_ekangupt@quicinc.com> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> --- drivers/misc/fastrpc.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)