mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation
@ 2026-03-10 13:52 Mukesh Ojha
  2026-03-10 13:52 ` [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset Mukesh Ojha
  2026-03-11  2:28 ` [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation Dmitry Baryshkov
  0 siblings, 2 replies; 11+ messages in thread
From: Mukesh Ojha @ 2026-03-10 13:52 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Mukesh Ojha
  Cc: linux-arm-msm, linux-remoteproc, linux-kernel

DTB PAS context creation should be done only for subsystems that support
a DTB firmware binary; otherwise, memory is wasted. Move the context
creation to the appropriate location and, while at it, fix the place
where the DTB PAS context was being released unconditionally.

Fixes: b13d8baf5601 ("remoteproc: pas: Replace metadata context with PAS context structure")
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 36 +++++++++++++++---------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 46204da046fa..3bde37ac510c 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -250,7 +250,9 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 	return 0;
 
 release_dtb_metadata:
-	qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
+	if (pas->dtb_pas_id)
+		qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
+
 	release_firmware(pas->dtb_firmware);
 
 	return ret;
@@ -623,6 +625,7 @@ static void qcom_pas_pds_detach(struct qcom_pas *pas, struct device **pds, size_
 
 static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 {
+	struct rproc *rproc = pas->rproc;
 	struct resource res;
 	int ret;
 
@@ -640,6 +643,12 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 		return PTR_ERR(pas->mem_region);
 	}
 
+	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
+						       pas->mem_phys, pas->mem_size);
+	if (IS_ERR(pas->pas_ctx))
+		return PTR_ERR(pas->pas_ctx);
+
+	pas->pas_ctx->use_tzmem = rproc->has_iommu;
 	if (!pas->dtb_pas_id)
 		return 0;
 
@@ -657,6 +666,14 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 		return PTR_ERR(pas->dtb_mem_region);
 	}
 
+	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
+							   pas->dtb_mem_phys,
+							   pas->dtb_mem_size);
+	if (IS_ERR(pas->dtb_pas_ctx))
+		return PTR_ERR(pas->dtb_pas_ctx);
+
+	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
+
 	return 0;
 }
 
@@ -838,23 +855,6 @@ static int qcom_pas_probe(struct platform_device *pdev)
 
 	qcom_add_ssr_subdev(rproc, &pas->ssr_subdev, desc->ssr_name);
 
-	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
-						       pas->mem_phys, pas->mem_size);
-	if (IS_ERR(pas->pas_ctx)) {
-		ret = PTR_ERR(pas->pas_ctx);
-		goto remove_ssr_sysmon;
-	}
-
-	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
-							   pas->dtb_mem_phys,
-							   pas->dtb_mem_size);
-	if (IS_ERR(pas->dtb_pas_ctx)) {
-		ret = PTR_ERR(pas->dtb_pas_ctx);
-		goto remove_ssr_sysmon;
-	}
-
-	pas->pas_ctx->use_tzmem = rproc->has_iommu;
-	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
 	ret = rproc_add(rproc);
 	if (ret)
 		goto remove_ssr_sysmon;
-- 
2.50.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
  2026-03-10 13:52 [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation Mukesh Ojha
@ 2026-03-10 13:52 ` Mukesh Ojha
  2026-03-11  2:34   ` Dmitry Baryshkov
  2026-03-18 23:20   ` kernel test robot
  2026-03-11  2:28 ` [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation Dmitry Baryshkov
  1 sibling, 2 replies; 11+ messages in thread
From: Mukesh Ojha @ 2026-03-10 13:52 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Mukesh Ojha
  Cc: linux-arm-msm, linux-remoteproc, linux-kernel

Qualcomm remoteproc drivers such as qcom_q6v5_mss, which do not use the
Peripheral Authentication Service (PAS), always map the MBA region before
use and unmap it once the usage is complete. This behavior was introduced
to avoid issues seen in the past where speculative accesses from the
application processor to the MBA region after it was assigned to the remote
Q6 led to an XPU violation. The issue was mitigated by unmapping the region
before handing control to the remote Q6.

Currently, most Qualcomm SoCs using the PAS driver run either with a
standalone QHEE or the Gunyah hypervisor. In these environments, the
hypervisor unmaps the Q6 memory from HLOS Stage-2 and remaps it into the
Q6 Stage-2 page table. As a result, speculative accesses from HLOS cannot
reach the region even if it remains mapped in HLOS Stage-1; therefore, XPU
violations cannot occur.

However, when the same SoC runs Linux at EL2, Linux itself must perform the
unmapping to avoid such issues. It is still correct to apply this mapping/
unmapping sequence even for SoCs that run under Gunyah, so this behavior
should not be conditional.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 48 +++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 3bde37ac510c..033d618ccba9 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -138,6 +138,13 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
 		return;
 	}
 
+	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
+	if (!pas->mem_region) {
+		dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
+			&pas->mem_phys, pas->mem_size);
+		return;
+	}
+
 	memcpy_fromio(dest, pas->mem_region + total_offset, size);
 }
 
@@ -240,9 +247,18 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 			return ret;
 		}
 
+		pas->dtb_mem_region = ioremap_wc(pas->dtb_mem_phys, pas->dtb_mem_size);
+		if (!pas->dtb_mem_region) {
+			dev_err(pas->dev, "unable to map dtb memory region: %pa+%zx\n",
+				&pas->dtb_mem_phys, pas->dtb_mem_size);
+			goto release_dtb_metadata;
+		}
+
 		ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
 					pas->dtb_firmware_name, pas->dtb_mem_region,
 					&pas->dtb_mem_reloc);
+		iounmap(pas->dtb_mem_region);
+		pas->dtb_mem_region = NULL;
 		if (ret)
 			goto release_dtb_metadata;
 	}
@@ -320,8 +336,23 @@ static int qcom_pas_start(struct rproc *rproc)
 		}
 	}
 
+	/*
+	 * During subsystem restart, when coredump is enabled, region is mapped but
+	 * not unmapped there, NULL check to reuse the mapping if its already mapped.
+	 */
+	if (!pas->mem_region) {
+		pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
+		if (!pas->mem_region) {
+			dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
+				&pas->mem_phys, pas->mem_size);
+			goto release_pas_metadata;
+		}
+	}
+
 	ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
 				pas->mem_region, &pas->mem_reloc);
+	iounmap(pas->mem_region);
+	pas->mem_region = NULL;
 	if (ret)
 		goto release_pas_metadata;
 
@@ -447,6 +478,13 @@ static void *qcom_pas_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is
 	if (is_iomem)
 		*is_iomem = true;
 
+	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
+	if (!pas->mem_region) {
+		dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
+			&pas->mem_phys, pas->mem_size);
+		return NULL;
+	}
+
 	return pas->mem_region + offset;
 }
 
@@ -637,11 +675,6 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 
 	pas->mem_phys = pas->mem_reloc = res.start;
 	pas->mem_size = resource_size(&res);
-	pas->mem_region = devm_ioremap_resource_wc(pas->dev, &res);
-	if (IS_ERR(pas->mem_region)) {
-		dev_err(pas->dev, "unable to map memory region: %pR\n", &res);
-		return PTR_ERR(pas->mem_region);
-	}
 
 	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
 						       pas->mem_phys, pas->mem_size);
@@ -660,11 +693,6 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 
 	pas->dtb_mem_phys = pas->dtb_mem_reloc = res.start;
 	pas->dtb_mem_size = resource_size(&res);
-	pas->dtb_mem_region = devm_ioremap_resource_wc(pas->dev, &res);
-	if (IS_ERR(pas->dtb_mem_region)) {
-		dev_err(pas->dev, "unable to map dtb memory region: %pR\n", &res);
-		return PTR_ERR(pas->dtb_mem_region);
-	}
 
 	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
 							   pas->dtb_mem_phys,
