Re: [PATCH v5 6/7] misc: fastrpc: Fix ownership reassignment of remote heap

[Ekansh Gupta]

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.
>
>> 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"

>
>> 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.
>
>> Fixes: 0871561055e6 ("misc: fastrpc: Add support for audiopd")
>> Cc: stable <stable@xxxxxxxxxx>
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@xxxxxxxxxxx>
>> ---
>>  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
>>