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 <[email protected]>
> > ---
> > 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
