On Wed, Jul 09, 2025 at 11:13:49AM +0530, Ekansh Gupta wrote: > > > On 6/12/2025 3:54 PM, Dmitry Baryshkov wrote: > > On Thu, Jun 12, 2025 at 03:02:52PM +0530, Ekansh Gupta wrote: > >> > >> On 6/12/2025 1:35 PM, Dmitry Baryshkov wrote: > >>> On Thu, Jun 12, 2025 at 10:50:10AM +0530, Ekansh Gupta wrote: > >>>> On 5/22/2025 5:43 PM, Dmitry Baryshkov wrote: > >>>>> On Thu, 22 May 2025 at 08:01, Ekansh Gupta > >>>>> <ekansh.gu...@oss.qualcomm.com> wrote: > >>>>>> On 5/19/2025 7:04 PM, Dmitry Baryshkov wrote: > >>>>>>> On Mon, May 19, 2025 at 04:28:34PM +0530, Ekansh Gupta wrote: > >>>>>>>> On 5/19/2025 4:22 PM, Dmitry Baryshkov wrote: > >>>>>>>>> On Tue, May 13, 2025 at 09:58:25AM +0530, Ekansh Gupta wrote: > >>>>>>>>>> User request for remote heap allocation is supported using ioctl > >>>>>>>>>> interface but support for unmap is missing. This could result in > >>>>>>>>>> memory leak issues. Add unmap user request support for remote heap. > >>>>>>>>> Can this memory be in use by the remote proc? > >>>>>>>> Remote heap allocation request is only intended for audioPD. Other > >>>>>>>> PDs > >>>>>>>> running on DSP are not intended to use this request. > >>>>>>> 'Intended'. That's fine. I asked a different question: _can_ it be in > >>>>>>> use? What happens if userspace by mistake tries to unmap memory too > >>>>>>> early? Or if it happens intentionally, at some specific time during > >>>>>>> work. > >>>>>> If the unmap is restricted to audio daemon, then the unmap will only > >>>>>> happen if the remoteproc is no longer using this memory. > >>>>>> > >>>>>> But without this restriction, yes it possible that some userspace > >>>>>> process > >>>>>> calls unmap which tries to move the ownership back to HLOS which the > >>>>>> remoteproc is still using the memory. This might lead to memory access > >>>>>> problems. > >>>>> This needs to be fixed in the driver. We need to track which memory is > >>>>> being used by the remoteproc and unmap it once remoteproc stops using > >>>>> it, without additional userspace intervention. > >>>> If it's the audio daemon which is requesting for unmap then it basically > >>>> means that > >>>> the remoteproc is no longer using the memory. Audio PD can request for > >>>> both grow > >>>> and shrink operations for it's dedicated heap. The case of grow is > >>>> already supported > >>>> from fastrpc_req_mmap but the case of shrink(when remoteproc is no > >>>> longer using the > >>>> memory) is not yet available. This memory is more specific to audio PD > >>>> rather than > >>>> complete remoteproc. > >>>> > >>>> If we have to control this completely from driver then I see a problem > >>>> in freeing/unmapping > >>>> the memory when the PD is no longer using the memory. > >>> What happens if userspace requests to free the memory that is still in > >>> use by the PD > >> I understand your point, for this I was thinking to limit the unmap > >> functionality to the process > >> that is already attached to audio PD on DSP, no other process will be able > >> to map/unmap this > >> memory from userspace. > > Ugh... and what if the adsprpcd misbehaves? > > > >>> How does PD signal the memory is no longer in use? > >> PD makes a reverse fastrpc request[1] to unmap the memory when it is no > >> longer used. > > I don't see how this can be made robust. I fear that the only way would > > be to unmap the memory only on audio PD restart / shutdown. Such > > requests should never leave the kernel. > > > > Moreover, the payload should not be trusted, however you don't validate > > the length that you've got from the remote side. > I was thinking of giving the entire reserved memory to audio PD when > init_create_static_process is called. This way, we can avoid any need to > support grow/free request from user process and the audio PD pool on > DSP will have sufficient memory support the use cases. > > This way the free can be moved to fastrpc_rpmsg_remove(When DSP > is shutting down) or during Audio PD restart(Static PD restart is not > yet supported, but clean-up can be done when PDR framework is > implemented in the future). > > Do you see any drawbacks with this design?
I'm sorry for the delay in responding to your email. I think this is a perfect idea. Can we be sure that there will be no extra requests from the DSP? > > > > >> [1] > >> https://github.com/quic/fastrpc/blob/development/src/apps_mem_imp.c#L231 > -- With best wishes Dmitry
