mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
To: Anandu Krishnan E <anandu.e@oss.qualcomm.com>,
	Srinivas Kandagatla <srini@kernel.org>,
	Amol Maheshwari <amahesh@qti.qualcomm.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, quic_bkumar@quicinc.com,
	quic_chennak@quicinc.com
Subject: Re: [PATCH] misc: fastrpc: map ADSP remote heap into remoteproc IOMMU domain
Date: Mon, 27 Jul 2026 12:19:54 +0530	[thread overview]
Message-ID: <d3e8023c-cc52-4617-8095-cf4bfe109391@oss.qualcomm.com> (raw)
In-Reply-To: <20260618-audio_fix_clean_v3-v1-1-ec1ee66fe455@oss.qualcomm.com>

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@oss.qualcomm.com
> 
> Link: https://lore.kernel.org/r/20260609025938.457-1-jianping.li@oss.qualcomm.com
> Signed-off-by: Anandu Krishnan E <anandu.e@oss.qualcomm.com>
> ---
> This patch depends on:
> 
>   [PATCH v8 0/4] misc: fastrpc: Add missing bug fixes
>   https://lore.kernel.org/r/20260609025938.457-1-jianping.li@oss.qualcomm.com
> 
> 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;
>  	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?> +			}
> +		}
>  	}
>  
> -	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@oss.qualcomm.com>
> 


  reply	other threads:[~2026-07-27  6:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-18  9:51 Anandu Krishnan E
2026-07-27  6:49 ` Ekansh Gupta [this message]
2026-07-27  7:10   ` Anandu Krishnan E

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d3e8023c-cc52-4617-8095-cf4bfe109391@oss.qualcomm.com \
    --to=ekansh.gupta@oss.qualcomm.com \
    --cc=amahesh@qti.qualcomm.com \
    --cc=anandu.e@oss.qualcomm.com \
    --cc=arnd@arndb.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quic_bkumar@quicinc.com \
    --cc=quic_chennak@quicinc.com \
    --cc=srini@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®