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>,
	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,
	Amol Maheshwari <amahesh@qti.qualcomm.com>,
	quic_bkumar@quicinc.com, quic_chennak@quicinc.com
Subject: Re: [PATCH v2] misc: fastrpc: map ADSP remote heap into remoteproc IOMMU domain
Date: Wed, 14 Oct 2026 10:51:44 +0530	[thread overview]
Message-ID: <cf34e58c-29cd-411e-b2fd-e5c6e2fcac08@oss.qualcomm.com> (raw)
In-Reply-To: <20261001-fastrpc-kvm-iommu-v2-v2-1-6c32ad6f662f@oss.qualcomm.com>

On 01-10-2026 20:30, 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 use_iommu_map 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.
> 
> Signed-off-by: Anandu Krishnan E <anandu.e@oss.qualcomm.com>
> To: Srinivas Kandagatla <srini@kernel.org>
> To: Amol Maheshwari <amahesh@qti.qualcomm.com>
> To: Arnd Bergmann <arnd@arndb.de>
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Cc: ekansh.gupta@oss.qualcomm.com
> Cc: quic_bkumar@quicinc.com
> Cc: quic_chennak@quicinc.com
> ---
> Changes in v2:
> - Rebase onto the fastrpc-for-next tree, which now has the remote
>   heap allocation and reserved-memory handling changes.
> - Rename has_iommu to use_iommu_map for clarity.
> - In fastrpc_rpmsg_remove(), the IOMMU unmap path now mirrors the
>   qcom_scm_assign_mem() path exactly, so the two are handled
>   consistently instead of leaving the IOMMU case with different
>   cleanup semantics.
> - Add fastrpc_get_rproc_node() to fix a device_node refcount leak:
>   the previous of_get_parent(of_get_parent(node)) pattern never put
>   the intermediate parent node.
> - Unmap the remote heap from the remoteproc's IOMMU domain if probe
>   fails after the mapping succeeds, instead of leaking the mapping
>   and causing -EEXIST on every subsequent probe attempt.
> - Log a warning in fastrpc_remote_heap_unmap() on each failure path
>   (remoteproc node/platform device/IOMMU domain not found, or a
>   partial iommu_unmap()), instead of silently leaving the mapping in
>   place with no diagnostic.
> 
> Link to v1: https://lore.kernel.org/r/20260618-audio_fix_clean_v3-v1-1-ec1ee66fe455@oss.qualcomm.com
> ---
>  drivers/misc/fastrpc.c | 149 ++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 128 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 5ac7b3e78ba7..89f4a80ad9be 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>
nit: follow alphabetical order>  #include <linux/of_reserved_mem.h>
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
> @@ -317,6 +318,8 @@ struct fastrpc_channel_ctx {
>  	/* Audio PD reserved remote heap region */
>  	phys_addr_t remote_heap_addr;
>  	u64 remote_heap_size;
> +	/* set when remoteproc has an IOMMU; use iommu_map instead of hyp_assign */
> +	bool use_iommu_map;
>  	u32 dsp_attributes[FASTRPC_MAX_DSP_ATTRIBUTES];
>  	struct fastrpc_device *secure_fdevice;
>  	struct fastrpc_device *fdevice;
> @@ -2507,9 +2510,90 @@ static const struct of_device_id fastrpc_poll_supported_machines[] __maybe_unuse
>  	{},
>  };
>  
> +static struct device_node *fastrpc_get_rproc_node(struct device_node *node)
> +{
> +	struct device_node *parent, *rproc_node;
> +
> +	parent = of_get_parent(node);
> +	if (!parent)
> +		return NULL;
> +
> +	rproc_node = of_get_parent(parent);
> +	of_node_put(parent);
> +
> +	return rproc_node;
> +}
> +
> +static int fastrpc_remote_heap_map(struct device *rdev,
> +				   struct device_node *rproc_node,
> +				   phys_addr_t addr, u64 size)
> +{
> +	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, addr, addr, 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",
> +			(u64)addr, size, ret);
> +
> +	put_device(&rproc_pdev->dev);
> +	return ret;
> +}
> +
> +static void fastrpc_remote_heap_unmap(struct rpmsg_device *rpdev,
> +				      phys_addr_t addr, u64 size)
> +{
> +	struct device_node *rproc_node;
> +	struct platform_device *rproc_pdev;
> +	struct iommu_domain *domain;
> +	size_t unmapped;
> +
> +	rproc_node = fastrpc_get_rproc_node(rpdev->dev.of_node);
> +	if (!rproc_node) {
> +		dev_warn(&rpdev->dev, "failed to find remoteproc node for heap unmap\n");
> +		return;
> +	}
> +
> +	rproc_pdev = of_find_device_by_node(rproc_node);
> +	of_node_put(rproc_node);
> +	if (!rproc_pdev) {
> +		dev_warn(&rpdev->dev, "failed to find remoteproc platform device for heap unmap\n");
> +		return;
> +	}
> +
> +	domain = iommu_get_domain_for_dev(&rproc_pdev->dev);
> +	if (!domain) {
> +		dev_warn(&rpdev->dev, "no IOMMU domain for remoteproc during heap unmap\n");
> +		put_device(&rproc_pdev->dev);
> +		return;
> +	}
> +
> +	unmapped = iommu_unmap(domain, addr, size);
> +	if (unmapped != size)
> +		dev_warn(&rpdev->dev, "partial/failed heap unmap: requested=0x%llx unmapped=0x%zx\n",
> +			 size, unmapped);
> +
> +	put_device(&rproc_pdev->dev);
> +}
> +
>  static int fastrpc_init_reserved_mem(struct fastrpc_channel_ctx *cctx,
>  				     struct device *rdev, u32 domain_id)
>  {
> +	struct device_node *rproc_node;
>  	struct resource res;
>  	u64 src_perms;
>  	int err;
> @@ -2536,6 +2620,19 @@ static int fastrpc_init_reserved_mem(struct fastrpc_channel_ctx *cctx,
>  	if (domain_id == ADSP_DOMAIN_ID) {
>  		cctx->remote_heap_addr = res.start;
>  		cctx->remote_heap_size = resource_size(&res);
> +
> +		rproc_node = fastrpc_get_rproc_node(rdev->of_node);
> +		if (rproc_node)
> +			cctx->use_iommu_map = of_property_present(rproc_node, "iommus");
> +
> +		if (cctx->use_iommu_map) {
> +			err = fastrpc_remote_heap_map(rdev, rproc_node, res.start,
> +						      resource_size(&res));
> +			of_node_put(rproc_node);
> +			return err;
> +		}
> +
> +		of_node_put(rproc_node);
>  	}
>  
>  	if (!cctx->vmcount)
> @@ -2611,7 +2708,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		data->unsigned_support = false;
>  		err = fastrpc_device_register(rdev, data, secure_dsp, domain);
>  		if (err)
> -			goto err_free_data;
> +			goto err_unmap_heap;
>  		break;
>  	case CDSP_DOMAIN_ID:
>  	case GDSP_DOMAIN_ID:
> @@ -2619,7 +2716,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		/* Create both device nodes so that we can allow both Signed and Unsigned PD */
>  		err = fastrpc_device_register(rdev, data, true, domain);
>  		if (err)
> -			goto err_free_data;
> +			goto err_unmap_heap;
>  
>  		err = fastrpc_device_register(rdev, data, false, domain);
>  		if (err)
> @@ -2627,7 +2724,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		break;
>  	default:
>  		err = -EINVAL;
> -		goto err_free_data;
> +		goto err_unmap_heap;
>  	}
>  
>  	kref_init(&data->refcount);
> @@ -2655,6 +2752,11 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  	if (data->secure_fdevice)
>  		misc_deregister(&data->secure_fdevice->miscdev);
>  
> +err_unmap_heap:
> +	if (data->use_iommu_map && data->remote_heap_size)
> +		fastrpc_remote_heap_unmap(rpdev, data->remote_heap_addr,
> +					  data->remote_heap_size);
what about hyp-unassign in case of Gunyah?> +
>  err_free_data:
>  	kfree(data);
>  	return err;
> @@ -2695,24 +2797,29 @@ 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_size && cctx->vmcount) {
> -		u64 src_perms = 0;
> -		int err, i;
> -		struct qcom_scm_vmperm dst_perms;
> -
> -		for (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_addr,
> -					  cctx->remote_heap_size, &src_perms,
> -					  &dst_perms, 1);
> -		if (err)
> -			dev_err(&rpdev->dev,
> -				"Failed to assign memory back to HLOS: addr %pa size %#llx err %d\n",
> -				&cctx->remote_heap_addr, cctx->remote_heap_size, err);
> +	if (cctx->remote_heap_size) {
> +		if (cctx->use_iommu_map) {
> +			fastrpc_remote_heap_unmap(rpdev, cctx->remote_heap_addr,
> +						  cctx->remote_heap_size);
> +		} else if (cctx->vmcount) {
> +			u64 src_perms = 0;
> +			int err, i;
> +			struct qcom_scm_vmperm dst_perms;
> +
> +			for (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_addr,
> +						  cctx->remote_heap_size, &src_perms,
> +						  &dst_perms, 1);
> +			if (err)
> +				dev_err(&rpdev->dev,
> +					"Failed to assign memory back to HLOS: addr %pa size %#llx err %d\n",
> +					&cctx->remote_heap_addr, cctx->remote_heap_size, err);
> +		}
>  	}
try moving this block to helper function as same can be used in
fastrpc_rpmsg_probe() error case. Maybe something like this:

static void fastrpc_remote_heap_release(struct fastrpc_channel_ctx
					*cctx, struct rpmsg_device
					*rpdev)
{
	if (!cctx->remote_heap_size)
		return;
	if (cctx->use_iommu_map)
		fastrpc_remote_heap_unmap(rpdev, ...);
	else if (cctx->vmcount)
		fastrpc_remote_heap_unassign(cctx, rpdev);
}
>  
>  	of_platform_depopulate(&rpdev->dev);
> 
> ---
> base-commit: ef071c4906eb45d16b60f09154cf0bc6ec8f5435
> change-id: 20260930-fastrpc-kvm-iommu-v2-1a7d343ed802
> 
> Best regards,
> --  
> Anandu Krishnan E <anandu.e@oss.qualcomm.com>


      parent reply	other threads:[~2026-10-14  5:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-01 15:00 Anandu Krishnan E
2026-10-01 15:11 ` Konrad Dybcio
2026-10-14  5:21 ` Ekansh Gupta [this message]

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=cf34e58c-29cd-411e-b2fd-e5c6e2fcac08@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®