1. Patchew
  2. linux
  3. [PATCH v2 0/4] Add missing bug fixes
  4. View patch

[PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

Jianping Li posted 4 patches 3 weeks, 4 days ago
[PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
Posted by Jianping Li 3 weeks, 4 days ago 
The entire reserved-memory region is now assigned to DSP VMIDs during
channel setup and stored in cctx->remote_heap. Memory is reclaimed in
rpmsg_remove by revoking DSP permissions and freeing the buffer, tying
heap lifecycle to the rpmsg channel.

Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 95 ++++++++++++++++++++----------------------
 1 file changed, 45 insertions(+), 50 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 833c265add5e..f9edca7a8de1 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -278,6 +278,8 @@ struct fastrpc_channel_ctx {
 	struct kref refcount;
 	/* Flag if dsp attributes are cached */
 	bool valid_attributes;
+	/* Flag if audio PD init mem was allocated */
+	bool audio_init_mem;
 	u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
 	struct fastrpc_device *secure_fdevice;
 	struct fastrpc_device *fdevice;
@@ -1304,7 +1306,6 @@ 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 client_id;
 		u32 namelen;
@@ -1334,31 +1335,6 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	inbuf.client_id = fl->client_id;
 	inbuf.namelen = init.namelen;
 	inbuf.pageslen = 0;
-	if (!fl->cctx->remote_heap) {
-		err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
-						&fl->cctx->remote_heap);
-		if (err)
-			goto err_name;
-
-		/* Map if we have any heap VMIDs associated with this ADSP Static Process. */
-		if (fl->cctx->vmcount) {
-			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
-
-			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
-							(u64)fl->cctx->remote_heap->size,
-							&src_perms,
-							fl->cctx->vmperms, fl->cctx->vmcount);
-			if (err) {
-				dev_err(fl->sctx->dev,
-					"Failed to assign memory with dma_addr %pad size 0x%llx err %d\n",
-					&fl->cctx->remote_heap->dma_addr,
-					fl->cctx->remote_heap->size, err);
-				goto err_map;
-			}
-			scm_done = true;
-			inbuf.pageslen = 1;
-		}
-	}
 
 	fl->pd = USER_PD;
 
@@ -1370,8 +1346,15 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 	args[1].length = inbuf.namelen;
 	args[1].fd = -1;
 
-	pages[0].addr = fl->cctx->remote_heap->dma_addr;
-	pages[0].size = fl->cctx->remote_heap->size;
+	if (!fl->cctx->audio_init_mem) {
+		pages[0].addr = fl->cctx->remote_heap->dma_addr;
+		pages[0].size = fl->cctx->remote_heap->size;
+		fl->cctx->audio_init_mem = true;
+		inbuf.pageslen = 1;
+	} else {
+		pages[0].addr = 0;
+		pages[0].size = 0;
+	}
 
 	args[2].ptr = (u64)(uintptr_t) pages;
 	args[2].length = sizeof(*pages);
@@ -1389,26 +1372,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
 
 	return 0;
 err_invoke:
-	if (fl->cctx->vmcount && scm_done) {
-		u64 src_perms = 0;
-		struct qcom_scm_vmperm dst_perms;
-		u32 i;
-
-		for (i = 0; i < fl->cctx->vmcount; i++)
-			src_perms |= BIT(fl->cctx->vmperms[i].vmid);
-
-		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
-		dst_perms.perm = QCOM_SCM_PERM_RWX;
-		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
-						(u64)fl->cctx->remote_heap->size,
-						&src_perms, &dst_perms, 1);
-		if (err)
-			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr %pad size 0x%llx err %d\n",
-				&fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
-	}
-err_map:
-	fastrpc_buf_free(fl->cctx->remote_heap);
-err_name:
+	fl->cctx->audio_init_mem = false;
 	kfree(name);
 err:
 	kfree(args);
@@ -2396,7 +2360,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 		}
 	}
 