-- 
2.50.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation
  2026-03-10 13:52 [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation Mukesh Ojha
  2026-03-10 13:52 ` [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset Mukesh Ojha
@ 2026-03-11  2:28 ` Dmitry Baryshkov
  2026-03-11  7:01   ` Mukesh Ojha
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2026-03-11  2:28 UTC (permalink / raw)
  To: Mukesh Ojha
  Cc: Bjorn Andersson, Mathieu Poirier, linux-arm-msm,
	linux-remoteproc, linux-kernel

On Tue, Mar 10, 2026 at 07:22:04PM +0530, Mukesh Ojha wrote:
> DTB PAS context creation should be done only for subsystems that support
> a DTB firmware binary; otherwise, memory is wasted. Move the context
> creation to the appropriate location and, while at it, fix the place
> where the DTB PAS context was being released unconditionally.

I think here you have too much for this description. Judging by the
text, I'd have assumed only the if(pas->dtb_pas_id) around the DTB
context creation.

> 
> Fixes: b13d8baf5601 ("remoteproc: pas: Replace metadata context with PAS context structure")
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 36 +++++++++++++++---------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 46204da046fa..3bde37ac510c 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -250,7 +250,9 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
>  	return 0;
>  
>  release_dtb_metadata:
> -	qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
> +	if (pas->dtb_pas_id)
> +		qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);

Unrelated question, but why is this not handled by devres action?

> +
>  	release_firmware(pas->dtb_firmware);
>  
>  	return ret;
> @@ -623,6 +625,7 @@ static void qcom_pas_pds_detach(struct qcom_pas *pas, struct device **pds, size_
>  
>  static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
>  {
> +	struct rproc *rproc = pas->rproc;
>  	struct resource res;
>  	int ret;
>  
> @@ -640,6 +643,12 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
>  		return PTR_ERR(pas->mem_region);
>  	}
>  
> +	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
> +						       pas->mem_phys, pas->mem_size);
> +	if (IS_ERR(pas->pas_ctx))
> +		return PTR_ERR(pas->pas_ctx);
> +
> +	pas->pas_ctx->use_tzmem = rproc->has_iommu;
>  	if (!pas->dtb_pas_id)
>  		return 0;
>  
> @@ -657,6 +666,14 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
>  		return PTR_ERR(pas->dtb_mem_region);
>  	}
>  
> +	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
> +							   pas->dtb_mem_phys,
> +							   pas->dtb_mem_size);
> +	if (IS_ERR(pas->dtb_pas_ctx))
> +		return PTR_ERR(pas->dtb_pas_ctx);
> +
> +	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
> +
>  	return 0;
>  }
>  
> @@ -838,23 +855,6 @@ static int qcom_pas_probe(struct platform_device *pdev)
>  
>  	qcom_add_ssr_subdev(rproc, &pas->ssr_subdev, desc->ssr_name);
>  
> -	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
> -						       pas->mem_phys, pas->mem_size);
> -	if (IS_ERR(pas->pas_ctx)) {
> -		ret = PTR_ERR(pas->pas_ctx);
> -		goto remove_ssr_sysmon;
> -	}
> -
> -	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
> -							   pas->dtb_mem_phys,
> -							   pas->dtb_mem_size);
> -	if (IS_ERR(pas->dtb_pas_ctx)) {
> -		ret = PTR_ERR(pas->dtb_pas_ctx);
> -		goto remove_ssr_sysmon;
> -	}
> -
> -	pas->pas_ctx->use_tzmem = rproc->has_iommu;
> -	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
>  	ret = rproc_add(rproc);
>  	if (ret)
>  		goto remove_ssr_sysmon;
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
  2026-03-10 13:52 ` [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset Mukesh Ojha
@ 2026-03-11  2:34   ` Dmitry Baryshkov
  2026-03-11  8:49     ` Mukesh Ojha
  2026-03-18 23:20   ` kernel test robot
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2026-03-11  2:34 UTC (permalink / raw)
  To: Mukesh Ojha
  Cc: Bjorn Andersson, Mathieu Poirier, linux-arm-msm,
	linux-remoteproc, linux-kernel

On Tue, Mar 10, 2026 at 07:22:05PM +0530, Mukesh Ojha wrote:
> Qualcomm remoteproc drivers such as qcom_q6v5_mss, which do not use the
> Peripheral Authentication Service (PAS), always map the MBA region before
> use and unmap it once the usage is complete. This behavior was introduced
> to avoid issues seen in the past where speculative accesses from the
> application processor to the MBA region after it was assigned to the remote
> Q6 led to an XPU violation. The issue was mitigated by unmapping the region
> before handing control to the remote Q6.
> 
> Currently, most Qualcomm SoCs using the PAS driver run either with a
> standalone QHEE or the Gunyah hypervisor. In these environments, the
> hypervisor unmaps the Q6 memory from HLOS Stage-2 and remaps it into the
> Q6 Stage-2 page table. As a result, speculative accesses from HLOS cannot
> reach the region even if it remains mapped in HLOS Stage-1; therefore, XPU
> violations cannot occur.
> 
> However, when the same SoC runs Linux at EL2, Linux itself must perform the
> unmapping to avoid such issues. It is still correct to apply this mapping/
> unmapping sequence even for SoCs that run under Gunyah, so this behavior
> should not be conditional.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 48 +++++++++++++++++++++++-------
>  1 file changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 3bde37ac510c..033d618ccba9 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -138,6 +138,13 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
>  		return;
>  	}
>  
> +	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);

Which will be called once per segment. Move this to qcom_pas_minidump()
and call iounmap() afterwards.

> +	if (!pas->mem_region) {
> +		dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
> +			&pas->mem_phys, pas->mem_size);
> +		return;
> +	}
> +
>  	memcpy_fromio(dest, pas->mem_region + total_offset, size);
>  }
>  
> @@ -240,9 +247,18 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
>  			return ret;
>  		}
>  
> +		pas->dtb_mem_region = ioremap_wc(pas->dtb_mem_phys, pas->dtb_mem_size);
> +		if (!pas->dtb_mem_region) {
> +			dev_err(pas->dev, "unable to map dtb memory region: %pa+%zx\n",
> +				&pas->dtb_mem_phys, pas->dtb_mem_size);
> +			goto release_dtb_metadata;
> +		}
> +
>  		ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
>  					pas->dtb_firmware_name, pas->dtb_mem_region,
>  					&pas->dtb_mem_reloc);
> +		iounmap(pas->dtb_mem_region);
> +		pas->dtb_mem_region = NULL;
>  		if (ret)
>  			goto release_dtb_metadata;
>  	}
> @@ -320,8 +336,23 @@ static int qcom_pas_start(struct rproc *rproc)
>  		}
>  	}
>  
> +	/*
> +	 * During subsystem restart, when coredump is enabled, region is mapped but
> +	 * not unmapped there, NULL check to reuse the mapping if its already mapped.

Why? Just unmap it there,

> +	 */
> +	if (!pas->mem_region) {
> +		pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
> +		if (!pas->mem_region) {
> +			dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
> +				&pas->mem_phys, pas->mem_size);
> +			goto release_pas_metadata;
> +		}
> +	}
> +
>  	ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
>  				pas->mem_region, &pas->mem_reloc);

Would it be easier to move ioremap_wc() / iounmap() to
qcom_mdt_pas_load()?

> +	iounmap(pas->mem_region);
> +	pas->mem_region = NULL;
>  	if (ret)
>  		goto release_pas_metadata;
>  
> @@ -447,6 +478,13 @@ static void *qcom_pas_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is
>  	if (is_iomem)
>  		*is_iomem = true;
>  
> +	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
> +	if (!pas->mem_region) {
> +		dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
> +			&pas->mem_phys, pas->mem_size);
> +		return NULL;
> +	}
> +
>  	return pas->mem_region + offset;
>  }
>  
> @@ -637,11 +675,6 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
>  
>  	pas->mem_phys = pas->mem_reloc = res.start;
>  	pas->mem_size = resource_size(&res);
> -	pas->mem_region = devm_ioremap_resource_wc(pas->dev, &res);
> -	if (IS_ERR(pas->mem_region)) {
> -		dev_err(pas->dev, "unable to map memory region: %pR\n", &res);
> -		return PTR_ERR(pas->mem_region);
> -	}
>  
>  	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
>  						       pas->mem_phys, pas->mem_size);
> @@ -660,11 +693,6 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
>  
>  	pas->dtb_mem_phys = pas->dtb_mem_reloc = res.start;
>  	pas->dtb_mem_size = resource_size(&res);
> -	pas->dtb_mem_region = devm_ioremap_resource_wc(pas->dev, &res);
> -	if (IS_ERR(pas->dtb_mem_region)) {
> -		dev_err(pas->dev, "unable to map dtb memory region: %pR\n", &res);
> -		return PTR_ERR(pas->dtb_mem_region);
> -	}
>  
>  	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
>  							   pas->dtb_mem_phys,
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation
  2026-03-11  2:28 ` [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation Dmitry Baryshkov
@ 2026-03-11  7:01   ` Mukesh Ojha
  0 siblings, 0 replies; 11+ messages in thread
From: Mukesh Ojha @ 2026-03-11  7:01 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Mathieu Poirier, linux-arm-msm,
	linux-remoteproc, linux-kernel

On Wed, Mar 11, 2026 at 04:28:12AM +0200, Dmitry Baryshkov wrote:
> On Tue, Mar 10, 2026 at 07:22:04PM +0530, Mukesh Ojha wrote:
> > DTB PAS context creation should be done only for subsystems that support
> > a DTB firmware binary; otherwise, memory is wasted. Move the context
> > creation to the appropriate location and, while at it, fix the place
> > where the DTB PAS context was being released unconditionally.
> 
> I think here you have too much for this description. Judging by the
> text, I'd have assumed only the if(pas->dtb_pas_id) around the DTB
> context creation.

Yes, you are right!

> 
> > 
> > Fixes: b13d8baf5601 ("remoteproc: pas: Replace metadata context with PAS context structure")
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> >  drivers/remoteproc/qcom_q6v5_pas.c | 36 +++++++++++++++---------------
> >  1 file changed, 18 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > index 46204da046fa..3bde37ac510c 100644
> > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > @@ -250,7 +250,9 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
> >  	return 0;
> >  
> >  release_dtb_metadata:
> > -	qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
> > +	if (pas->dtb_pas_id)
> > +		qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
> 
> Unrelated question, but why is this not handled by devres action?

dtb_pas_ctx is still managed by devres but here, we are releasing memory
hold by ctx->ptr.

-Mukesh
> 
> > +
> >  	release_firmware(pas->dtb_firmware);
> >  
> >  	return ret;
> > @@ -623,6 +625,7 @@ static void qcom_pas_pds_detach(struct qcom_pas *pas, struct device **pds, size_
> >  
> >  static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
> >  {
> > +	struct rproc *rproc = pas->rproc;
> >  	struct resource res;
> >  	int ret;
> >  
> > @@ -640,6 +643,12 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
> >  		return PTR_ERR(pas->mem_region);
> >  	}
> >  
> > +	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
> > +						       pas->mem_phys, pas->mem_size);
> > +	if (IS_ERR(pas->pas_ctx))
> > +		return PTR_ERR(pas->pas_ctx);
> > +
> > +	pas->pas_ctx->use_tzmem = rproc->has_iommu;
> >  	if (!pas->dtb_pas_id)
> >  		return 0;
> >  
> > @@ -657,6 +666,14 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
> >  		return PTR_ERR(pas->dtb_mem_region);
> >  	}
> >  
> > +	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
> > +							   pas->dtb_mem_phys,
> > +							   pas->dtb_mem_size);
> > +	if (IS_ERR(pas->dtb_pas_ctx))
> > +		return PTR_ERR(pas->dtb_pas_ctx);
> > +
> > +	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
> > +
> >  	return 0;
> >  }
> >  
> > @@ -838,23 +855,6 @@ static int qcom_pas_probe(struct platform_device *pdev)
> >  
> >  	qcom_add_ssr_subdev(rproc, &pas->ssr_subdev, desc->ssr_name);
> >  
> > -	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
> > -						       pas->mem_phys, pas->mem_size);
> > -	if (IS_ERR(pas->pas_ctx)) {
> > -		ret = PTR_ERR(pas->pas_ctx);
> > -		goto remove_ssr_sysmon;
> > -	}
> > -
> > -	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
> > -							   pas->dtb_mem_phys,
> > -							   pas->dtb_mem_size);
> > -	if (IS_ERR(pas->dtb_pas_ctx)) {
> > -		ret = PTR_ERR(pas->dtb_pas_ctx);
> > -		goto remove_ssr_sysmon;
> > -	}
> > -
> > -	pas->pas_ctx->use_tzmem = rproc->has_iommu;
> > -	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
> >  	ret = rproc_add(rproc);
> >  	if (ret)
> >  		goto remove_ssr_sysmon;
> > -- 
> > 2.50.1
> > 
> 
> -- 
> With best wishes
> Dmitry

-- 
-Mukesh Ojha

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
  2026-03-11  2:34   ` Dmitry Baryshkov
@ 2026-03-11  8:49     ` Mukesh Ojha
  2026-03-13  3:40       ` Dmitry Baryshkov
  0 siblings, 1 reply; 11+ messages in thread
From: Mukesh Ojha @ 2026-03-11  8:49 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Mathieu Poirier, linux-arm-msm,
	linux-remoteproc, linux-kernel

On Wed, Mar 11, 2026 at 04:34:53AM +0200, Dmitry Baryshkov wrote:
> On Tue, Mar 10, 2026 at 07:22:05PM +0530, Mukesh Ojha wrote:
> > Qualcomm remoteproc drivers such as qcom_q6v5_mss, which do not use the
> > Peripheral Authentication Service (PAS), always map the MBA region before
> > use and unmap it once the usage is complete. This behavior was introduced
> > to avoid issues seen in the past where speculative accesses from the
> > application processor to the MBA region after it was assigned to the remote
> > Q6 led to an XPU violation. The issue was mitigated by unmapping the region
> > before handing control to the remote Q6.
> > 
> > Currently, most Qualcomm SoCs using the PAS driver run either with a
> > standalone QHEE or the Gunyah hypervisor. In these environments, the
> > hypervisor unmaps the Q6 memory from HLOS Stage-2 and remaps it into the
> > Q6 Stage-2 page table. As a result, speculative accesses from HLOS cannot
> > reach the region even if it remains mapped in HLOS Stage-1; therefore, XPU
> > violations cannot occur.
> > 
> > However, when the same SoC runs Linux at EL2, Linux itself must perform the
> > unmapping to avoid such issues. It is still correct to apply this mapping/
> > unmapping sequence even for SoCs that run under Gunyah, so this behavior
> > should not be conditional.
> > 
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> >  drivers/remoteproc/qcom_q6v5_pas.c | 48 +++++++++++++++++++++++-------
> >  1 file changed, 38 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > index 3bde37ac510c..033d618ccba9 100644
> > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > @@ -138,6 +138,13 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
> >  		return;
> >  	}
> >  
> > +	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
> 
> Which will be called once per segment. Move this to qcom_pas_minidump()
> and call iounmap() afterwards.

Good eyes!, Ah!! my bad.

And moving it to qcom_pas_minidump() will cover minidump cases and
fallback cases to full dumps if minidump_id is mentioned for a
subsystem. However, we may miss mapping for pure rproc_coredump() case
when the framework assigned .coredump = rproc_coredump when vendor
coredump callback is not provided when subsystem does not have minidump_id
mentioned. Do you think writing qcom_pas_coredump() wrapper over
rproc_coredump() will be fine ?

--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -141,6 +141,11 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
        memcpy_fromio(dest, pas->mem_region + total_offset, size);
 }

+static void qcom_pas_coredump(struct rproc *rproc)
+{
+       rproc_coredump(rproc);
+}
+
 static void qcom_pas_minidump(struct rproc *rproc)
 {
        struct qcom_pas *pas = rproc->priv;
@@ -518,6 +523,7 @@ static const struct rproc_ops qcom_pas_ops = {
        .parse_fw = qcom_pas_parse_firmware,
        .load = qcom_pas_load,
        .panic = qcom_pas_panic,
+       .coredump = qcom_pas_coredump,
 };


> 
> > +	if (!pas->mem_region) {
> > +		dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
> > +			&pas->mem_phys, pas->mem_size);
> > +		return;
> > +	}
> > +
> >  	memcpy_fromio(dest, pas->mem_region + total_offset, size);
> >  }
> >  
> > @@ -240,9 +247,18 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
> >  			return ret;
> >  		}
> >  
> > +		pas->dtb_mem_region = ioremap_wc(pas->dtb_mem_phys, pas->dtb_mem_size);
> > +		if (!pas->dtb_mem_region) {
> > +			dev_err(pas->dev, "unable to map dtb memory region: %pa+%zx\n",
> > +				&pas->dtb_mem_phys, pas->dtb_mem_size);
> > +			goto release_dtb_metadata;
> > +		}
> > +
> >  		ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
> >  					pas->dtb_firmware_name, pas->dtb_mem_region,
> >  					&pas->dtb_mem_reloc);
> > +		iounmap(pas->dtb_mem_region);
> > +		pas->dtb_mem_region = NULL;
> >  		if (ret)
> >  			goto release_dtb_metadata;
> >  	}
> > @@ -320,8 +336,23 @@ static int qcom_pas_start(struct rproc *rproc)
> >  		}
> >  	}
> >  
> > +	/*
> > +	 * During subsystem restart, when coredump is enabled, region is mapped but
> > +	 * not unmapped there, NULL check to reuse the mapping if its already mapped.
> 
> Why? Just unmap it there,
> 
> > +	 */
> > +	if (!pas->mem_region) {
> > +		pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
> > +		if (!pas->mem_region) {
> > +			dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
> > +				&pas->mem_phys, pas->mem_size);
> > +			goto release_pas_metadata;
> > +		}
> > +	}
> > +
> >  	ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
> >  				pas->mem_region, &pas->mem_reloc);
> 
> Would it be easier to move ioremap_wc() / iounmap() to
> qcom_mdt_pas_load()?

