Received-SPF: pass (google.com: domain of linux-kernel+bounces-210904-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <7d75f6b1-60fe-4a03-a251-4675a3364148@quicinc.com>
Date: Wed, 12 Jun 2024 09:54:47 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 6/7] misc: fastrpc: Fix ownership reassignment of
 remote heap
Content-Language: en-US
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
CC: <srinivas.kandagatla@linaro.org>, <linux-arm-msm@vger.kernel.org>,
        <gregkh@linuxfoundation.org>, <quic_bkumar@quicinc.com>,
        <linux-kernel@vger.kernel.org>, <quic_chennak@quicinc.com>,
        stable
	<stable@kernel.org>
References: <20240611103442.27198-1-quic_ekangupt@quicinc.com>
 <20240611103442.27198-7-quic_ekangupt@quicinc.com>
 <xc2ys75plbtrenastitqafadfrtolpd3bjdqcrl3wnozpc6kdo@e6e73ousyea7>
From: Ekansh Gupta <quic_ekangupt@quicinc.com>
In-Reply-To: <xc2ys75plbtrenastitqafadfrtolpd3bjdqcrl3wnozpc6kdo@e6e73ousyea7>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit


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@kernel.org>
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.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
>>