Re: [PATCH v11] misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

[Jianping <jianping.li@oss.qualcomm.com>]

在 2026/7/31 22:17, Dmitry Baryshkov 写道:
> On Fri, Jul 31, 2026 at 05:32:10PM +0800, Jianping Li wrote:
>> Allocating and freeing Audio PD memory from userspace is unsafe because
>> the kernel cannot reliably determine when the DSP has finished using the
>> memory. Userspace may free buffers while they are still in use by the DSP,
>> and remote free requests cannot be safely trusted.
>>
>> Additionally, the current implementation allows userspace to repeatedly
>> grow the Audio PD heap, but does not support shrinking it. This can lead
>> to unbounded memory usage over time, effectively causing a memory leak.
>>
>> Fix this by allocating the entire Audio PD reserved-memory region during
>> rpmsg probe and tying its lifetime to the rpmsg channel. This removes
>> userspace-controlled alloc/free and ensures that memory is reclaimed only
>> when the DSP process is torn down.
>>
>> Validate the presence of the Audio PD reserved-memory region during
> 
> This will break compatibility with existing DTs, which is a no-go.
> Existing DTs _must_ continue to work.
You're right.

I'll keep the existing behavior and only use the reserved-memory region
when it is present, instead of failing the probe.

Also move the check to create_static_process
> 
>> rpmsg probe and fail early if it is missing, so that a misconfigured
>> device tree is caught at probe time instead of at process creation.
>>
>> Fixes: 0871561055e66 ("misc: fastrpc: Add support for audiopd")
>> Cc: stable@kernel.org
>> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
>>
>> Patch [v10]: https://lore.kernel.org/all/20260716095847.479-1-jianping.li@oss.qualcomm.com/
> 
> All of this should go under the --- line.
Ack, I'll move the version history below the --- line.
> 
>>
>> @@ -2584,12 +2547,22 @@ 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;
>>   
>>   		err = of_reserved_mem_region_to_resource(rdev->of_node, 0, &res);
>> +
>> +		if (err && domain_id == ADSP_DOMAIN_ID) {
>> +			dev_err(rdev, "missing mandatory remote heap memory-region\n");
>> +			goto err_free_data;
>> +		}
> 
> This is what I mean. This has been working beforehand. It must continue
> to work.
ACK. Will remove the checks in probe and ensure probe can continue to 
execute.
> 
>> +
>>   		if (!err) {
>> +			if (domain_id == ADSP_DOMAIN_ID) {
>> +				data->remote_heap_addr = res.start;
>> +				data->remote_heap_size = resource_size(&res);
>> +			}
> 
> Too much of the spaghetty code. Can we replace all domain checks with
> the functions?
I agree.

I'll look at introducing helper functions describing
the per-domain capabilities instead of open-coding ADSP/SDSP checks
throughout the driver.
> 
>>   			src_perms = BIT(QCOM_SCM_VMID_HLOS);
>>   
>>   			err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
>