This would be fine as well, since, qcom_mdt_pas_load() is a new API and
only pas driver is the only user., so, no one is impacted at the moment.

-Mukesh
> 
> > +	iounmap(pas->mem_region);
> > +	pas->mem_region = NULL;
> >  	if (ret)
> >  		goto release_pas_metadata;
> >  
> > @@ -447,6 +478,13 @@ static void *qcom_pas_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is
> >  	if (is_iomem)
> >  		*is_iomem = true;
> >  
> > +	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
> > +	if (!pas->mem_region) {
> > +		dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
> > +			&pas->mem_phys, pas->mem_size);
> > +		return NULL;
> > +	}
> > +
> >  	return pas->mem_region + offset;
> >  }
> >  
> > @@ -637,11 +675,6 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
> >  
> >  	pas->mem_phys = pas->mem_reloc = res.start;
> >  	pas->mem_size = resource_size(&res);
> > -	pas->mem_region = devm_ioremap_resource_wc(pas->dev, &res);
> > -	if (IS_ERR(pas->mem_region)) {
> > -		dev_err(pas->dev, "unable to map memory region: %pR\n", &res);
> > -		return PTR_ERR(pas->mem_region);
> > -	}
> >  
> >  	pas->pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->pas_id,
> >  						       pas->mem_phys, pas->mem_size);
> > @@ -660,11 +693,6 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
> >  
> >  	pas->dtb_mem_phys = pas->dtb_mem_reloc = res.start;
> >  	pas->dtb_mem_size = resource_size(&res);
> > -	pas->dtb_mem_region = devm_ioremap_resource_wc(pas->dev, &res);
> > -	if (IS_ERR(pas->dtb_mem_region)) {
> > -		dev_err(pas->dev, "unable to map dtb memory region: %pR\n", &res);
> > -		return PTR_ERR(pas->dtb_mem_region);
> > -	}
> >  
> >  	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_alloc(pas->dev, pas->dtb_pas_id,
> >  							   pas->dtb_mem_phys,
> > -- 
> > 2.50.1
> > 
> 
> -- 
> With best wishes
> Dmitry

-- 
-Mukesh Ojha

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
  2026-03-11  8:49     ` Mukesh Ojha
@ 2026-03-13  3:40       ` Dmitry Baryshkov
  2026-03-13  5:57         ` Mukesh Ojha
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2026-03-13  3:40 UTC (permalink / raw)
  To: Mukesh Ojha
  Cc: Bjorn Andersson, Mathieu Poirier, linux-arm-msm,
	linux-remoteproc, linux-kernel

On Wed, Mar 11, 2026 at 02:19:18PM +0530, Mukesh Ojha wrote:
> On Wed, Mar 11, 2026 at 04:34:53AM +0200, Dmitry Baryshkov wrote:
> > On Tue, Mar 10, 2026 at 07:22:05PM +0530, Mukesh Ojha wrote:
> > > Qualcomm remoteproc drivers such as qcom_q6v5_mss, which do not use the
> > > Peripheral Authentication Service (PAS), always map the MBA region before
> > > use and unmap it once the usage is complete. This behavior was introduced
> > > to avoid issues seen in the past where speculative accesses from the
> > > application processor to the MBA region after it was assigned to the remote
> > > Q6 led to an XPU violation. The issue was mitigated by unmapping the region
> > > before handing control to the remote Q6.
> > > 
> > > Currently, most Qualcomm SoCs using the PAS driver run either with a
> > > standalone QHEE or the Gunyah hypervisor. In these environments, the
> > > hypervisor unmaps the Q6 memory from HLOS Stage-2 and remaps it into the
> > > Q6 Stage-2 page table. As a result, speculative accesses from HLOS cannot
> > > reach the region even if it remains mapped in HLOS Stage-1; therefore, XPU
> > > violations cannot occur.
> > > 
> > > However, when the same SoC runs Linux at EL2, Linux itself must perform the
> > > unmapping to avoid such issues. It is still correct to apply this mapping/
> > > unmapping sequence even for SoCs that run under Gunyah, so this behavior
> > > should not be conditional.
> > > 
> > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > > ---
> > >  drivers/remoteproc/qcom_q6v5_pas.c | 48 +++++++++++++++++++++++-------
> > >  1 file changed, 38 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > > index 3bde37ac510c..033d618ccba9 100644
> > > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > > @@ -138,6 +138,13 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
> > >  		return;
> > >  	}
> > >  
> > > +	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
> > 
> > Which will be called once per segment. Move this to qcom_pas_minidump()
> > and call iounmap() afterwards.
> 
> Good eyes!, Ah!! my bad.
> 
> And moving it to qcom_pas_minidump() will cover minidump cases and
> fallback cases to full dumps if minidump_id is mentioned for a
> subsystem. However, we may miss mapping for pure rproc_coredump() case
> when the framework assigned .coredump = rproc_coredump when vendor
> coredump callback is not provided when subsystem does not have minidump_id
> mentioned. Do you think writing qcom_pas_coredump() wrapper over
> rproc_coredump() will be fine ?
> 
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -141,6 +141,11 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
>         memcpy_fromio(dest, pas->mem_region + total_offset, size);
>  }
> 
> +static void qcom_pas_coredump(struct rproc *rproc)
> +{
> +       rproc_coredump(rproc);

It won't be that simple, most likely. You'd need to call ioremap_wc()
before and iounmap() afterwards. Then it would make sense.

And alternative option would be to use
rproc_coredump_add_custom_segment() and add a custom function which
would perform ioremap() / memcpy() / iounmap().

> +}
> +
>  static void qcom_pas_minidump(struct rproc *rproc)
>  {
>         struct qcom_pas *pas = rproc->priv;

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
  2026-03-13  3:40       ` Dmitry Baryshkov
@ 2026-03-13  5:57         ` Mukesh Ojha
  2026-03-13 17:20           ` Dmitry Baryshkov
  0 siblings, 1 reply; 11+ messages in thread
From: Mukesh Ojha @ 2026-03-13  5:57 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Mathieu Poirier, linux-arm-msm,
	linux-remoteproc, linux-kernel

On Fri, Mar 13, 2026 at 05:40:22AM +0200, Dmitry Baryshkov wrote:
> On Wed, Mar 11, 2026 at 02:19:18PM +0530, Mukesh Ojha wrote:
> > On Wed, Mar 11, 2026 at 04:34:53AM +0200, Dmitry Baryshkov wrote:
> > > On Tue, Mar 10, 2026 at 07:22:05PM +0530, Mukesh Ojha wrote:
> > > > Qualcomm remoteproc drivers such as qcom_q6v5_mss, which do not use the
> > > > Peripheral Authentication Service (PAS), always map the MBA region before
> > > > use and unmap it once the usage is complete. This behavior was introduced
> > > > to avoid issues seen in the past where speculative accesses from the
> > > > application processor to the MBA region after it was assigned to the remote
> > > > Q6 led to an XPU violation. The issue was mitigated by unmapping the region
> > > > before handing control to the remote Q6.
> > > > 
> > > > Currently, most Qualcomm SoCs using the PAS driver run either with a
> > > > standalone QHEE or the Gunyah hypervisor. In these environments, the
> > > > hypervisor unmaps the Q6 memory from HLOS Stage-2 and remaps it into the
> > > > Q6 Stage-2 page table. As a result, speculative accesses from HLOS cannot
> > > > reach the region even if it remains mapped in HLOS Stage-1; therefore, XPU
> > > > violations cannot occur.
> > > > 
> > > > However, when the same SoC runs Linux at EL2, Linux itself must perform the
> > > > unmapping to avoid such issues. It is still correct to apply this mapping/
> > > > unmapping sequence even for SoCs that run under Gunyah, so this behavior
> > > > should not be conditional.
> > > > 
> > > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > > > ---
> > > >  drivers/remoteproc/qcom_q6v5_pas.c | 48 +++++++++++++++++++++++-------
> > > >  1 file changed, 38 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > > > index 3bde37ac510c..033d618ccba9 100644
> > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > > > @@ -138,6 +138,13 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
> > > >  		return;
> > > >  	}
> > > >  
> > > > +	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
> > > 
> > > Which will be called once per segment. Move this to qcom_pas_minidump()
> > > and call iounmap() afterwards.
> > 
> > Good eyes!, Ah!! my bad.
> > 
> > And moving it to qcom_pas_minidump() will cover minidump cases and
> > fallback cases to full dumps if minidump_id is mentioned for a
> > subsystem. However, we may miss mapping for pure rproc_coredump() case
> > when the framework assigned .coredump = rproc_coredump when vendor
> > coredump callback is not provided when subsystem does not have minidump_id
> > mentioned. Do you think writing qcom_pas_coredump() wrapper over
> > rproc_coredump() will be fine ?
> > 
> > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > @@ -141,6 +141,11 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
> >         memcpy_fromio(dest, pas->mem_region + total_offset, size);
> >  }
> > 
> > +static void qcom_pas_coredump(struct rproc *rproc)
> > +{
> > +       rproc_coredump(rproc);
> 
> It won't be that simple, most likely. You'd need to call ioremap_wc()
> before and iounmap() afterwards. Then it would make sense.

I meant the same with the help of wrapper..

> 
> And alternative option would be to use
> rproc_coredump_add_custom_segment() and add a custom function which
> would perform ioremap() / memcpy() / iounmap().

This will involve map, unmap called for each segment.
However, I feel, it will be inefficient compared to qcom_pas_coredump()
where, we can map only once.

Thanks,
Mukesh

> 
> > +}
> > +
> >  static void qcom_pas_minidump(struct rproc *rproc)
> >  {
> >         struct qcom_pas *pas = rproc->priv;
> 
> -- 
> With best wishes
> Dmitry

-- 
-Mukesh Ojha

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
  2026-03-13  5:57         ` Mukesh Ojha
@ 2026-03-13 17:20           ` Dmitry Baryshkov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Baryshkov @ 2026-03-13 17:20 UTC (permalink / raw)
  To: Mukesh Ojha
  Cc: Bjorn Andersson, Mathieu Poirier, linux-arm-msm,
	linux-remoteproc, linux-kernel

On Fri, Mar 13, 2026 at 11:27:18AM +0530, Mukesh Ojha wrote:
> On Fri, Mar 13, 2026 at 05:40:22AM +0200, Dmitry Baryshkov wrote:
> > On Wed, Mar 11, 2026 at 02:19:18PM +0530, Mukesh Ojha wrote:
> > > On Wed, Mar 11, 2026 at 04:34:53AM +0200, Dmitry Baryshkov wrote:
> > > > On Tue, Mar 10, 2026 at 07:22:05PM +0530, Mukesh Ojha wrote:
> > > > > Qualcomm remoteproc drivers such as qcom_q6v5_mss, which do not use the
> > > > > Peripheral Authentication Service (PAS), always map the MBA region before
> > > > > use and unmap it once the usage is complete. This behavior was introduced
> > > > > to avoid issues seen in the past where speculative accesses from the
> > > > > application processor to the MBA region after it was assigned to the remote
> > > > > Q6 led to an XPU violation. The issue was mitigated by unmapping the region
> > > > > before handing control to the remote Q6.
> > > > > 
> > > > > Currently, most Qualcomm SoCs using the PAS driver run either with a
> > > > > standalone QHEE or the Gunyah hypervisor. In these environments, the
> > > > > hypervisor unmaps the Q6 memory from HLOS Stage-2 and remaps it into the
> > > > > Q6 Stage-2 page table. As a result, speculative accesses from HLOS cannot
> > > > > reach the region even if it remains mapped in HLOS Stage-1; therefore, XPU
> > > > > violations cannot occur.
> > > > > 
> > > > > However, when the same SoC runs Linux at EL2, Linux itself must perform the
> > > > > unmapping to avoid such issues. It is still correct to apply this mapping/
> > > > > unmapping sequence even for SoCs that run under Gunyah, so this behavior
> > > > > should not be conditional.
> > > > > 
> > > > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > > > > ---
> > > > >  drivers/remoteproc/qcom_q6v5_pas.c | 48 +++++++++++++++++++++++-------
> > > > >  1 file changed, 38 insertions(+), 10 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > > > > index 3bde37ac510c..033d618ccba9 100644
> > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > > > > @@ -138,6 +138,13 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
> > > > >  		return;
> > > > >  	}
> > > > >  
> > > > > +	pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
> > > > 
> > > > Which will be called once per segment. Move this to qcom_pas_minidump()
> > > > and call iounmap() afterwards.
> > > 
> > > Good eyes!, Ah!! my bad.
> > > 
> > > And moving it to qcom_pas_minidump() will cover minidump cases and
> > > fallback cases to full dumps if minidump_id is mentioned for a
> > > subsystem. However, we may miss mapping for pure rproc_coredump() case
> > > when the framework assigned .coredump = rproc_coredump when vendor
> > > coredump callback is not provided when subsystem does not have minidump_id
> > > mentioned. Do you think writing qcom_pas_coredump() wrapper over
> > > rproc_coredump() will be fine ?
> > > 
> > > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > > @@ -141,6 +141,11 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
> > >         memcpy_fromio(dest, pas->mem_region + total_offset, size);
> > >  }
> > > 
> > > +static void qcom_pas_coredump(struct rproc *rproc)
> > > +{
> > > +       rproc_coredump(rproc);
> > 
> > It won't be that simple, most likely. You'd need to call ioremap_wc()
> > before and iounmap() afterwards. Then it would make sense.
> 
> I meant the same with the help of wrapper..
> 
> > 
> > And alternative option would be to use
> > rproc_coredump_add_custom_segment() and add a custom function which
> > would perform ioremap() / memcpy() / iounmap().
> 
> This will involve map, unmap called for each segment.
> However, I feel, it will be inefficient compared to qcom_pas_coredump()
> where, we can map only once.

So, let's have the wrapper.

> 
> Thanks,
> Mukesh
> 
> > 
> > > +}
> > > +
> > >  static void qcom_pas_minidump(struct rproc *rproc)
> > >  {
> > >         struct qcom_pas *pas = rproc->priv;
> > 
> > -- 
> > With best wishes
> > Dmitry
> 
> -- 
> -Mukesh Ojha

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
  2026-03-10 13:52 ` [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset Mukesh Ojha
  2026-03-11  2:34   ` Dmitry Baryshkov
@ 2026-03-18 23:20   ` kernel test robot
  2026-03-24 12:55     ` Mukesh Ojha
  1 sibling, 1 reply; 11+ messages in thread
From: kernel test robot @ 2026-03-18 23:20 UTC (permalink / raw)
  To: Mukesh Ojha, Bjorn Andersson, Mathieu Poirier
  Cc: oe-kbuild-all, linux-arm-msm, linux-remoteproc, linux-kernel

Hi Mukesh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on remoteproc/rproc-next]
[also build test WARNING on linus/master v7.0-rc4 next-20260318]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mukesh-Ojha/remoteproc-qcom-pas-Map-unmap-subsystem-region-before-auth_and_reset/20260310-225821
base:   https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git rproc-next
patch link:    https://lore.kernel.org/r/20260310135205.2497789-2-mukesh.ojha%40oss.qualcomm.com
patch subject: [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
config: arm64-randconfig-r133-20260318 (https://download.01.org/0day-ci/archive/20260319/202603190738.1VFMgKNm-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260319/202603190738.1VFMgKNm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603190738.1VFMgKNm-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/remoteproc/qcom_q6v5_pas.c:141:25: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *mem_region @@     got void [noderef] __iomem * @@
   drivers/remoteproc/qcom_q6v5_pas.c:141:25: sparse:     expected void *mem_region
   drivers/remoteproc/qcom_q6v5_pas.c:141:25: sparse:     got void [noderef] __iomem *
   drivers/remoteproc/qcom_q6v5_pas.c:148:45: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const volatile [noderef] __iomem *src @@     got void * @@
   drivers/remoteproc/qcom_q6v5_pas.c:148:45: sparse:     expected void const volatile [noderef] __iomem *src
   drivers/remoteproc/qcom_q6v5_pas.c:148:45: sparse:     got void *
   drivers/remoteproc/qcom_q6v5_pas.c:250:37: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *dtb_mem_region @@     got void [noderef] __iomem * @@
   drivers/remoteproc/qcom_q6v5_pas.c:250:37: sparse:     expected void *dtb_mem_region
   drivers/remoteproc/qcom_q6v5_pas.c:250:37: sparse:     got void [noderef] __iomem *
>> drivers/remoteproc/qcom_q6v5_pas.c:260:28: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem *addr @@     got void *dtb_mem_region @@
   drivers/remoteproc/qcom_q6v5_pas.c:260:28: sparse:     expected void volatile [noderef] __iomem *addr
   drivers/remoteproc/qcom_q6v5_pas.c:260:28: sparse:     got void *dtb_mem_region
   drivers/remoteproc/qcom_q6v5_pas.c:344:33: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *mem_region @@     got void [noderef] __iomem * @@
   drivers/remoteproc/qcom_q6v5_pas.c:344:33: sparse:     expected void *mem_region
   drivers/remoteproc/qcom_q6v5_pas.c:344:33: sparse:     got void [noderef] __iomem *
>> drivers/remoteproc/qcom_q6v5_pas.c:354:20: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem *addr @@     got void *mem_region @@
   drivers/remoteproc/qcom_q6v5_pas.c:354:20: sparse:     expected void volatile [noderef] __iomem *addr
   drivers/remoteproc/qcom_q6v5_pas.c:354:20: sparse:     got void *mem_region
   drivers/remoteproc/qcom_q6v5_pas.c:481:25: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *mem_region @@     got void [noderef] __iomem * @@
   drivers/remoteproc/qcom_q6v5_pas.c:481:25: sparse:     expected void *mem_region
   drivers/remoteproc/qcom_q6v5_pas.c:481:25: sparse:     got void [noderef] __iomem *

vim +260 drivers/remoteproc/qcom_q6v5_pas.c

   124	
   125	static void qcom_pas_segment_dump(struct rproc *rproc,
   126					  struct rproc_dump_segment *segment,
   127					  void *dest, size_t offset, size_t size)
   128	{
   129		struct qcom_pas *pas = rproc->priv;
   130		int total_offset;
   131	
   132		total_offset = segment->da + segment->offset + offset - pas->mem_phys;
   133		if (total_offset < 0 || total_offset + size > pas->mem_size) {
   134			dev_err(pas->dev,
   135				"invalid copy request for segment %pad with offset %zu and size %zu)\n",
   136				&segment->da, offset, size);
   137			memset(dest, 0xff, size);
   138			return;
   139		}
   140	
 > 141		pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
   142		if (!pas->mem_region) {
   143			dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
   144				&pas->mem_phys, pas->mem_size);
   145			return;
   146		}
   147	
   148		memcpy_fromio(dest, pas->mem_region + total_offset, size);
   149	}
   150	
   151	static void qcom_pas_minidump(struct rproc *rproc)
   152	{
   153		struct qcom_pas *pas = rproc->priv;
   154	
   155		if (rproc->dump_conf == RPROC_COREDUMP_DISABLED)
   156			return;
   157	
   158		qcom_minidump(rproc, pas->minidump_id, qcom_pas_segment_dump);
   159	}
   160	
   161	static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
   162				       size_t pd_count)
   163	{
   164		int ret;
   165		int i;
   166	
   167		for (i = 0; i < pd_count; i++) {
   168			dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
   169			ret = pm_runtime_get_sync(pds[i]);
   170			if (ret < 0) {
   171				pm_runtime_put_noidle(pds[i]);
   172				dev_pm_genpd_set_performance_state(pds[i], 0);
   173				goto unroll_pd_votes;
   174			}
   175		}
   176	
   177		return 0;
   178	
   179	unroll_pd_votes:
   180		for (i--; i >= 0; i--) {
   181			dev_pm_genpd_set_performance_state(pds[i], 0);
   182			pm_runtime_put(pds[i]);
   183		}
   184	
   185		return ret;
   186	};
   187	
   188	static void qcom_pas_pds_disable(struct qcom_pas *pas, struct device **pds,
   189					 size_t pd_count)
   190	{
   191		int i;
   192	
   193		for (i = 0; i < pd_count; i++) {
   194			dev_pm_genpd_set_performance_state(pds[i], 0);
   195			pm_runtime_put(pds[i]);
   196		}
   197	}
   198	
   199	static int qcom_pas_shutdown_poll_decrypt(struct qcom_pas *pas)
   200	{
   201		unsigned int retry_num = 50;
   202		int ret;
   203	
   204		do {
   205			msleep(QCOM_PAS_DECRYPT_SHUTDOWN_DELAY_MS);
   206			ret = qcom_scm_pas_shutdown(pas->pas_id);
   207		} while (ret == -EINVAL && --retry_num);
   208	
   209		return ret;
   210	}
   211	
   212	static int qcom_pas_unprepare(struct rproc *rproc)
   213	{
   214		struct qcom_pas *pas = rproc->priv;
   215	
   216		/*
   217		 * qcom_pas_load() did pass pas_metadata to the SCM driver for storing
   218		 * metadata context. It might have been released already if
   219		 * auth_and_reset() was successful, but in other cases clean it up
   220		 * here.
   221		 */
   222		qcom_scm_pas_metadata_release(pas->pas_ctx);
   223		if (pas->dtb_pas_id)
   224			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
   225	
   226		return 0;
   227	}
   228	
   229	static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
   230	{
   231		struct qcom_pas *pas = rproc->priv;
   232		int ret;
   233	
   234		/* Store firmware handle to be used in qcom_pas_start() */
   235		pas->firmware = fw;
   236	
   237		if (pas->lite_pas_id)
   238			qcom_scm_pas_shutdown(pas->lite_pas_id);
   239		if (pas->lite_dtb_pas_id)
   240			qcom_scm_pas_shutdown(pas->lite_dtb_pas_id);
   241	
   242		if (pas->dtb_pas_id) {
   243			ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name, pas->dev);
   244			if (ret) {
   245				dev_err(pas->dev, "request_firmware failed for %s: %d\n",
   246					pas->dtb_firmware_name, ret);
   247				return ret;
   248			}
   249	
 > 250			pas->dtb_mem_region = ioremap_wc(pas->dtb_mem_phys, pas->dtb_mem_size);
   251			if (!pas->dtb_mem_region) {
   252				dev_err(pas->dev, "unable to map dtb memory region: %pa+%zx\n",
   253					&pas->dtb_mem_phys, pas->dtb_mem_size);
   254				goto release_dtb_metadata;
   255			}
   256	
   257			ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
   258						pas->dtb_firmware_name, pas->dtb_mem_region,
   259						&pas->dtb_mem_reloc);
 > 260			iounmap(pas->dtb_mem_region);
   261			pas->dtb_mem_region = NULL;
   262			if (ret)
   263				goto release_dtb_metadata;
   264		}
   265	
   266		return 0;
   267	
   268	release_dtb_metadata:
   269		if (pas->dtb_pas_id)
   270			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
   271	
   272		release_firmware(pas->dtb_firmware);
   273	
   274		return ret;
   275	}
   276	
   277	static void qcom_pas_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t size)
   278	{
   279		if (rproc->has_iommu)
   280			iommu_unmap(rproc->domain, mem_phys, size);
   281	}
   282	
   283	static int qcom_pas_map_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t size)
   284	{
   285		int ret = 0;
   286	
   287		if (rproc->has_iommu)
   288			ret = iommu_map(rproc->domain, mem_phys, mem_phys, size,
   289					IOMMU_READ | IOMMU_WRITE, GFP_KERNEL);
   290		return ret;
   291	}
   292	
   293	static int qcom_pas_start(struct rproc *rproc)
   294	{
   295		struct qcom_pas *pas = rproc->priv;
   296		int ret;
   297	
   298		ret = qcom_q6v5_prepare(&pas->q6v5);
   299		if (ret)
   300			return ret;
   301	
   302		ret = qcom_pas_pds_enable(pas, pas->proxy_pds, pas->proxy_pd_count);
   303		if (ret < 0)
   304			goto disable_irqs;
   305	
   306		ret = clk_prepare_enable(pas->xo);
   307		if (ret)
   308			goto disable_proxy_pds;
   309	
   310		ret = clk_prepare_enable(pas->aggre2_clk);
   311		if (ret)
   312			goto disable_xo_clk;
   313	
   314		if (pas->cx_supply) {
   315			ret = regulator_enable(pas->cx_supply);
   316			if (ret)
   317				goto disable_aggre2_clk;
   318		}
   319	
   320		if (pas->px_supply) {
   321			ret = regulator_enable(pas->px_supply);
   322			if (ret)
   323				goto disable_cx_supply;
   324		}
   325	
   326		if (pas->dtb_pas_id) {
   327			ret = qcom_pas_map_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
   328			if (ret)
   329				goto disable_px_supply;
   330	
   331			ret = qcom_scm_pas_prepare_and_auth_reset(pas->dtb_pas_ctx);
   332			if (ret) {
   333				dev_err(pas->dev,
   334					"failed to authenticate dtb image and release reset\n");
   335				goto unmap_dtb_carveout;
   336			}
   337		}
   338	
   339		/*
   340		 * During subsystem restart, when coredump is enabled, region is mapped but
   341		 * not unmapped there, NULL check to reuse the mapping if its already mapped.
   342		 */
   343		if (!pas->mem_region) {
   344			pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
   345			if (!pas->mem_region) {
   346				dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
   347					&pas->mem_phys, pas->mem_size);
   348				goto release_pas_metadata;
   349			}
   350		}
   351	
   352		ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
   353					pas->mem_region, &pas->mem_reloc);
 > 354		iounmap(pas->mem_region);
   355		pas->mem_region = NULL;
   356		if (ret)
   357			goto release_pas_metadata;
   358	
   359		qcom_pil_info_store(pas->info_name, pas->mem_phys, pas->mem_size);
   360	
   361		ret = qcom_pas_map_carveout(rproc, pas->mem_phys, pas->mem_size);
   362		if (ret)
   363			goto release_pas_metadata;
   364	
   365		ret = qcom_scm_pas_prepare_and_auth_reset(pas->pas_ctx);
   366		if (ret) {
   367			dev_err(pas->dev,
   368				"failed to authenticate image and release reset\n");
   369			goto unmap_carveout;
   370		}
   371	
   372		ret = qcom_q6v5_wait_for_start(&pas->q6v5, msecs_to_jiffies(5000));
   373		if (ret == -ETIMEDOUT) {
   374			dev_err(pas->dev, "start timed out\n");
   375			qcom_scm_pas_shutdown(pas->pas_id);
   376			goto unmap_carveout;
   377		}
   378	
   379		qcom_scm_pas_metadata_release(pas->pas_ctx);
   380		if (pas->dtb_pas_id)
   381			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
   382	
   383		/* firmware is used to pass reference from qcom_pas_start(), drop it now */
   384		pas->firmware = NULL;
   385	
   386		return 0;
   387	
   388	unmap_carveout:
   389		qcom_pas_unmap_carveout(rproc, pas->mem_phys, pas->mem_size);
   390	release_pas_metadata:
   391		qcom_scm_pas_metadata_release(pas->pas_ctx);
   392		if (pas->dtb_pas_id)
   393			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
   394	
   395	unmap_dtb_carveout:
   396		if (pas->dtb_pas_id)
   397			qcom_pas_unmap_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
   398	disable_px_supply:
   399		if (pas->px_supply)
   400			regulator_disable(pas->px_supply);
   401	disable_cx_supply:
   402		if (pas->cx_supply)
   403			regulator_disable(pas->cx_supply);
   404	disable_aggre2_clk:
   405		clk_disable_unprepare(pas->aggre2_clk);
   406	disable_xo_clk:
   407		clk_disable_unprepare(pas->xo);
   408	disable_proxy_pds:
   409		qcom_pas_pds_disable(pas, pas->proxy_pds, pas->proxy_pd_count);
   410	disable_irqs:
   411		qcom_q6v5_unprepare(&pas->q6v5);
   412	
   413		/* firmware is used to pass reference from qcom_pas_start(), drop it now */
   414		pas->firmware = NULL;
   415	
   416		return ret;
   417	}
   418	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset
  2026-03-18 23:20   ` kernel test robot
@ 2026-03-24 12:55     ` Mukesh Ojha
  0 siblings, 0 replies; 11+ messages in thread
From: Mukesh Ojha @ 2026-03-24 12:55 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Mathieu Poirier, oe-kbuild-all, linux-arm-msm, linux-remoteproc,
	linux-kernel

On Thu, Mar 19, 2026 at 07:20:26AM +0800, kernel test robot wrote:
[...]

Bjorn,

Some sparse error are even seen without my change, should we fix them ?

-Mukesh
> 
> sparse warnings: (new ones prefixed by >>)
>    drivers/remoteproc/qcom_q6v5_pas.c:141:25: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *mem_region @@     got void [noderef] __iomem * @@
>    drivers/remoteproc/qcom_q6v5_pas.c:141:25: sparse:     expected void *mem_region
>    drivers/remoteproc/qcom_q6v5_pas.c:141:25: sparse:     got void [noderef] __iomem *
>    drivers/remoteproc/qcom_q6v5_pas.c:148:45: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const volatile [noderef] __iomem *src @@     got void * @@
>    drivers/remoteproc/qcom_q6v5_pas.c:148:45: sparse:     expected void const volatile [noderef] __iomem *src
>    drivers/remoteproc/qcom_q6v5_pas.c:148:45: sparse:     got void *
>    drivers/remoteproc/qcom_q6v5_pas.c:250:37: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *dtb_mem_region @@     got void [noderef] __iomem * @@
>    drivers/remoteproc/qcom_q6v5_pas.c:250:37: sparse:     expected void *dtb_mem_region
>    drivers/remoteproc/qcom_q6v5_pas.c:250:37: sparse:     got void [noderef] __iomem *
> >> drivers/remoteproc/qcom_q6v5_pas.c:260:28: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem *addr @@     got void *dtb_mem_region @@
>    drivers/remoteproc/qcom_q6v5_pas.c:260:28: sparse:     expected void volatile [noderef] __iomem *addr
>    drivers/remoteproc/qcom_q6v5_pas.c:260:28: sparse:     got void *dtb_mem_region
>    drivers/remoteproc/qcom_q6v5_pas.c:344:33: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *mem_region @@     got void [noderef] __iomem * @@
>    drivers/remoteproc/qcom_q6v5_pas.c:344:33: sparse:     expected void *mem_region
>    drivers/remoteproc/qcom_q6v5_pas.c:344:33: sparse:     got void [noderef] __iomem *
> >> drivers/remoteproc/qcom_q6v5_pas.c:354:20: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem *addr @@     got void *mem_region @@
>    drivers/remoteproc/qcom_q6v5_pas.c:354:20: sparse:     expected void volatile [noderef] __iomem *addr
>    drivers/remoteproc/qcom_q6v5_pas.c:354:20: sparse:     got void *mem_region
>    drivers/remoteproc/qcom_q6v5_pas.c:481:25: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *mem_region @@     got void [noderef] __iomem * @@
>    drivers/remoteproc/qcom_q6v5_pas.c:481:25: sparse:     expected void *mem_region
>    drivers/remoteproc/qcom_q6v5_pas.c:481:25: sparse:     got void [noderef] __iomem *
> 
> vim +260 drivers/remoteproc/qcom_q6v5_pas.c
> 
>    124	
>    125	static void qcom_pas_segment_dump(struct rproc *rproc,
>    126					  struct rproc_dump_segment *segment,
>    127					  void *dest, size_t offset, size_t size)
>    128	{
>    129		struct qcom_pas *pas = rproc->priv;
>    130		int total_offset;
>    131	
>    132		total_offset = segment->da + segment->offset + offset - pas->mem_phys;
>    133		if (total_offset < 0 || total_offset + size > pas->mem_size) {
>    134			dev_err(pas->dev,
>    135				"invalid copy request for segment %pad with offset %zu and size %zu)\n",
>    136				&segment->da, offset, size);
>    137			memset(dest, 0xff, size);
>    138			return;
>    139		}
>    140	
>  > 141		pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
>    142		if (!pas->mem_region) {
>    143			dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
>    144				&pas->mem_phys, pas->mem_size);
>    145			return;
>    146		}
>    147	
>    148		memcpy_fromio(dest, pas->mem_region + total_offset, size);
>    149	}
>    150	
>    151	static void qcom_pas_minidump(struct rproc *rproc)
>    152	{
>    153		struct qcom_pas *pas = rproc->priv;
>    154	
>    155		if (rproc->dump_conf == RPROC_COREDUMP_DISABLED)
>    156			return;
>    157	
>    158		qcom_minidump(rproc, pas->minidump_id, qcom_pas_segment_dump);
>    159	}
>    160	
>    161	static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
>    162				       size_t pd_count)
>    163	{
>    164		int ret;
>    165		int i;
>    166	
>    167		for (i = 0; i < pd_count; i++) {
>    168			dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
>    169			ret = pm_runtime_get_sync(pds[i]);
>    170			if (ret < 0) {
>    171				pm_runtime_put_noidle(pds[i]);
>    172				dev_pm_genpd_set_performance_state(pds[i], 0);
>    173				goto unroll_pd_votes;
>    174			}
>    175		}
>    176	
>    177		return 0;
>    178	
>    179	unroll_pd_votes:
>    180		for (i--; i >= 0; i--) {
>    181			dev_pm_genpd_set_performance_state(pds[i], 0);
>    182			pm_runtime_put(pds[i]);
>    183		}
>    184	
>    185		return ret;
>    186	};
>    187	
>    188	static void qcom_pas_pds_disable(struct qcom_pas *pas, struct device **pds,
>    189					 size_t pd_count)
>    190	{
>    191		int i;
>    192	
>    193		for (i = 0; i < pd_count; i++) {
>    194			dev_pm_genpd_set_performance_state(pds[i], 0);
>    195			pm_runtime_put(pds[i]);
>    196		}
>    197	}
>    198	
>    199	static int qcom_pas_shutdown_poll_decrypt(struct qcom_pas *pas)
>    200	{
>    201		unsigned int retry_num = 50;
>    202		int ret;
>    203	
>    204		do {
>    205			msleep(QCOM_PAS_DECRYPT_SHUTDOWN_DELAY_MS);
>    206			ret = qcom_scm_pas_shutdown(pas->pas_id);
>    207		} while (ret == -EINVAL && --retry_num);
>    208	
>    209		return ret;
>    210	}
>    211	
>    212	static int qcom_pas_unprepare(struct rproc *rproc)
>    213	{
>    214		struct qcom_pas *pas = rproc->priv;
>    215	
>    216		/*
>    217		 * qcom_pas_load() did pass pas_metadata to the SCM driver for storing
>    218		 * metadata context. It might have been released already if
>    219		 * auth_and_reset() was successful, but in other cases clean it up
>    220		 * here.
>    221		 */
>    222		qcom_scm_pas_metadata_release(pas->pas_ctx);
>    223		if (pas->dtb_pas_id)
>    224			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
>    225	
>    226		return 0;
>    227	}
>    228	
>    229	static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
>    230	{
>    231		struct qcom_pas *pas = rproc->priv;
>    232		int ret;
>    233	
>    234		/* Store firmware handle to be used in qcom_pas_start() */
>    235		pas->firmware = fw;
>    236	
>    237		if (pas->lite_pas_id)
>    238			qcom_scm_pas_shutdown(pas->lite_pas_id);
>    239		if (pas->lite_dtb_pas_id)
>    240			qcom_scm_pas_shutdown(pas->lite_dtb_pas_id);
>    241	
>    242		if (pas->dtb_pas_id) {
>    243			ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name, pas->dev);
>    244			if (ret) {
>    245				dev_err(pas->dev, "request_firmware failed for %s: %d\n",
>    246					pas->dtb_firmware_name, ret);
>    247				return ret;
>    248			}
>    249	
>  > 250			pas->dtb_mem_region = ioremap_wc(pas->dtb_mem_phys, pas->dtb_mem_size);
>    251			if (!pas->dtb_mem_region) {
>    252				dev_err(pas->dev, "unable to map dtb memory region: %pa+%zx\n",
>    253					&pas->dtb_mem_phys, pas->dtb_mem_size);
>    254				goto release_dtb_metadata;
>    255			}
>    256	
>    257			ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
>    258						pas->dtb_firmware_name, pas->dtb_mem_region,
>    259						&pas->dtb_mem_reloc);
>  > 260			iounmap(pas->dtb_mem_region);
>    261			pas->dtb_mem_region = NULL;
>    262			if (ret)
>    263				goto release_dtb_metadata;
>    264		}
>    265	
>    266		return 0;
>    267	
>    268	release_dtb_metadata:
>    269		if (pas->dtb_pas_id)
>    270			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
>    271	
>    272		release_firmware(pas->dtb_firmware);
>    273	
>    274		return ret;
>    275	}
>    276	
>    277	static void qcom_pas_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t size)
>    278	{
>    279		if (rproc->has_iommu)
>    280			iommu_unmap(rproc->domain, mem_phys, size);
>    281	}
>    282	
>    283	static int qcom_pas_map_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t size)
>    284	{
>    285		int ret = 0;
>    286	
>    287		if (rproc->has_iommu)
>    288			ret = iommu_map(rproc->domain, mem_phys, mem_phys, size,
>    289					IOMMU_READ | IOMMU_WRITE, GFP_KERNEL);
>    290		return ret;
>    291	}
>    292	
>    293	static int qcom_pas_start(struct rproc *rproc)
>    294	{
>    295		struct qcom_pas *pas = rproc->priv;
>    296		int ret;
>    297	
>    298		ret = qcom_q6v5_prepare(&pas->q6v5);
>    299		if (ret)
>    300			return ret;
>    301	
>    302		ret = qcom_pas_pds_enable(pas, pas->proxy_pds, pas->proxy_pd_count);
>    303		if (ret < 0)
>    304			goto disable_irqs;
>    305	
>    306		ret = clk_prepare_enable(pas->xo);
>    307		if (ret)
>    308			goto disable_proxy_pds;
>    309	
>    310		ret = clk_prepare_enable(pas->aggre2_clk);
>    311		if (ret)
>    312			goto disable_xo_clk;
>    313	
>    314		if (pas->cx_supply) {
>    315			ret = regulator_enable(pas->cx_supply);
>    316			if (ret)
>    317				goto disable_aggre2_clk;
>    318		}
>    319	
>    320		if (pas->px_supply) {
>    321			ret = regulator_enable(pas->px_supply);
>    322			if (ret)
>    323				goto disable_cx_supply;
>    324		}
>    325	
>    326		if (pas->dtb_pas_id) {
>    327			ret = qcom_pas_map_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
>    328			if (ret)
>    329				goto disable_px_supply;
>    330	
>    331			ret = qcom_scm_pas_prepare_and_auth_reset(pas->dtb_pas_ctx);
>    332			if (ret) {
>    333				dev_err(pas->dev,
>    334					"failed to authenticate dtb image and release reset\n");
>    335				goto unmap_dtb_carveout;
>    336			}
>    337		}
>    338	
>    339		/*
>    340		 * During subsystem restart, when coredump is enabled, region is mapped but
>    341		 * not unmapped there, NULL check to reuse the mapping if its already mapped.
>    342		 */
>    343		if (!pas->mem_region) {
>    344			pas->mem_region = ioremap_wc(pas->mem_phys, pas->mem_size);
>    345			if (!pas->mem_region) {
>    346				dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
>    347					&pas->mem_phys, pas->mem_size);
>    348				goto release_pas_metadata;
>    349			}
>    350		}
>    351	
>    352		ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
>    353					pas->mem_region, &pas->mem_reloc);
>  > 354		iounmap(pas->mem_region);
>    355		pas->mem_region = NULL;
>    356		if (ret)
>    357			goto release_pas_metadata;
>    358	
>    359		qcom_pil_info_store(pas->info_name, pas->mem_phys, pas->mem_size);
>    360	
>    361		ret = qcom_pas_map_carveout(rproc, pas->mem_phys, pas->mem_size);
>    362		if (ret)
>    363			goto release_pas_metadata;
>    364	
>    365		ret = qcom_scm_pas_prepare_and_auth_reset(pas->pas_ctx);
>    366		if (ret) {
>    367			dev_err(pas->dev,
>    368				"failed to authenticate image and release reset\n");
>    369			goto unmap_carveout;
>    370		}
>    371	
>    372		ret = qcom_q6v5_wait_for_start(&pas->q6v5, msecs_to_jiffies(5000));
>    373		if (ret == -ETIMEDOUT) {
>    374			dev_err(pas->dev, "start timed out\n");
>    375			qcom_scm_pas_shutdown(pas->pas_id);
>    376			goto unmap_carveout;
>    377		}
>    378	
>    379		qcom_scm_pas_metadata_release(pas->pas_ctx);
>    380		if (pas->dtb_pas_id)
>    381			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
>    382	
>    383		/* firmware is used to pass reference from qcom_pas_start(), drop it now */
>    384		pas->firmware = NULL;
>    385	
>    386		return 0;
>    387	
>    388	unmap_carveout:
>    389		qcom_pas_unmap_carveout(rproc, pas->mem_phys, pas->mem_size);
>    390	release_pas_metadata:
>    391		qcom_scm_pas_metadata_release(pas->pas_ctx);
>    392		if (pas->dtb_pas_id)
>    393			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
>    394	
>    395	unmap_dtb_carveout:
>    396		if (pas->dtb_pas_id)
>    397			qcom_pas_unmap_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
>    398	disable_px_supply:
>    399		if (pas->px_supply)
>    400			regulator_disable(pas->px_supply);
>    401	disable_cx_supply:
>    402		if (pas->cx_supply)
>    403			regulator_disable(pas->cx_supply);
>    404	disable_aggre2_clk:
>    405		clk_disable_unprepare(pas->aggre2_clk);
>    406	disable_xo_clk:
>    407		clk_disable_unprepare(pas->xo);
>    408	disable_proxy_pds:
>    409		qcom_pas_pds_disable(pas, pas->proxy_pds, pas->proxy_pd_count);
>    410	disable_irqs:
>    411		qcom_q6v5_unprepare(&pas->q6v5);
>    412	
>    413		/* firmware is used to pass reference from qcom_pas_start(), drop it now */
>    414		pas->firmware = NULL;
>    415	
>    416		return ret;
>    417	}
>    418	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

-- 
-Mukesh Ojha

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-03-24 12:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-10 13:52 [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation Mukesh Ojha
2026-03-10 13:52 ` [PATCH 2/2] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset Mukesh Ojha
2026-03-11  2:34   ` Dmitry Baryshkov
2026-03-11  8:49     ` Mukesh Ojha
2026-03-13  3:40       ` Dmitry Baryshkov
2026-03-13  5:57         ` Mukesh Ojha
2026-03-13 17:20           ` Dmitry Baryshkov
2026-03-18 23:20   ` kernel test robot
2026-03-24 12:55     ` Mukesh Ojha
2026-03-11  2:28 ` [PATCH 1/2] remoteproc: qcom: pas: Fix the dtb PAS context creation Dmitry Baryshkov
2026-03-11  7:01   ` Mukesh Ojha

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®