Re: [PATCH] misc: fastrpc: map ADSP remote heap into remoteproc IOMMU domain

[Anandu Krishnan E]

On 27-07-2026 12:19 pm, Ekansh Gupta wrote:
> On 18-06-2026 15:21, Anandu Krishnan E wrote:
>> On KVM-based targets the kernel runs at EL2 without a separate
>> hypervisor to manage inter-VM memory access control.  In this
>> configuration the remoteproc is assigned its own IOMMU domain, and
>> any memory carveout the DSP must access requires an explicit mapping
>> into that domain before the DSP can reach it.
>>
>> The existing code calls qcom_scm_assign_mem() to transfer ownership
>> of the ADSP remote heap carveout from HLOS to the DSP VM.  This SCM
>> call is only meaningful when a separate hypervisor (e.g. Gunyah) is
>> present to enforce inter-VM memory access control.  On KVM-based
>> targets no such hypervisor exists, so the carveout must instead be
>> mapped into the remoteproc's IOMMU domain via an identity mapping
>> (IOVA == PA) using iommu_map().  Without this mapping the DSP
>> triggers an SMMU translation fault when accessing the remote heap
>> during audio PD static process creation.
>>
>> Detect whether the remoteproc has an IOMMU by checking for the
>> "iommus" property on the remoteproc DT node and store the result in
>> a new has_iommu flag in fastrpc_channel_ctx.  When the flag is set,
>> map the carveout into the remoteproc's IOMMU domain instead of
>> calling qcom_scm_assign_mem().  Introduce fastrpc_remote_heap_map()
>> and fastrpc_remote_heap_unmap() helpers to encapsulate the IOMMU
>> domain lookup and map/unmap operations.
>>
>> This change relies on the remote heap handling introduced in:
>> https://lore.kernel.org/r/20260609025938.457-1-jianping.li@xxxxxxxxxxxxxxxx
>>
>> Link: https://lore.kernel.org/r/20260609025938.457-1-jianping.li@xxxxxxxxxxxxxxxx
>> Signed-off-by: Anandu Krishnan E <anandu.e@xxxxxxxxxxxxxxxx>
>> ---
>> This patch depends on:
>>
>>   [PATCH v8 0/4] misc: fastrpc: Add missing bug fixes
>>   https://lore.kernel.org/r/20260609025938.457-1-jianping.li@xxxxxxxxxxxxxxxx
>>
>> Please apply after that series.
>> ---
>>  drivers/misc/fastrpc.c | 141 +++++++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 114 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index a5af901c1cb8..9ce7af2091f1 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -21,6 +21,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/firmware/qcom/qcom_scm.h>
>>  #include <uapi/misc/fastrpc.h>
>> +#include <linux/iommu.h>
>>  #include <linux/of_reserved_mem.h>
>>  #include <linux/bits.h>
>>  
>> @@ -285,6 +286,8 @@ struct fastrpc_channel_ctx {
>>  	struct list_head invoke_interrupted_mmaps;
>>  	bool secure;
>>  	bool unsigned_support;
>> +	/* set when remoteproc has an IOMMU; use iommu_map instead of hyp_assign */
>> +	bool has_iommu;
> maybe something like use_iommu_map will be more meaningful here?>  	u64
> dma_mask;
sure will update 
>>  	const struct fastrpc_soc_data *soc_data;
>>  };
>> @@ -2343,10 +2346,65 @@ static const struct fastrpc_soc_data default_soc_data = {
>>  	.dma_addr_bits_default = 32,
>>  };
>>  
>> +static int fastrpc_remote_heap_map(struct device *rdev,
>> +				   struct device_node *rproc_node,
>> +				   struct fastrpc_buf *heap)
>> +{
>> +	struct platform_device *rproc_pdev;
>> +	struct iommu_domain *domain;
>> +	int ret;
>> +
>> +	rproc_pdev = of_find_device_by_node(rproc_node);
>> +	if (!rproc_pdev) {
>> +		dev_err(rdev, "failed to find remoteproc platform device\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	domain = iommu_get_domain_for_dev(&rproc_pdev->dev);
>> +	if (!domain) {
>> +		put_device(&rproc_pdev->dev);
>> +		dev_err(rdev, "no IOMMU domain for remoteproc\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	ret = iommu_map(domain, heap->dma_addr, heap->dma_addr, heap->size,
>> +			IOMMU_READ | IOMMU_WRITE, GFP_KERNEL);
>> +	if (ret)
>> +		dev_err(rdev, "failed to map remote heap phys=0x%llx size=0x%llx err=%d\n",
>> +			heap->dma_addr, heap->size, ret);
>> +
>> +	put_device(&rproc_pdev->dev);
>> +	return ret;
>> +}
>> +
>> +static void fastrpc_remote_heap_unmap(struct rpmsg_device *rpdev,
>> +				      struct fastrpc_buf *heap)
>> +{
>> +	struct device_node *rproc_node;
>> +	struct platform_device *rproc_pdev;
>> +	struct iommu_domain *domain;
>> +
>> +	rproc_node = of_get_parent(of_get_parent(rpdev->dev.of_node));
>> +	if (!rproc_node)
>> +		return;
>> +
>> +	rproc_pdev = of_find_device_by_node(rproc_node);
>> +	of_node_put(rproc_node);
>> +	if (!rproc_pdev)
>> +		return;
>> +
>> +	domain = iommu_get_domain_for_dev(&rproc_pdev->dev);
>> +	if (domain)
>> +		iommu_unmap(domain, heap->dma_addr, heap->size);
>> +
>> +	put_device(&rproc_pdev->dev);
>> +}
>> +
>>  static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>  {
>>  	struct device *rdev = &rpdev->dev;
>>  	struct fastrpc_channel_ctx *data;
>> +	struct device_node *rproc_node;
>>  	int i, err, domain_id = -1, vmcount;
>>  	const char *domain;
>>  	bool secure_dsp;
>> @@ -2390,9 +2448,12 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>  		}
>>  	}
>>  
>> +	rproc_node = of_get_parent(of_get_parent(rdev->of_node));
>> +	if (rproc_node)
>> +		data->has_iommu = of_property_present(rproc_node, "iommus");
>> +
>>  	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) {
>> @@ -2401,21 +2462,41 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>  					kzalloc_obj(*data->remote_heap, GFP_KERNEL);
>>  				if (!data->remote_heap) {
>>  					err = -ENOMEM;
>> -					goto err_free_data;
>> +					goto err_put_node;
>>  				}
>>  
>>  				data->remote_heap->dma_addr = res.start;
>>  				data->remote_heap->size = resource_size(&res);
>> +
>> +				if (data->has_iommu) {
>> +					err = fastrpc_remote_heap_map(rdev,
>> +								      rproc_node,
>> +								      data->remote_heap);
>> +					if (err) {
>> +						kfree(data->remote_heap);
>> +						data->remote_heap = NULL;
>> +						goto err_put_node;
>> +					}
>> +				}
>>  			}
>> -			src_perms = BIT(QCOM_SCM_VMID_HLOS);
>>  
>> -			err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
>> -				    data->vmperms, data->vmcount);
>> -			if (err)
>> -				goto err_free_data;
>> +			if (!data->has_iommu) {
>> +				u64 src_perms = BIT(QCOM_SCM_VMID_HLOS);
>> +
>> +				err = qcom_scm_assign_mem(res.start,
>> +							  resource_size(&res),
>> +							  &src_perms,
>> +							  data->vmperms,
>> +							  data->vmcount);
>> +				if (err)
>> +					goto err_put_node;
>> +			}
>>  		}
>>  	}
>>  
>> +	of_node_put(rproc_node);
>> +	rproc_node = NULL;
>> +
>>  	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
>>  	data->secure = secure_dsp;
>>  	data->soc_data = soc_data;
>> @@ -2471,6 +2552,9 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>  	if (data->secure_fdevice)
>>  		misc_deregister(&data->secure_fdevice->miscdev);
>>  
>> +err_put_node:
>> +	of_node_put(rproc_node);
>> +
>>  err_free_data:
>>  	kfree(data->remote_heap);
>>  	kfree(data);
>> @@ -2513,28 +2597,31 @@ 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);
>>  
>> -	if (cctx->remote_heap && 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)
>> -			dev_err(&rpdev->dev,
>> -				"Failed to assign memory back to HLOS: dma_addr %pad size %#llx err %d\n",
>> -				&cctx->remote_heap->dma_addr, cctx->remote_heap->size, err);
>> +	if (cctx->remote_heap) {
>> +		if (cctx->has_iommu) {
>> +			fastrpc_remote_heap_unmap(rpdev, cctx->remote_heap);
>> +			kfree(cctx->remote_heap);
>> +			cctx->remote_heap = NULL;
>> +		} else 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) {
>> +				kfree(cctx->remote_heap);
>> +				cctx->remote_heap = NULL;
> why is this needed under error check? Won't it lead to memory leak? > +			}
> yes , will udpate and share new version
>> +		}
>>  	}
>>  
>> -	kfree(cctx->remote_heap);
>> -	cctx->remote_heap = NULL;
>> -
>>  	of_platform_depopulate(&rpdev->dev);
>>  
>>  	fastrpc_channel_ctx_put(cctx);
>>
>> ---
>> base-commit: f1256704b88067430e255e5c85d6b727e1bbb2ea
>> change-id: 20260618-audio_fix_clean_v3-9df607546095
>>
>> Best regards,
>> --  
>> Anandu Krishnan E <anandu.e@xxxxxxxxxxxxxxxx>
>>