* [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 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 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
* 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 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
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®