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.

> +     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

Reply via email to