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 <[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);
> > 
> > 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

Reply via email to