-	if (domain_id == SDSP_DOMAIN_ID) {
+	if (domain_id == SDSP_DOMAIN_ID || domain_id == ADSP_DOMAIN_ID) {
 		struct resource res;
 		u64 src_perms;
 
@@ -2408,6 +2372,15 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 				    data->vmperms, data->vmcount);
 		}
 
+		if (domain_id == ADSP_DOMAIN_ID) {
+			data->remote_heap =
+				kzalloc(sizeof(*data->remote_heap), GFP_KERNEL);
+			if (!data->remote_heap)
+				return -ENOMEM;
+
+			data->remote_heap->dma_addr = res.start;
+			data->remote_heap->size = resource_size(&res);
+		}
 	}
 
 	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
@@ -2488,10 +2461,13 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
 	struct fastrpc_buf *buf, *b;
 	struct fastrpc_user *user;
 	unsigned long flags;
+	bool skip_free = false;
+	int err;
 
 	/* No invocations past this point */
 	spin_lock_irqsave(&cctx->lock, flags);
 	cctx->rpdev = NULL;
+	cctx->audio_init_mem = false;
 	list_for_each_entry(user, &cctx->users, user)
 		fastrpc_notify_users(user);
 	spin_unlock_irqrestore(&cctx->lock, flags);
@@ -2505,7 +2481,26 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
 	list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
 		list_del(&buf->node);
 
-	fastrpc_buf_free(cctx->remote_heap);
+	if (cctx->remote_heap) {
+		if (cctx->vmcount) {
+			u64 src_perms = 0;
+			struct qcom_scm_vmperm dst_perms;
+
+			for (u32 i = 0; i < cctx->vmcount; i++)
+				src_perms |= BIT(cctx->vmperms[i].vmid);
+
+			dst_perms.vmid = QCOM_SCM_VMID_HLOS;
+			dst_perms.perm = QCOM_SCM_PERM_RWX;
+
+			err = qcom_scm_assign_mem(cctx->remote_heap->dma_addr,
+						  cctx->remote_heap->size,
+						  &src_perms, &dst_perms, 1);
+			if (err)
+				skip_free = true;
+		}
+		if (!skip_free)
+			fastrpc_buf_free(cctx->remote_heap);
+	}
 
 	of_platform_depopulate(&rpdev->dev);
 
