[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <xvy4syldmqdklij4wbub6x52gexijlrgtoketd3urohxrca5g2@mmioafzpmy7s>
Date: Wed, 12 Jun 2024 15:49:09 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Ekansh Gupta <quic_ekangupt@...cinc.com>
Cc: srinivas.kandagatla@...aro.org, linux-arm-msm@...r.kernel.org,
gregkh@...uxfoundation.org, quic_bkumar@...cinc.com, linux-kernel@...r.kernel.org,
quic_chennak@...cinc.com, stable <stable@...nel.org>
Subject: Re: [PATCH v5 6/7] misc: fastrpc: Fix ownership reassignment of
remote heap
On Wed, Jun 12, 2024 at 09:54:47AM +0530, Ekansh Gupta wrote:
>
>
> On 6/11/2024 5:29 PM, Dmitry Baryshkov wrote:
> > On Tue, Jun 11, 2024 at 04:04:39PM +0530, Ekansh Gupta wrote:
> >> Audio PD daemon will allocate memory for audio PD dynamic loading
> > What is Audio PD daemon? Is it something running on the CPU or on the
> > DSP? Is it adsprpcd or some other daemon?
> It's adsprpcd which is going to attach to Audio PD.
Ack
> >
> >> usage when it is attaching for the first time to audio PD. As
> >> part of this, the memory ownership is moved to the VM where
> > Which VM?
> In audio PD case, it's the following VMIDs:
> QCOM_SCM_VMID_LPASS
> QCOM_SCM_VMID_ADSP_HEAP
>
> Defined here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/dt-bindings/firmware/qcom,scm.h?h=v6.10-rc3
>
> These are expected to be added to fastrpc DT node as "qcom,vmids"
Ok, good.
>
> >
> >> audio PD can use it. In case daemon process is killed without any
> >> impact to DSP audio PD, the daemon process will retry to attach to
> >> audio PD and in this case memory won't be reallocated. If the invoke
> >> fails due to any reason, as part of err_invoke, the memory ownership
> >> is getting reassigned to HLOS even when the memory was not allocated.
> >> At this time the audio PD might still be using the memory and an
> >> attemp of ownership reassignment would result in memory issue.
> > What kind of 'memory issues'? Is it even possible to reclaim the memory
> > back?
> In case when audio PD on DSP is still using the memory, the ownership should not be
> moved to HLOS. This might happen in daemon kill scenario where remote_heap is not
> allocated again, but if due to any reason if the fastrpc_internal_invoke fails, that might
> result in the ownership change of remote_heap memory.
You are describing the expected behaviour. not the observed issue.
Also, the second quesiton didn't get the answer. Is it possible to free
/reclaim the memory?
> >
> >> Fixes: 0871561055e6 ("misc: fastrpc: Add support for audiopd")
> >> Cc: stable <stable@...nel.org>
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@...cinc.com>
> >> ---
> >> drivers/misc/fastrpc.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> >> index 1ba85c70e3ff..24dc1cba40e9 100644
> >> --- a/drivers/misc/fastrpc.c
> >> +++ b/drivers/misc/fastrpc.c
> >> @@ -1238,6 +1238,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> >> struct fastrpc_phy_page pages[1];
> >> char *name;
> >> int err;
> >> + bool scm_done = false;
> >> struct {
> >> int pgid;
> >> u32 namelen;
> >> @@ -1289,6 +1290,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> >> fl->cctx->remote_heap->phys, fl->cctx->remote_heap->size, err);
> >> goto err_map;
> >> }
> >> + scm_done = true;
> >> }
> >> }
> >>
> >> @@ -1324,7 +1326,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> >>
> >> return 0;
> >> err_invoke:
> >> - if (fl->cctx->vmcount) {
> >> + if (fl->cctx->vmcount && scm_done) {
> >> u64 src_perms = 0;
> >> struct qcom_scm_vmperm dst_perms;
> >> u32 i;
> >> --
> >> 2.43.0
> >>
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists