[PATCH v4 0/6] remoteproc: qcom: pas: Misc fixes

[PATCH v4 0/6] remoteproc: qcom: pas: Misc fixes

[Mukesh Ojha]

This series is a collection of misc fixes for the Qualcomm PAS remoteproc
driver and its supporting SCM/MDT loader infrastructure.

- Fix sparse __iomem warnings in qcom_adsp, qcom_pas and qcom_wcnss by
  annotating mem_region fields with __iomem and using __force at call sites.
- Guard the DTB metadata release in qcom_pas_load() with a dtb_pas_id check,
  consistent with all other release sites in the driver.
- Move PAS context allocation from probe into qcom_pas_alloc_memory_region()
  so the DTB context is only created for subsystems that actually use it.
- Map/unmap the subsystem memory region on demand around firmware load and
  coredump, avoiding potential XPU violations on EL2 platforms where the
  region may be handed off to the remote side.
- Drop the now-unused dtb_mem_region field from struct qcom_pas.
- Add a keep_mdt_buf flag to struct qcom_scm_pas_context to make metadata
  buffer retention explicit rather than implicit.

Changes in v4:
 https://lore.kernel.org/lkml/20260331183957.2015440-1-mukesh.ojha@oss.qualcomm.com/
 https://lore.kernel.org/lkml/20260331191210.2019758-2-mukesh.ojha@oss.qualcomm.com/

 - Last series mistakenly divided two series cause laptop shutdown.
 - Some minor refactor common code rest is same a rebased on latest
   kernel.
 - Added new 6/6 to the series.

Mukesh Ojha (6):
  remoteproc: qcom: fix sparse warnings for __iomem annotated mem_region
  remoteproc: qcom: pas: Guard dtb metadata release with dtb_pas_id
    check
  remoteproc: qcom: pas: Fix the PAS context creation placement
  remoteproc: qcom: pas: Map/unmap subsystem region before
    auth_and_reset
  remoteproc: qcom: pas: Drop unused dtb_mem_region field
  firmware: qcom: scm: introduce keep_mdt_buf flag in PAS context

 drivers/firmware/qcom/qcom_scm.c       | 21 +++++-
 drivers/remoteproc/qcom_q6v5_adsp.c    |  6 +-
 drivers/remoteproc/qcom_q6v5_pas.c     | 99 +++++++++++++++-----------
 drivers/remoteproc/qcom_wcnss.c        |  6 +-
 drivers/soc/qcom/mdt_loader.c          | 18 +++--
 include/linux/firmware/qcom/qcom_scm.h |  1 +
 include/linux/soc/qcom/mdt_loader.h    |  4 +-
 7 files changed, 99 insertions(+), 56 deletions(-)

-- 
2.53.0

[PATCH v4 1/6] remoteproc: qcom: fix sparse warnings for __iomem annotated mem_region

[Mukesh Ojha]

The firmware memory regions in qcom_adsp, qcom_pas and qcom_wcnss are
mapped using devm_ioremap_wc() / devm_ioremap_resource_wc(), which
return void __iomem *. However, the mem_region (and dtb_mem_region)
fields in the respective driver structs were declared as plain void *,
causing sparse to flag address space mismatches:

qcom_q6v5_adsp.c:639:26: warning: incorrect type in assignment (different address spaces)
qcom_q6v5_adsp.c:639:26:    expected void *mem_region
qcom_q6v5_adsp.c:639:26:    got void [noderef] __iomem *
qcom_q6v5_pas.c:141:45: warning: incorrect type in argument 2 (different address spaces)
qcom_q6v5_pas.c:141:45:    expected void const volatile [noderef] __iomem *src
qcom_q6v5_pas.c:141:45:    got void *
qcom_q6v5_pas.c:637:25: warning: incorrect type in assignment (different address spaces)
qcom_q6v5_pas.c:637:25:    expected void *mem_region
qcom_q6v5_pas.c:637:25:    got void [noderef] __iomem *
qcom_q6v5_pas.c:654:29: warning: incorrect type in assignment (different address spaces)
qcom_q6v5_pas.c:654:29:    expected void *dtb_mem_region
qcom_q6v5_pas.c:654:29:    got void [noderef] __iomem *
qcom_wcnss.c:540:27: warning: incorrect type in assignment (different address spaces)
qcom_wcnss.c:540:27:    expected void *mem_region
qcom_wcnss.c:540:27:    got void [noderef] __iomem *

Fix this by annotating the struct fields with __iomem to correctly
reflect the address space of the underlying mapping.

These regions are subsequently passed to qcom_mdt_load(),
qcom_mdt_load_no_init() and qcom_mdt_pas_load(), all of which take
void * and use plain memcpy()/memset() internally to write firmware
segments into the region. This is intentional and safe: the mappings
are write-combining (WC), which on arm64 permits bulk CPU stores
without requiring the memcpy_toio()/memset_io() accessors. Changing
the MDT loader API to accept void __iomem * would be a more invasive
change and would affect callers.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_adsp.c |  6 +++---
 drivers/remoteproc/qcom_q6v5_pas.c  | 10 +++++-----
 drivers/remoteproc/qcom_wcnss.c     |  6 +++---
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index b5c8d6d38c9c..d2b50af6d748 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -105,7 +105,7 @@ struct qcom_adsp {
 
 	phys_addr_t mem_phys;
 	phys_addr_t mem_reloc;
-	void *mem_region;
+	void __iomem *mem_region;
 	size_t mem_size;
 	bool has_iommu;
 
@@ -318,7 +318,7 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
 	int ret;
 
 	ret = qcom_mdt_load_no_init(adsp->dev, fw, rproc->firmware,
-				    adsp->mem_region, adsp->mem_phys,
+				    (__force void *)adsp->mem_region, adsp->mem_phys,
 				    adsp->mem_size, &adsp->mem_reloc);
 	if (ret)
 		return ret;
@@ -491,7 +491,7 @@ static void *adsp_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iom
 	if (offset < 0 || offset + len > adsp->mem_size)
 		return NULL;
 
-	return adsp->mem_region + offset;
+	return (__force void *)adsp->mem_region + offset;
 }
 
 static int adsp_parse_firmware(struct rproc *rproc, const struct firmware *fw)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index da27d1d3c9da..45be8c5049e1 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -100,8 +100,8 @@ struct qcom_pas {
 	phys_addr_t mem_reloc;
 	phys_addr_t dtb_mem_reloc;
 	phys_addr_t region_assign_phys[MAX_ASSIGN_COUNT];
-	void *mem_region;
-	void *dtb_mem_region;
+	void __iomem *mem_region;
+	void __iomem *dtb_mem_region;
 	size_t mem_size;
 	size_t dtb_mem_size;
 	size_t region_assign_size[MAX_ASSIGN_COUNT];
@@ -241,7 +241,7 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 		}
 
 		ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
-					pas->dtb_firmware_name, pas->dtb_mem_region,
+					pas->dtb_firmware_name, (__force void *)pas->dtb_mem_region,
 					&pas->dtb_mem_reloc);
 		if (ret)
 			goto release_dtb_metadata;
@@ -319,7 +319,7 @@ static int qcom_pas_start(struct rproc *rproc)
 	}
 
 	ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
-				pas->mem_region, &pas->mem_reloc);
+				(__force void *)pas->mem_region, &pas->mem_reloc);
 	if (ret)
 		goto release_pas_metadata;
 
@@ -445,7 +445,7 @@ static void *qcom_pas_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is
 	if (is_iomem)
 		*is_iomem = true;
 
-	return pas->mem_region + offset;
+	return (__force void *)pas->mem_region + offset;
 }
 
 static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *fw)
diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index 4add9037dbd5..da68bc1903be 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -94,7 +94,7 @@ struct qcom_wcnss {
 
 	phys_addr_t mem_phys;
 	phys_addr_t mem_reloc;
-	void *mem_region;
+	void __iomem *mem_region;
 	size_t mem_size;
 
 	struct qcom_rproc_subdev smd_subdev;
@@ -158,7 +158,7 @@ static int wcnss_load(struct rproc *rproc, const struct firmware *fw)
 	int ret;
 
 	ret = qcom_mdt_load(wcnss->dev, fw, rproc->firmware, WCNSS_PAS_ID,
-			    wcnss->mem_region, wcnss->mem_phys,
+			    (__force void *)wcnss->mem_region, wcnss->mem_phys,
 			    wcnss->mem_size, &wcnss->mem_reloc);
 	if (ret)
 		return ret;
@@ -327,7 +327,7 @@ static void *wcnss_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_io
 	if (offset < 0 || offset + len > wcnss->mem_size)
 		return NULL;
 
-	return wcnss->mem_region + offset;
+	return (__force void *)wcnss->mem_region + offset;
 }
 
 static const struct rproc_ops wcnss_ops = {
-- 
2.53.0

[PATCH v4 2/6] remoteproc: qcom: pas: Guard dtb metadata release with dtb_pas_id check

[Mukesh Ojha]

All other call sites of qcom_scm_pas_metadata_release() for the DTB
context are guarded by a check on pas->dtb_pas_id, but the call inside
qcom_pas_load() was not. Fix this by moving the call to the guarded
block.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 45be8c5049e1..2785d19408d1 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -243,17 +243,14 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 		ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
 					pas->dtb_firmware_name, (__force void *)pas->dtb_mem_region,
 					&pas->dtb_mem_reloc);
-		if (ret)
-			goto release_dtb_metadata;
+		if (ret) {
+			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
+			release_firmware(pas->dtb_firmware);
+			return ret;
+		}
 	}
 
 	return 0;
-
-release_dtb_metadata:
-	qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
-	release_firmware(pas->dtb_firmware);
-
-	return ret;
 }
 
 static void qcom_pas_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t size)
-- 
2.53.0

[PATCH v4 3/6] remoteproc: qcom: pas: Fix the PAS context creation placement

[Mukesh Ojha]

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.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 32 ++++++++++++++----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 2785d19408d1..a64fbf7225aa 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -620,6 +620,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;
 
@@ -637,6 +638,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;
 
@@ -654,6 +661,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;
 }
 
@@ -835,23 +850,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.53.0

[PATCH v4 4/6] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset

[Mukesh Ojha]

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  | 45 ++++++++++++++++++++---------
 drivers/soc/qcom/mdt_loader.c       | 18 +++++++++---
 include/linux/soc/qcom/mdt_loader.h |  4 +--
 3 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index a64fbf7225aa..6f9fe38fb1c4 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -141,6 +141,18 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
 	memcpy_fromio(dest, pas->mem_region + total_offset, size);
 }
 
+static int qcom_pas_map_mem_region(struct qcom_pas *pas)
+{
+	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 -ENOMEM;
+	}
+
+	return 0;
+}
+
 static void qcom_pas_minidump(struct rproc *rproc)
 {
 	struct qcom_pas *pas = rproc->priv;
@@ -148,7 +160,12 @@ static void qcom_pas_minidump(struct rproc *rproc)
 	if (rproc->dump_conf == RPROC_COREDUMP_DISABLED)
 		return;
 
+	if (qcom_pas_map_mem_region(pas))
+		return;
+
 	qcom_minidump(rproc, pas->minidump_id, qcom_pas_segment_dump);
+	iounmap(pas->mem_region);
+	pas->mem_region = NULL;
 }
 
 static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
@@ -241,8 +258,7 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 		}
 
 		ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
-					pas->dtb_firmware_name, (__force void *)pas->dtb_mem_region,
-					&pas->dtb_mem_reloc);
+					pas->dtb_firmware_name, &pas->dtb_mem_reloc);
 		if (ret) {
 			qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
 			release_firmware(pas->dtb_firmware);
@@ -316,7 +332,7 @@ static int qcom_pas_start(struct rproc *rproc)
 	}
 
 	ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
-				(__force void *)pas->mem_region, &pas->mem_reloc);
+				&pas->mem_reloc);
 	if (ret)
 		goto release_pas_metadata;
 
@@ -507,6 +523,18 @@ static unsigned long qcom_pas_panic(struct rproc *rproc)
 	return qcom_q6v5_panic(&pas->q6v5);
 }
 
+static void qcom_pas_coredump(struct rproc *rproc)
+{
+	struct qcom_pas *pas = rproc->priv;
+
+	if (qcom_pas_map_mem_region(pas))
+		return;
+
+	rproc_coredump(rproc);
+	iounmap(pas->mem_region);
+	pas->mem_region = NULL;
+}
+
 static const struct rproc_ops qcom_pas_ops = {
 	.unprepare = qcom_pas_unprepare,
 	.start = qcom_pas_start,
@@ -515,6 +543,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,
 };
 
 static const struct rproc_ops qcom_pas_minidump_ops = {
@@ -632,11 +661,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);
@@ -655,11 +679,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,
diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index c004d444d698..ed882dd43587 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -11,6 +11,7 @@
 #include <linux/device.h>
 #include <linux/elf.h>
 #include <linux/firmware.h>
+#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/firmware/qcom/qcom_scm.h>
@@ -478,22 +479,31 @@ EXPORT_SYMBOL_GPL(qcom_mdt_load);
  * @ctx:        Pointer to the PAS (Peripheral Authentication Service) context
  * @fw:         Firmware object representing the .mdt file
  * @firmware:   Name of the firmware used to construct segment file names
- * @mem_region: Memory region allocated for loading the firmware
  * @reloc_base: Physical address adjusted after relocation
  *
  * Return: 0 on success or a negative error code on failure.
  */
 int qcom_mdt_pas_load(struct qcom_scm_pas_context *ctx, const struct firmware *fw,
-		      const char *firmware, void *mem_region, phys_addr_t *reloc_base)
+		      const char *firmware, phys_addr_t *reloc_base)
 {
+	void __iomem *mem_region;
 	int ret;
 
 	ret = __qcom_mdt_pas_init(ctx->dev, fw, firmware, ctx->pas_id, ctx->mem_phys, ctx);
 	if (ret)
 		return ret;
 
-	return qcom_mdt_load_no_init(ctx->dev, fw, firmware, mem_region, ctx->mem_phys,
-				     ctx->mem_size, reloc_base);
+	mem_region = ioremap_wc(ctx->mem_phys, ctx->mem_size);
+	if (!mem_region) {
+		dev_err(ctx->dev, "unable to map memory region: %pa+%zx\n", &ctx->mem_phys,
+			ctx->mem_size);
+		return -ENOMEM;
+	}
+
+	ret = qcom_mdt_load_no_init(ctx->dev, fw, firmware, (__force void *)mem_region,
+				    ctx->mem_phys, ctx->mem_size, reloc_base);
+	iounmap(mem_region);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(qcom_mdt_pas_load);
 
diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
index 82372e0db0a1..7c551b98e182 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -21,7 +21,7 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 		  phys_addr_t *reloc_base);
 
 int qcom_mdt_pas_load(struct qcom_scm_pas_context *ctx, const struct firmware *fw,
-		      const char *firmware, void *mem_region, phys_addr_t *reloc_base);
+		      const char *firmware, phys_addr_t *reloc_base);
 
 int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
 			  const char *fw_name, void *mem_region,
@@ -47,7 +47,7 @@ static inline int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 
 static inline int qcom_mdt_pas_load(struct qcom_scm_pas_context *ctx,
 				    const struct firmware *fw, const char *firmware,
-				    void *mem_region, phys_addr_t *reloc_base)
+				    phys_addr_t *reloc_base)
 {
 	return -ENODEV;
 }
-- 
2.53.0

[PATCH v4 5/6] remoteproc: qcom: pas: Drop unused dtb_mem_region field

[Mukesh Ojha]

dtb_mem_region is no longer referenced after the ioremap was moved
to respective places where mapping is required. Remove it from
struct qcom_pas.

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

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 6f9fe38fb1c4..5be3070fd52b 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -100,8 +100,9 @@ struct qcom_pas {
 	phys_addr_t mem_reloc;
 	phys_addr_t dtb_mem_reloc;
 	phys_addr_t region_assign_phys[MAX_ASSIGN_COUNT];
+
 	void __iomem *mem_region;
-	void __iomem *dtb_mem_region;
+
 	size_t mem_size;
 	size_t dtb_mem_size;
 	size_t region_assign_size[MAX_ASSIGN_COUNT];
-- 
2.53.0

[PATCH v4 6/6] firmware: qcom: scm: introduce keep_mdt_buf flag in PAS context

[Mukesh Ojha]

The PAS image initialization path always retains the metadata buffer
when a valid qcom_scm_pas_context is provided, even if the caller does
not require it. This implicit behavior leads to unclear buffer ownership
and forces new users of qcom_mdt_pas_load() to manually release
metadata, which is error‑ prone and incorrect.

Add a keep_mdt_buf flag to struct qcom_scm_pas_context to make metadata
retention explicit.  Metadata buffers are now freed by default and are
only preserved when this flag is set. qcom_q6v5_pas enables this during
probe for contexts that require retained metadata for subsequent PAS
operations, while existing callers continue to work unchanged.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c       | 21 ++++++++++++++++++---
 drivers/remoteproc/qcom_q6v5_pas.c     |  2 ++
 include/linux/firmware/qcom/qcom_scm.h |  1 +
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 9b06a69d3a6d..2cae35e7c583 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -651,7 +651,7 @@ static int qcom_scm_pas_prep_and_init_image(struct qcom_scm_pas_context *ctx,
 	mdata_phys = qcom_tzmem_to_phys(mdata_buf);
 
 	ret = __qcom_scm_pas_init_image(ctx->pas_id, mdata_phys, &res);
-	if (ret < 0)
+	if (ret < 0 || !ctx->keep_mdt_buf)
 		qcom_tzmem_free(mdata_buf);
 	else
 		ctx->ptr = mdata_buf;
@@ -707,9 +707,24 @@ int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
 	memcpy(mdata_buf, metadata, size);
 
 	ret = __qcom_scm_pas_init_image(pas_id, mdata_phys, &res);
-	if (ret < 0 || !ctx) {
+
+	/*
+	 * Some clients still pass the PAS context as NULL. Until all clients
+	 * switch to qcom_mdt_pas_load() and provide a valid PAS context, check
+	 * for NULL before dereferencing it.
+	 *
+	 * When a valid context is provided, metadata handling differs across
+	 * clients. For example, modem clients pass metadata to TrustZone that
+	 * must not be freed until the authentication and reset SMCs are
+	 * invoked, as the buffers remain locked until then.
+	 *
+	 * Other clients free their metadata immediately after the PAS_INIT
+	 * SMC call. Therefore, keep_mdt_buf should be set to true for modem
+	 * clients and false for others.
+	 */
+	if (ret < 0 || !ctx || !ctx->keep_mdt_buf) {
 		dma_free_coherent(__scm->dev, size, mdata_buf, mdata_phys);
-	} else if (ctx) {
+	} else {
 		ctx->ptr = mdata_buf;
 		ctx->phys = mdata_phys;
 		ctx->size = size;
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 5be3070fd52b..7858e14c0bee 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -669,6 +669,7 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 		return PTR_ERR(pas->pas_ctx);
 
 	pas->pas_ctx->use_tzmem = rproc->has_iommu;
+	pas->pas_ctx->keep_mdt_buf = true;
 	if (!pas->dtb_pas_id)
 		return 0;
 
@@ -688,6 +689,7 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 		return PTR_ERR(pas->dtb_pas_ctx);
 
 	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
+	pas->dtb_pas_ctx->keep_mdt_buf = true;
 
 	return 0;
 }
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index 5747bd191bf1..6d8d3deb02e0 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -75,6 +75,7 @@ struct qcom_scm_pas_context {
 	dma_addr_t phys;
 	ssize_t size;
 	bool use_tzmem;
+	bool keep_mdt_buf;
 };
 
 struct qcom_scm_pas_context *devm_qcom_scm_pas_context_alloc(struct device *dev,
-- 
2.53.0

Re: [PATCH v4 0/6] remoteproc: qcom: pas: Misc fixes

[Mukesh Ojha]

On Fri, May 01, 2026 at 12:42:47AM +0530, Mukesh Ojha wrote:
> This series is a collection of misc fixes for the Qualcomm PAS remoteproc
> driver and its supporting SCM/MDT loader infrastructure.
> 
> - Fix sparse __iomem warnings in qcom_adsp, qcom_pas and qcom_wcnss by
>   annotating mem_region fields with __iomem and using __force at call sites.
> - Guard the DTB metadata release in qcom_pas_load() with a dtb_pas_id check,
>   consistent with all other release sites in the driver.
> - Move PAS context allocation from probe into qcom_pas_alloc_memory_region()
>   so the DTB context is only created for subsystems that actually use it.
> - Map/unmap the subsystem memory region on demand around firmware load and
>   coredump, avoiding potential XPU violations on EL2 platforms where the
>   region may be handed off to the remote side.
> - Drop the now-unused dtb_mem_region field from struct qcom_pas.
> - Add a keep_mdt_buf flag to struct qcom_scm_pas_context to make metadata
>   buffer retention explicit rather than implicit.

Reminder for review and if they look fine, can this be picked up?

> 
> Changes in v4:
>  https://lore.kernel.org/lkml/20260331183957.2015440-1-mukesh.ojha@oss.qualcomm.com/
>  https://lore.kernel.org/lkml/20260331191210.2019758-2-mukesh.ojha@oss.qualcomm.com/
> 
>  - Last series mistakenly divided two series cause laptop shutdown.
>  - Some minor refactor common code rest is same a rebased on latest
>    kernel.
>  - Added new 6/6 to the series.
> 
> Mukesh Ojha (6):
>   remoteproc: qcom: fix sparse warnings for __iomem annotated mem_region
>   remoteproc: qcom: pas: Guard dtb metadata release with dtb_pas_id
>     check
>   remoteproc: qcom: pas: Fix the PAS context creation placement
>   remoteproc: qcom: pas: Map/unmap subsystem region before
>     auth_and_reset
>   remoteproc: qcom: pas: Drop unused dtb_mem_region field
>   firmware: qcom: scm: introduce keep_mdt_buf flag in PAS context
> 
>  drivers/firmware/qcom/qcom_scm.c       | 21 +++++-
>  drivers/remoteproc/qcom_q6v5_adsp.c    |  6 +-
>  drivers/remoteproc/qcom_q6v5_pas.c     | 99 +++++++++++++++-----------
>  drivers/remoteproc/qcom_wcnss.c        |  6 +-
>  drivers/soc/qcom/mdt_loader.c          | 18 +++--
>  include/linux/firmware/qcom/qcom_scm.h |  1 +
>  include/linux/soc/qcom/mdt_loader.h    |  4 +-
>  7 files changed, 99 insertions(+), 56 deletions(-)
> 
> -- 
> 2.53.0
> 

-- 
-Mukesh Ojha

Re: [PATCH v4 1/6] remoteproc: qcom: fix sparse warnings for __iomem annotated mem_region

[Konrad Dybcio]

On 4/30/26 9:12 PM, Mukesh Ojha wrote:
> The firmware memory regions in qcom_adsp, qcom_pas and qcom_wcnss are
> mapped using devm_ioremap_wc() / devm_ioremap_resource_wc(), which
> return void __iomem *. However, the mem_region (and dtb_mem_region)
> fields in the respective driver structs were declared as plain void *,
> causing sparse to flag address space mismatches:
> 
> qcom_q6v5_adsp.c:639:26: warning: incorrect type in assignment (different address spaces)
> qcom_q6v5_adsp.c:639:26:    expected void *mem_region
> qcom_q6v5_adsp.c:639:26:    got void [noderef] __iomem *
> qcom_q6v5_pas.c:141:45: warning: incorrect type in argument 2 (different address spaces)
> qcom_q6v5_pas.c:141:45:    expected void const volatile [noderef] __iomem *src
> qcom_q6v5_pas.c:141:45:    got void *
> qcom_q6v5_pas.c:637:25: warning: incorrect type in assignment (different address spaces)
> qcom_q6v5_pas.c:637:25:    expected void *mem_region
> qcom_q6v5_pas.c:637:25:    got void [noderef] __iomem *
> qcom_q6v5_pas.c:654:29: warning: incorrect type in assignment (different address spaces)
> qcom_q6v5_pas.c:654:29:    expected void *dtb_mem_region
> qcom_q6v5_pas.c:654:29:    got void [noderef] __iomem *
> qcom_wcnss.c:540:27: warning: incorrect type in assignment (different address spaces)
> qcom_wcnss.c:540:27:    expected void *mem_region
> qcom_wcnss.c:540:27:    got void [noderef] __iomem *
> 
> Fix this by annotating the struct fields with __iomem to correctly
> reflect the address space of the underlying mapping.
> 
> These regions are subsequently passed to qcom_mdt_load(),
> qcom_mdt_load_no_init() and qcom_mdt_pas_load(), all of which take
> void * and use plain memcpy()/memset() internally to write firmware
> segments into the region. This is intentional and safe: the mappings
> are write-combining (WC), which on arm64 permits bulk CPU stores
> without requiring the memcpy_toio()/memset_io() accessors. Changing
> the MDT loader API to accept void __iomem * would be a more invasive
> change and would affect callers.

Zooming out a bit more, should we even be ioremapping these regions
in the first place? Are they not just RAM/Normal Memory in arm parlance?

Would switching to devm_memremap(, MEMREMAP_WC) (like we do in RMTFS
today) be an even better solution here?

Konrad

Re: [PATCH v4 1/6] remoteproc: qcom: fix sparse warnings for __iomem annotated mem_region

[Mukesh Ojha]

On Thu, Jun 18, 2026 at 03:53:22PM +0200, Konrad Dybcio wrote:
> On 4/30/26 9:12 PM, Mukesh Ojha wrote:
> > The firmware memory regions in qcom_adsp, qcom_pas and qcom_wcnss are
> > mapped using devm_ioremap_wc() / devm_ioremap_resource_wc(), which
> > return void __iomem *. However, the mem_region (and dtb_mem_region)
> > fields in the respective driver structs were declared as plain void *,
> > causing sparse to flag address space mismatches:
> > 
> > qcom_q6v5_adsp.c:639:26: warning: incorrect type in assignment (different address spaces)
> > qcom_q6v5_adsp.c:639:26:    expected void *mem_region
> > qcom_q6v5_adsp.c:639:26:    got void [noderef] __iomem *
> > qcom_q6v5_pas.c:141:45: warning: incorrect type in argument 2 (different address spaces)
> > qcom_q6v5_pas.c:141:45:    expected void const volatile [noderef] __iomem *src
> > qcom_q6v5_pas.c:141:45:    got void *
> > qcom_q6v5_pas.c:637:25: warning: incorrect type in assignment (different address spaces)
> > qcom_q6v5_pas.c:637:25:    expected void *mem_region
> > qcom_q6v5_pas.c:637:25:    got void [noderef] __iomem *
> > qcom_q6v5_pas.c:654:29: warning: incorrect type in assignment (different address spaces)
> > qcom_q6v5_pas.c:654:29:    expected void *dtb_mem_region
> > qcom_q6v5_pas.c:654:29:    got void [noderef] __iomem *
> > qcom_wcnss.c:540:27: warning: incorrect type in assignment (different address spaces)
> > qcom_wcnss.c:540:27:    expected void *mem_region
> > qcom_wcnss.c:540:27:    got void [noderef] __iomem *
> > 
> > Fix this by annotating the struct fields with __iomem to correctly
> > reflect the address space of the underlying mapping.
> > 
> > These regions are subsequently passed to qcom_mdt_load(),
> > qcom_mdt_load_no_init() and qcom_mdt_pas_load(), all of which take
> > void * and use plain memcpy()/memset() internally to write firmware
> > segments into the region. This is intentional and safe: the mappings
> > are write-combining (WC), which on arm64 permits bulk CPU stores
> > without requiring the memcpy_toio()/memset_io() accessors. Changing
> > the MDT loader API to accept void __iomem * would be a more invasive
> > change and would affect callers.
> 
> Zooming out a bit more, should we even be ioremapping these regions
> in the first place? Are they not just RAM/Normal Memory in arm parlance?

Yes, ioremap may not be required. Don't know why it was kept in the
initial implementation, as for most Qualcomm SoCs this is going to be
normal RAM and not SRAM or something, while other implementations like
qcom_q6v5_mss still use memremap with MEMREMAP_WC.

> 
> Would switching to devm_memremap(, MEMREMAP_WC) (like we do in RMTFS
> today) be an even better solution here?

This looks fine and I do not see any immediate impact; however, I would
like to hear from Bjorn on this, if there is any history..

-- 
-Mukesh Ojha

Re: [PATCH v4 4/6] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset

[Konrad Dybcio]

On 4/30/26 9:12 PM, 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>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v4 5/6] remoteproc: qcom: pas: Drop unused dtb_mem_region field

[Konrad Dybcio]

On 4/30/26 9:12 PM, Mukesh Ojha wrote:
> dtb_mem_region is no longer referenced after the ioremap was moved
> to respective places where mapping is required. Remove it from
> struct qcom_pas.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---

I'dve squashed it into the previous patch but it's ok

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v4 6/6] firmware: qcom: scm: introduce keep_mdt_buf flag in PAS context

[Konrad Dybcio]

On 4/30/26 9:12 PM, Mukesh Ojha wrote:
> The PAS image initialization path always retains the metadata buffer
> when a valid qcom_scm_pas_context is provided, even if the caller does
> not require it. This implicit behavior leads to unclear buffer ownership
> and forces new users of qcom_mdt_pas_load() to manually release
> metadata, which is error‑ prone and incorrect.
> 
> Add a keep_mdt_buf flag to struct qcom_scm_pas_context to make metadata
> retention explicit.  Metadata buffers are now freed by default and are
> only preserved when this flag is set. qcom_q6v5_pas enables this during
> probe for contexts that require retained metadata for subsequent PAS
> operations, while existing callers continue to work unchanged.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---

[...]

>  	ret = __qcom_scm_pas_init_image(pas_id, mdata_phys, &res);
> -	if (ret < 0 || !ctx) {
> +
> +	/*
> +	 * Some clients still pass the PAS context as NULL. Until all clients
> +	 * switch to qcom_mdt_pas_load() and provide a valid PAS context, check
> +	 * for NULL before dereferencing it.
> +	 *
> +	 * When a valid context is provided, metadata handling differs across
> +	 * clients. For example, modem clients pass metadata to TrustZone that
> +	 * must not be freed until the authentication and reset SMCs are
> +	 * invoked, as the buffers remain locked until then.
> +	 *
> +	 * Other clients free their metadata immediately after the PAS_INIT
> +	 * SMC call. Therefore, keep_mdt_buf should be set to true for modem
> +	 * clients and false for others.

You say this...

[...]

> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -669,6 +669,7 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
>  		return PTR_ERR(pas->pas_ctx);
>  
>  	pas->pas_ctx->use_tzmem = rproc->has_iommu;
> +	pas->pas_ctx->keep_mdt_buf = true;
>  	if (!pas->dtb_pas_id)
>  		return 0;
>  
> @@ -688,6 +689,7 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
>  		return PTR_ERR(pas->dtb_pas_ctx);
>  
>  	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
> +	pas->dtb_pas_ctx->keep_mdt_buf = true;

And you set it globally for all PAS rprocs

Konrad

Re: [PATCH v4 6/6] firmware: qcom: scm: introduce keep_mdt_buf flag in PAS context

[Mukesh Ojha]

On Tue, Jun 30, 2026 at 12:39:12PM +0200, Konrad Dybcio wrote:
> On 4/30/26 9:12 PM, Mukesh Ojha wrote:
> > The PAS image initialization path always retains the metadata buffer
> > when a valid qcom_scm_pas_context is provided, even if the caller does
> > not require it. This implicit behavior leads to unclear buffer ownership
> > and forces new users of qcom_mdt_pas_load() to manually release
> > metadata, which is error‑ prone and incorrect.
> > 
> > Add a keep_mdt_buf flag to struct qcom_scm_pas_context to make metadata
> > retention explicit.  Metadata buffers are now freed by default and are
> > only preserved when this flag is set. qcom_q6v5_pas enables this during
> > probe for contexts that require retained metadata for subsequent PAS
> > operations, while existing callers continue to work unchanged.
> > 
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> 
> [...]
> 
> >  	ret = __qcom_scm_pas_init_image(pas_id, mdata_phys, &res);
> > -	if (ret < 0 || !ctx) {
> > +
> > +	/*
> > +	 * Some clients still pass the PAS context as NULL. Until all clients
> > +	 * switch to qcom_mdt_pas_load() and provide a valid PAS context, check
> > +	 * for NULL before dereferencing it.
> > +	 *
> > +	 * When a valid context is provided, metadata handling differs across
> > +	 * clients. For example, modem clients pass metadata to TrustZone that
> > +	 * must not be freed until the authentication and reset SMCs are
> > +	 * invoked, as the buffers remain locked until then.
> > +	 *
> > +	 * Other clients free their metadata immediately after the PAS_INIT
> > +	 * SMC call. Therefore, keep_mdt_buf should be set to true for modem
> > +	 * clients and false for others.
> 
> You say this...
> 
> [...]
> 
> > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > @@ -669,6 +669,7 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
> >  		return PTR_ERR(pas->pas_ctx);
> >  
> >  	pas->pas_ctx->use_tzmem = rproc->has_iommu;
> > +	pas->pas_ctx->keep_mdt_buf = true;
> >  	if (!pas->dtb_pas_id)
> >  		return 0;
> >  
> > @@ -688,6 +689,7 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
> >  		return PTR_ERR(pas->dtb_pas_ctx);
> >  
> >  	pas->dtb_pas_ctx->use_tzmem = rproc->has_iommu;
> > +	pas->dtb_pas_ctx->keep_mdt_buf = true;
> 
> And you set it globally for all PAS rprocs

You are right, but in the upstream Qualcomm PAS driver as of
today, we do not differentiate the freeing of the metadata region
among various subsystems — ADSP, CDSP, MPSS — and it is true for
all: release metadata only once auth and reset is successful. And
I see no side effect even if you delay the metadata release from
PAS init to after auth and reset(). But all the non-remoteproc
subsystems like IRIS, IPA, etc., have their metadata released
right after a successful or failed PAS init call.

I could refine the above comment to reflect what is there today
in upstream.

-- 
-Mukesh Ojha

Re: [PATCH v4 1/6] remoteproc: qcom: fix sparse warnings for __iomem annotated mem_region

[Mukesh Ojha]

On Mon, Jun 22, 2026 at 09:20:50PM +0530, Mukesh Ojha wrote:
> On Thu, Jun 18, 2026 at 03:53:22PM +0200, Konrad Dybcio wrote:
> > On 4/30/26 9:12 PM, Mukesh Ojha wrote:
> > > The firmware memory regions in qcom_adsp, qcom_pas and qcom_wcnss are
> > > mapped using devm_ioremap_wc() / devm_ioremap_resource_wc(), which
> > > return void __iomem *. However, the mem_region (and dtb_mem_region)
> > > fields in the respective driver structs were declared as plain void *,
> > > causing sparse to flag address space mismatches:
> > > 
> > > qcom_q6v5_adsp.c:639:26: warning: incorrect type in assignment (different address spaces)
> > > qcom_q6v5_adsp.c:639:26:    expected void *mem_region
> > > qcom_q6v5_adsp.c:639:26:    got void [noderef] __iomem *
> > > qcom_q6v5_pas.c:141:45: warning: incorrect type in argument 2 (different address spaces)
> > > qcom_q6v5_pas.c:141:45:    expected void const volatile [noderef] __iomem *src
> > > qcom_q6v5_pas.c:141:45:    got void *
> > > qcom_q6v5_pas.c:637:25: warning: incorrect type in assignment (different address spaces)
> > > qcom_q6v5_pas.c:637:25:    expected void *mem_region
> > > qcom_q6v5_pas.c:637:25:    got void [noderef] __iomem *
> > > qcom_q6v5_pas.c:654:29: warning: incorrect type in assignment (different address spaces)
> > > qcom_q6v5_pas.c:654:29:    expected void *dtb_mem_region
> > > qcom_q6v5_pas.c:654:29:    got void [noderef] __iomem *
> > > qcom_wcnss.c:540:27: warning: incorrect type in assignment (different address spaces)
> > > qcom_wcnss.c:540:27:    expected void *mem_region
> > > qcom_wcnss.c:540:27:    got void [noderef] __iomem *
> > > 
> > > Fix this by annotating the struct fields with __iomem to correctly
> > > reflect the address space of the underlying mapping.
> > > 
> > > These regions are subsequently passed to qcom_mdt_load(),
> > > qcom_mdt_load_no_init() and qcom_mdt_pas_load(), all of which take
> > > void * and use plain memcpy()/memset() internally to write firmware
> > > segments into the region. This is intentional and safe: the mappings
> > > are write-combining (WC), which on arm64 permits bulk CPU stores
> > > without requiring the memcpy_toio()/memset_io() accessors. Changing
> > > the MDT loader API to accept void __iomem * would be a more invasive
> > > change and would affect callers.
> > 
> > Zooming out a bit more, should we even be ioremapping these regions
> > in the first place? Are they not just RAM/Normal Memory in arm parlance?
> 
> Yes, ioremap may not be required. Don't know why it was kept in the
> initial implementation, as for most Qualcomm SoCs this is going to be
> normal RAM and not SRAM or something, while other implementations like
> qcom_q6v5_mss still use memremap with MEMREMAP_WC.
> 
> > 
> > Would switching to devm_memremap(, MEMREMAP_WC) (like we do in RMTFS
> > today) be an even better solution here?
> 
> This looks fine and I do not see any immediate impact; however, I would
> like to hear from Bjorn on this, if there is any history..

Bjorn,

I was looking for your view on this before I work on its
revision.

-- 
-Mukesh Ojha

Re: [PATCH v4 2/6] remoteproc: qcom: pas: Guard dtb metadata release with dtb_pas_id check

[Dmitry Baryshkov]

On Fri, May 01, 2026 at 12:42:49AM +0530, Mukesh Ojha wrote:
> All other call sites of qcom_scm_pas_metadata_release() for the DTB
> context are guarded by a check on pas->dtb_pas_id, but the call inside
> qcom_pas_load() was not. Fix this by moving the call to the guarded
> block.

Fixes: 29814986b82e ("remoteproc: qcom_q6v5_pas: add support for dtb co-firmware loading")


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 3/6] remoteproc: qcom: pas: Fix the PAS context creation placement

[Dmitry Baryshkov]

On Fri, May 01, 2026 at 12:42:50AM +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.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 32 ++++++++++++++----------------
>  1 file changed, 15 insertions(+), 17 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v4 4/6] remoteproc: qcom: pas: Map/unmap subsystem region before auth_and_reset

[Dmitry Baryshkov]

On Fri, May 01, 2026 at 12:42:51AM +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  | 45 ++++++++++++++++++++---------
>  drivers/soc/qcom/mdt_loader.c       | 18 +++++++++---
>  include/linux/soc/qcom/mdt_loader.h |  4 +--
>  3 files changed, 48 insertions(+), 19 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v4 5/6] remoteproc: qcom: pas: Drop unused dtb_mem_region field

[Dmitry Baryshkov]

On Fri, May 01, 2026 at 12:42:52AM +0530, Mukesh Ojha wrote:
> dtb_mem_region is no longer referenced after the ioremap was moved
> to respective places where mapping is required. Remove it from
> struct qcom_pas.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v4 6/6] firmware: qcom: scm: introduce keep_mdt_buf flag in PAS context

[Dmitry Baryshkov]

On Fri, May 01, 2026 at 12:42:53AM +0530, Mukesh Ojha wrote:
> The PAS image initialization path always retains the metadata buffer
> when a valid qcom_scm_pas_context is provided, even if the caller does
> not require it. This implicit behavior leads to unclear buffer ownership
> and forces new users of qcom_mdt_pas_load() to manually release
> metadata, which is error‑ prone and incorrect.
> 
> Add a keep_mdt_buf flag to struct qcom_scm_pas_context to make metadata
> retention explicit.  Metadata buffers are now freed by default and are
> only preserved when this flag is set. qcom_q6v5_pas enables this during
> probe for contexts that require retained metadata for subsequent PAS
> operations, while existing callers continue to work unchanged.

Would it make sense to make new API backwards-compatible, so that we
dont' need to touch other drivers at the same time? It probably means
adding ->free_mdt_buf flag rather than keep_mdt_buf.

> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c       | 21 ++++++++++++++++++---
>  drivers/remoteproc/qcom_q6v5_pas.c     |  2 ++
>  include/linux/firmware/qcom/qcom_scm.h |  1 +
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 6/6] firmware: qcom: scm: introduce keep_mdt_buf flag in PAS context

[Mukesh Ojha]

On Mon, Jul 06, 2026 at 09:13:22PM +0300, Dmitry Baryshkov wrote:
> On Fri, May 01, 2026 at 12:42:53AM +0530, Mukesh Ojha wrote:
> > The PAS image initialization path always retains the metadata buffer
> > when a valid qcom_scm_pas_context is provided, even if the caller does
> > not require it. This implicit behavior leads to unclear buffer ownership
> > and forces new users of qcom_mdt_pas_load() to manually release
> > metadata, which is error‑ prone and incorrect.
> > 
> > Add a keep_mdt_buf flag to struct qcom_scm_pas_context to make metadata
> > retention explicit.  Metadata buffers are now freed by default and are
> > only preserved when this flag is set. qcom_q6v5_pas enables this during
> > probe for contexts that require retained metadata for subsequent PAS
> > operations, while existing callers continue to work unchanged.
> 
> Would it make sense to make new API backwards-compatible, so that we
> dont' need to touch other drivers at the same time? It probably means
> adding ->free_mdt_buf flag rather than keep_mdt_buf.

Only remoteproc peripherals need to set the flag and do the
release call later, while the rest of the clients like Iris, IPA,
etc. do not need to set the flag, which is aligned with backward
compatibility for non-rproc clients where their metadata memory
gets freed in the same call where it was allocated.

free_mdt_buf does not look suitable, at least name-wise, for the
reason that now I need to add this change to set this flag for
all non-rproc clients, while with keep_mdt_buf, just rproc
drivers need to set this flag which will make this backword
compatible .

-- 
-Mukesh Ojha