-- 
2.43.0
Re: [PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
Posted by Dmitry Baryshkov 3 weeks, 3 days ago 
On Thu, Jan 15, 2026 at 04:28:51PM +0800, Jianping Li wrote:
> The entire reserved-memory region is now assigned to DSP VMIDs during
> channel setup and stored in cctx->remote_heap. Memory is reclaimed in
> rpmsg_remove by revoking DSP permissions and freeing the buffer, tying
> heap lifecycle to the rpmsg channel.

Why?

> 
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 95 ++++++++++++++++++++----------------------
>  1 file changed, 45 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 833c265add5e..f9edca7a8de1 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -278,6 +278,8 @@ struct fastrpc_channel_ctx {
>  	struct kref refcount;
>  	/* Flag if dsp attributes are cached */
>  	bool valid_attributes;
> +	/* Flag if audio PD init mem was allocated */
> +	bool audio_init_mem;
>  	u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
>  	struct fastrpc_device *secure_fdevice;
>  	struct fastrpc_device *fdevice;
> @@ -1304,7 +1306,6 @@ 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 client_id;
>  		u32 namelen;
> @@ -1334,31 +1335,6 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  	inbuf.client_id = fl->client_id;
>  	inbuf.namelen = init.namelen;
>  	inbuf.pageslen = 0;
> -	if (!fl->cctx->remote_heap) {
> -		err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
> -						&fl->cctx->remote_heap);
> -		if (err)
> -			goto err_name;
> -
> -		/* Map if we have any heap VMIDs associated with this ADSP Static Process. */
> -		if (fl->cctx->vmcount) {
> -			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
> -
> -			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> -							(u64)fl->cctx->remote_heap->size,
> -							&src_perms,
> -							fl->cctx->vmperms, fl->cctx->vmcount);
> -			if (err) {
> -				dev_err(fl->sctx->dev,
> -					"Failed to assign memory with dma_addr %pad size 0x%llx err %d\n",
> -					&fl->cctx->remote_heap->dma_addr,
> -					fl->cctx->remote_heap->size, err);
> -				goto err_map;
> -			}
> -			scm_done = true;
> -			inbuf.pageslen = 1;
> -		}
> -	}
>  
>  	fl->pd = USER_PD;
>  
> @@ -1370,8 +1346,15 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  	args[1].length = inbuf.namelen;
>  	args[1].fd = -1;
>  
> -	pages[0].addr = fl->cctx->remote_heap->dma_addr;
> -	pages[0].size = fl->cctx->remote_heap->size;
> +	if (!fl->cctx->audio_init_mem) {
> +		pages[0].addr = fl->cctx->remote_heap->dma_addr;
> +		pages[0].size = fl->cctx->remote_heap->size;

Do we need a flag? Can't we assume that remote_heap is always to be
allocated to the PD?

> +		fl->cctx->audio_init_mem = true;
> +		inbuf.pageslen = 1;
> +	} else {
> +		pages[0].addr = 0;
> +		pages[0].size = 0;
> +	}
>  
>  	args[2].ptr = (u64)(uintptr_t) pages;
>  	args[2].length = sizeof(*pages);
> @@ -1389,26 +1372,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>  
>  	return 0;
>  err_invoke:
> -	if (fl->cctx->vmcount && scm_done) {
> -		u64 src_perms = 0;
> -		struct qcom_scm_vmperm dst_perms;
> -		u32 i;
> -
> -		for (i = 0; i < fl->cctx->vmcount; i++)
> -			src_perms |= BIT(fl->cctx->vmperms[i].vmid);
> -
> -		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
> -		dst_perms.perm = QCOM_SCM_PERM_RWX;
> -		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
> -						(u64)fl->cctx->remote_heap->size,
> -						&src_perms, &dst_perms, 1);
> -		if (err)
> -			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr %pad size 0x%llx err %d\n",
> -				&fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
> -	}
> -err_map:
> -	fastrpc_buf_free(fl->cctx->remote_heap);
> -err_name:
> +	fl->cctx->audio_init_mem = false;
>  	kfree(name);
>  err:
>  	kfree(args);
> @@ -2396,7 +2360,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		}
>  	}
>  
> -	if (domain_id == SDSP_DOMAIN_ID) {
> +	if (domain_id == SDSP_DOMAIN_ID || domain_id == ADSP_DOMAIN_ID) {
>  		struct resource res;
>  		u64 src_perms;
>  
> @@ -2408,6 +2372,15 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  				    data->vmperms, data->vmcount);
>  		}
>  
> +		if (domain_id == ADSP_DOMAIN_ID) {
> +			data->remote_heap =
> +				kzalloc(sizeof(*data->remote_heap), GFP_KERNEL);
> +			if (!data->remote_heap)
> +				return -ENOMEM;
> +
> +			data->remote_heap->dma_addr = res.start;
> +			data->remote_heap->size = resource_size(&res);
> +		}
>  	}
>  
>  	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
> @@ -2488,10 +2461,13 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
>  	struct fastrpc_buf *buf, *b;
>  	struct fastrpc_user *user;
>  	unsigned long flags;
> +	bool skip_free = false;
> +	int err;
>  
>  	/* No invocations past this point */
>  	spin_lock_irqsave(&cctx->lock, flags);
>  	cctx->rpdev = NULL;
> +	cctx->audio_init_mem = false;
>  	list_for_each_entry(user, &cctx->users, user)
>  		fastrpc_notify_users(user);
>  	spin_unlock_irqrestore(&cctx->lock, flags);
> @@ -2505,7 +2481,26 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
>  	list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
>  		list_del(&buf->node);
>  
> -	fastrpc_buf_free(cctx->remote_heap);
> +	if (cctx->remote_heap) {
> +		if (cctx->vmcount) {
> +			u64 src_perms = 0;
> +			struct qcom_scm_vmperm dst_perms;
> +
> +			for (u32 i = 0; i < cctx->vmcount; i++)
> +				src_perms |= BIT(cctx->vmperms[i].vmid);
> +
> +			dst_perms.vmid = QCOM_SCM_VMID_HLOS;
> +			dst_perms.perm = QCOM_SCM_PERM_RWX;
> +
> +			err = qcom_scm_assign_mem(cctx->remote_heap->dma_addr,
> +						  cctx->remote_heap->size,
> +						  &src_perms, &dst_perms, 1);
> +			if (err)
> +				skip_free = true;
> +		}
> +		if (!skip_free)
> +			fastrpc_buf_free(cctx->remote_heap);
> +	}
>  
>  	of_platform_depopulate(&rpdev->dev);
>  
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry
Re: [PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
Posted by Jianping 1 week ago 

On 1/16/2026 4:49 AM, Dmitry Baryshkov wrote:
> On Thu, Jan 15, 2026 at 04:28:51PM +0800, Jianping Li wrote:
>> The entire reserved-memory region is now assigned to DSP VMIDs during
>> channel setup and stored in cctx->remote_heap. Memory is reclaimed in
>> rpmsg_remove by revoking DSP permissions and freeing the buffer, tying
>> heap lifecycle to the rpmsg channel.
> 
> Why?
The reason for allocating the entire reserved‑memory region is to avoid 
unsafe alloc/free
operations from user.

This design(alloc/free from user) was fragile because:
Userspace could request free while the DSP was still using the memory.
There was no reliable, kernel‑controlled way to know when the DSP had 
finished using pages.
Remote‑side “reverse fastrpc” free requests could not be safely trusted 
or validated.

Allocating the full reserved region upfront removes the need for alloc 
from user completely.
This way the free can be moved to fastrpc_rpmsg_remove(When DSP is 
shutting down).

And I will be adding this detail in commit text also.
> 
>>
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>> ---
>>   drivers/misc/fastrpc.c | 95 ++++++++++++++++++++----------------------
>>   1 file changed, 45 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 833c265add5e..f9edca7a8de1 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -278,6 +278,8 @@ struct fastrpc_channel_ctx {
>>   	struct kref refcount;
>>   	/* Flag if dsp attributes are cached */
>>   	bool valid_attributes;
>> +	/* Flag if audio PD init mem was allocated */
>> +	bool audio_init_mem;
>>   	u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
>>   	struct fastrpc_device *secure_fdevice;
>>   	struct fastrpc_device *fdevice;
>> @@ -1304,7 +1306,6 @@ 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 client_id;
>>   		u32 namelen;
>> @@ -1334,31 +1335,6 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   	inbuf.client_id = fl->client_id;
>>   	inbuf.namelen = init.namelen;
>>   	inbuf.pageslen = 0;
>> -	if (!fl->cctx->remote_heap) {
>> -		err = fastrpc_remote_heap_alloc(fl, fl->sctx->dev, init.memlen,
>> -						&fl->cctx->remote_heap);
>> -		if (err)
>> -			goto err_name;
>> -
>> -		/* Map if we have any heap VMIDs associated with this ADSP Static Process. */
>> -		if (fl->cctx->vmcount) {
>> -			u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>> -
>> -			err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>> -							(u64)fl->cctx->remote_heap->size,
>> -							&src_perms,
>> -							fl->cctx->vmperms, fl->cctx->vmcount);
>> -			if (err) {
>> -				dev_err(fl->sctx->dev,
>> -					"Failed to assign memory with dma_addr %pad size 0x%llx err %d\n",
>> -					&fl->cctx->remote_heap->dma_addr,
>> -					fl->cctx->remote_heap->size, err);
>> -				goto err_map;
>> -			}
>> -			scm_done = true;
>> -			inbuf.pageslen = 1;
>> -		}
>> -	}
>>   
>>   	fl->pd = USER_PD;
>>   
>> @@ -1370,8 +1346,15 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   	args[1].length = inbuf.namelen;
>>   	args[1].fd = -1;
>>   
>> -	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>> -	pages[0].size = fl->cctx->remote_heap->size;
>> +	if (!fl->cctx->audio_init_mem) {
>> +		pages[0].addr = fl->cctx->remote_heap->dma_addr;
>> +		pages[0].size = fl->cctx->remote_heap->size;
> 
> Do we need a flag? Can't we assume that remote_heap is always to be
> allocated to the PD?
We do need the audio_init_mem flag.
Once the PD starts and daemon takes the memory for the first time, PD 
will start using the memory,
meanwhile, the daemon can be killed and restarted. In this case, the 
memory is still with the PD and the next
daemon connection should not take any memory for the next request. This 
flag is maintained to ensure that.
The memory needs to be resent only if Audio PD on DSP restarts(due to 
PD-restart or Subsystem-restart)

> 
>> +		fl->cctx->audio_init_mem = true;
>> +		inbuf.pageslen = 1;
>> +	} else {
>> +		pages[0].addr = 0;
>> +		pages[0].size = 0;
>> +	}
>>   
>>   	args[2].ptr = (u64)(uintptr_t) pages;
>>   	args[2].length = sizeof(*pages);
>> @@ -1389,26 +1372,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>   
>>   	return 0;
>>   err_invoke:
>> -	if (fl->cctx->vmcount && scm_done) {
>> -		u64 src_perms = 0;
>> -		struct qcom_scm_vmperm dst_perms;
>> -		u32 i;
>> -
>> -		for (i = 0; i < fl->cctx->vmcount; i++)
>> -			src_perms |= BIT(fl->cctx->vmperms[i].vmid);
>> -
>> -		dst_perms.vmid = QCOM_SCM_VMID_HLOS;
>> -		dst_perms.perm = QCOM_SCM_PERM_RWX;
>> -		err = qcom_scm_assign_mem(fl->cctx->remote_heap->dma_addr,
>> -						(u64)fl->cctx->remote_heap->size,
>> -						&src_perms, &dst_perms, 1);
>> -		if (err)
>> -			dev_err(fl->sctx->dev, "Failed to assign memory dma_addr %pad size 0x%llx err %d\n",
>> -				&fl->cctx->remote_heap->dma_addr, fl->cctx->remote_heap->size, err);
>> -	}
>> -err_map:
>> -	fastrpc_buf_free(fl->cctx->remote_heap);
>> -err_name:
>> +	fl->cctx->audio_init_mem = false;
>>   	kfree(name);
>>   err:
>>   	kfree(args);
>> @@ -2396,7 +2360,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>   		}
>>   	}
>>   
>> -	if (domain_id == SDSP_DOMAIN_ID) {
>> +	if (domain_id == SDSP_DOMAIN_ID || domain_id == ADSP_DOMAIN_ID) {
>>   		struct resource res;
>>   		u64 src_perms;
>>   
>> @@ -2408,6 +2372,15 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>   				    data->vmperms, data->vmcount);
>>   		}
>>   
>> +		if (domain_id == ADSP_DOMAIN_ID) {
>> +			data->remote_heap =
>> +				kzalloc(sizeof(*data->remote_heap), GFP_KERNEL);
>> +			if (!data->remote_heap)
>> +				return -ENOMEM;
>> +
>> +			data->remote_heap->dma_addr = res.start;
>> +			data->remote_heap->size = resource_size(&res);
>> +		}
>>   	}
>>   
>>   	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
>> @@ -2488,10 +2461,13 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
>>   	struct fastrpc_buf *buf, *b;
>>   	struct fastrpc_user *user;
>>   	unsigned long flags;
>> +	bool skip_free = false;
>> +	int err;
>>   
>>   	/* No invocations past this point */
>>   	spin_lock_irqsave(&cctx->lock, flags);
>>   	cctx->rpdev = NULL;
>> +	cctx->audio_init_mem = false;
>>   	list_for_each_entry(user, &cctx->users, user)
>>   		fastrpc_notify_users(user);
>>   	spin_unlock_irqrestore(&cctx->lock, flags);
>> @@ -2505,7 +2481,26 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
>>   	list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
>>   		list_del(&buf->node);
>>   
>> -	fastrpc_buf_free(cctx->remote_heap);
>> +	if (cctx->remote_heap) {
>> +		if (cctx->vmcount) {
>> +			u64 src_perms = 0;
>> +			struct qcom_scm_vmperm dst_perms;
>> +
>> +			for (u32 i = 0; i < cctx->vmcount; i++)
>> +				src_perms |= BIT(cctx->vmperms[i].vmid);
>> +
>> +			dst_perms.vmid = QCOM_SCM_VMID_HLOS;
>> +			dst_perms.perm = QCOM_SCM_PERM_RWX;
>> +
>> +			err = qcom_scm_assign_mem(cctx->remote_heap->dma_addr,
>> +						  cctx->remote_heap->size,
>> +						  &src_perms, &dst_perms, 1);
>> +			if (err)
>> +				skip_free = true;
>> +		}
>> +		if (!skip_free)
>> +			fastrpc_buf_free(cctx->remote_heap);
>> +	}
>>   
>>   	of_platform_depopulate(&rpdev->dev);
>>   
>> -- 
>> 2.43.0
>>
>
Re: [PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
Posted by Dmitry Baryshkov 5 days, 13 hours ago 
On Mon, Feb 02, 2026 at 03:06:59PM +0800, Jianping wrote:
> 
> 
> On 1/16/2026 4:49 AM, Dmitry Baryshkov wrote:
> > On Thu, Jan 15, 2026 at 04:28:51PM +0800, Jianping Li wrote:
> > > The entire reserved-memory region is now assigned to DSP VMIDs during
> > > channel setup and stored in cctx->remote_heap. Memory is reclaimed in
> > > rpmsg_remove by revoking DSP permissions and freeing the buffer, tying
> > > heap lifecycle to the rpmsg channel.
> > 
> > > @@ -1370,8 +1346,15 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > >   	args[1].length = inbuf.namelen;
> > >   	args[1].fd = -1;
> > > -	pages[0].addr = fl->cctx->remote_heap->dma_addr;
> > > -	pages[0].size = fl->cctx->remote_heap->size;
> > > +	if (!fl->cctx->audio_init_mem) {
> > > +		pages[0].addr = fl->cctx->remote_heap->dma_addr;
> > > +		pages[0].size = fl->cctx->remote_heap->size;
> > 
> > Do we need a flag? Can't we assume that remote_heap is always to be
> > allocated to the PD?
> We do need the audio_init_mem flag.
> Once the PD starts and daemon takes the memory for the first time, PD will
> start using the memory,
> meanwhile, the daemon can be killed and restarted. In this case, the memory
> is still with the PD and the next
> daemon connection should not take any memory for the next request. This flag
> is maintained to ensure that.
> The memory needs to be resent only if Audio PD on DSP restarts(due to
> PD-restart or Subsystem-restart)

This needs to be explained in the changelog.

> 
> > 
> > > +		fl->cctx->audio_init_mem = true;

What if there are two racing IOCTLs, trying to init AudioPD process?

> > > +		inbuf.pageslen = 1;
> > > +	} else {
> > > +		pages[0].addr = 0;
> > > +		pages[0].size = 0;
> > > +	}
> > >   	args[2].ptr = (u64)(uintptr_t) pages;
> > >   	args[2].length = sizeof(*pages);

-- 
With best wishes
Dmitry
Re: [PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
Posted by Jianping 4 days, 1 hour ago 

On 2/4/2026 5:19 AM, Dmitry Baryshkov wrote:
> On Mon, Feb 02, 2026 at 03:06:59PM +0800, Jianping wrote:
>>
>>
>> On 1/16/2026 4:49 AM, Dmitry Baryshkov wrote:
>>> On Thu, Jan 15, 2026 at 04:28:51PM +0800, Jianping Li wrote:
>>>> The entire reserved-memory region is now assigned to DSP VMIDs during
>>>> channel setup and stored in cctx->remote_heap. Memory is reclaimed in
>>>> rpmsg_remove by revoking DSP permissions and freeing the buffer, tying
>>>> heap lifecycle to the rpmsg channel.
>>>
>>>> @@ -1370,8 +1346,15 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>>>    	args[1].length = inbuf.namelen;
>>>>    	args[1].fd = -1;
>>>> -	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>>>> -	pages[0].size = fl->cctx->remote_heap->size;
>>>> +	if (!fl->cctx->audio_init_mem) {
>>>> +		pages[0].addr = fl->cctx->remote_heap->dma_addr;
>>>> +		pages[0].size = fl->cctx->remote_heap->size;
>>>
>>> Do we need a flag? Can't we assume that remote_heap is always to be
>>> allocated to the PD?
>> We do need the audio_init_mem flag.
>> Once the PD starts and daemon takes the memory for the first time, PD will
>> start using the memory,
>> meanwhile, the daemon can be killed and restarted. In this case, the memory
>> is still with the PD and the next
>> daemon connection should not take any memory for the next request. This flag
>> is maintained to ensure that.
>> The memory needs to be resent only if Audio PD on DSP restarts(due to
>> PD-restart or Subsystem-restart)
> 
> This needs to be explained in the changelog.
I will add it.
> 
>>
>>>
>>>> +		fl->cctx->audio_init_mem = true;
> 
> What if there are two racing IOCTLs, trying to init AudioPD process?

There may be a chance that two threads enter ioctl at the same time, and 
seeing that audio_init_mem is false causes it to send twice, so a 
cctx->lock needs to be added here to ensure that two threads do not 
enter simultaneously.

> 
>>>> +		inbuf.pageslen = 1;
>>>> +	} else {
>>>> +		pages[0].addr = 0;
>>>> +		pages[0].size = 0;
>>>> +	}
>>>>    	args[2].ptr = (u64)(uintptr_t) pages;
>>>>    	args[2].length = sizeof(*pages);
>
Re: [PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
Posted by Bjorn Andersson 5 days, 12 hours ago 
On Tue, Feb 03, 2026 at 11:19:39PM +0200, Dmitry Baryshkov wrote:
> On Mon, Feb 02, 2026 at 03:06:59PM +0800, Jianping wrote:
> > 
> > 
> > On 1/16/2026 4:49 AM, Dmitry Baryshkov wrote:
> > > On Thu, Jan 15, 2026 at 04:28:51PM +0800, Jianping Li wrote:
> > > > The entire reserved-memory region is now assigned to DSP VMIDs during
> > > > channel setup and stored in cctx->remote_heap. Memory is reclaimed in
> > > > rpmsg_remove by revoking DSP permissions and freeing the buffer, tying
> > > > heap lifecycle to the rpmsg channel.
> > > 
> > > > @@ -1370,8 +1346,15 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
> > > >   	args[1].length = inbuf.namelen;
> > > >   	args[1].fd = -1;
> > > > -	pages[0].addr = fl->cctx->remote_heap->dma_addr;
> > > > -	pages[0].size = fl->cctx->remote_heap->size;
> > > > +	if (!fl->cctx->audio_init_mem) {
> > > > +		pages[0].addr = fl->cctx->remote_heap->dma_addr;
> > > > +		pages[0].size = fl->cctx->remote_heap->size;
> > > 
> > > Do we need a flag? Can't we assume that remote_heap is always to be
> > > allocated to the PD?
> > We do need the audio_init_mem flag.
> > Once the PD starts and daemon takes the memory for the first time, PD will
> > start using the memory,
> > meanwhile, the daemon can be killed and restarted. In this case, the memory
> > is still with the PD and the next
> > daemon connection should not take any memory for the next request. This flag
> > is maintained to ensure that.
> > The memory needs to be resent only if Audio PD on DSP restarts(due to
> > PD-restart or Subsystem-restart)
> 
> This needs to be explained in the changelog.
> 

Not in the "changelog", in the commit message.


@Jianping please read https://docs.kernel.org/process/submitting-patches.html#describe-your-changes 

Your commit messages should "Describe your problem". From this
description it should be clear why the change is needed and why the
change is done in the specific way.

Regards,
Bjorn

> > 
> > > 
> > > > +		fl->cctx->audio_init_mem = true;
> 
> What if there are two racing IOCTLs, trying to init AudioPD process?
> 
> > > > +		inbuf.pageslen = 1;
> > > > +	} else {
> > > > +		pages[0].addr = 0;
> > > > +		pages[0].size = 0;
> > > > +	}
> > > >   	args[2].ptr = (u64)(uintptr_t) pages;
> > > >   	args[2].length = sizeof(*pages);
> 
> -- 
> With best wishes
> Dmitry
>
Re: [PATCH v2 4/4] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe
Posted by Jianping 4 days, 1 hour ago 

On 2/4/2026 5:42 AM, Bjorn Andersson wrote:
> On Tue, Feb 03, 2026 at 11:19:39PM +0200, Dmitry Baryshkov wrote:
>> On Mon, Feb 02, 2026 at 03:06:59PM +0800, Jianping wrote:
>>>
>>>
>>> On 1/16/2026 4:49 AM, Dmitry Baryshkov wrote:
>>>> On Thu, Jan 15, 2026 at 04:28:51PM +0800, Jianping Li wrote:
>>>>> The entire reserved-memory region is now assigned to DSP VMIDs during
>>>>> channel setup and stored in cctx->remote_heap. Memory is reclaimed in
>>>>> rpmsg_remove by revoking DSP permissions and freeing the buffer, tying
>>>>> heap lifecycle to the rpmsg channel.
>>>>
>>>>> @@ -1370,8 +1346,15 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>>>>>    	args[1].length = inbuf.namelen;
>>>>>    	args[1].fd = -1;
>>>>> -	pages[0].addr = fl->cctx->remote_heap->dma_addr;
>>>>> -	pages[0].size = fl->cctx->remote_heap->size;
>>>>> +	if (!fl->cctx->audio_init_mem) {
>>>>> +		pages[0].addr = fl->cctx->remote_heap->dma_addr;
>>>>> +		pages[0].size = fl->cctx->remote_heap->size;
>>>>
>>>> Do we need a flag? Can't we assume that remote_heap is always to be
>>>> allocated to the PD?
>>> We do need the audio_init_mem flag.
>>> Once the PD starts and daemon takes the memory for the first time, PD will
>>> start using the memory,
>>> meanwhile, the daemon can be killed and restarted. In this case, the memory
>>> is still with the PD and the next
>>> daemon connection should not take any memory for the next request. This flag
>>> is maintained to ensure that.
>>> The memory needs to be resent only if Audio PD on DSP restarts(due to
>>> PD-restart or Subsystem-restart)
>>
>> This needs to be explained in the changelog.
>>
> 
> Not in the "changelog", in the commit message.
> 
> 
> @Jianping please read https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> 
> Your commit messages should "Describe your problem". From this
> description it should be clear why the change is needed and why the
> change is done in the specific way.
> 
> Regards,
> Bjorn

Thanks Bjorn for the reminder, I will revise my commit message.

Thanks,
Jianping

> 
>>>
>>>>
>>>>> +		fl->cctx->audio_init_mem = true;
>>
>> What if there are two racing IOCTLs, trying to init AudioPD process?
>>
>>>>> +		inbuf.pageslen = 1;
>>>>> +	} else {
>>>>> +		pages[0].addr = 0;
>>>>> +		pages[0].size = 0;
>>>>> +	}
>>>>>    	args[2].ptr = (u64)(uintptr_t) pages;
>>>>>    	args[2].length = sizeof(*pages);
>>
>> -- 
>> With best wishes
>> Dmitry
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.