Re: [PATCH v2 1/2] remoteproc: qcom_q6v5_mss: Don't reassign mpss region on shutdown

[Jeffrey Hugo]

On Fri, Nov 8, 2019 at 5:40 PM Bjorn Andersson
<bjorn.andersson@xxxxxxxxxx> wrote:
>
> Trying to reclaim mpss memory while the mba is not running causes the
> system to crash on devices with security fuses blown, so leave it
> assigned to the remote on shutdown and recover it on a subsequent boot.
>
> Fixes: 6c5a9dc2481b ("remoteproc: qcom: Make secure world call for mem ownership switch")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>

Stuff still works on the laptop, and I don't hit the access violation
with the crash dump scenario on the mtp.

Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@xxxxxxxxx>
Tested-by: Jeffrey Hugo <jeffrey.l.hugo@xxxxxxxxx>

> ---
>
> Changes since v1:
> - Assign memory back to Linux in coredump case
>
>  drivers/remoteproc/qcom_q6v5_mss.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index de919f2e8b94..efab574b2e12 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -875,11 +875,6 @@ static void q6v5_mba_reclaim(struct q6v5 *qproc)
>                 writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>         }
>
> -       ret = q6v5_xfer_mem_ownership(qproc, &qproc->mpss_perm,
> -                                     false, qproc->mpss_phys,
> -                                     qproc->mpss_size);
> -       WARN_ON(ret);
> -
>         q6v5_reset_assert(qproc);
>
>         q6v5_clk_disable(qproc->dev, qproc->reset_clks,
> @@ -969,6 +964,10 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
>                         max_addr = ALIGN(phdr->p_paddr + phdr->p_memsz, SZ_4K);
>         }
>
> +       /* Try to reset ownership back to Linux */
> +       q6v5_xfer_mem_ownership(qproc, &qproc->mpss_perm, false,
> +                               qproc->mpss_phys, qproc->mpss_size);
> +
>         mpss_reloc = relocate ? min_addr : qproc->mpss_phys;
>         qproc->mpss_reloc = mpss_reloc;
>         /* Load firmware segments */
> @@ -1058,9 +1057,14 @@ static void qcom_q6v5_dump_segment(struct rproc *rproc,
>         void *ptr = rproc_da_to_va(rproc, segment->da, segment->size);
>
>         /* Unlock mba before copying segments */
> -       if (!qproc->dump_mba_loaded)
> +       if (!qproc->dump_mba_loaded) {
>                 ret = q6v5_mba_load(qproc);
>
> +               /* Try to reset ownership back to Linux */
> +               q6v5_xfer_mem_ownership(qproc, &qproc->mpss_perm, false,
> +                                       qproc->mpss_phys, qproc->mpss_size);
> +       }
> +
>         if (!ptr || ret)
>                 memset(dest, 0xff, segment->size);
>         else
> @@ -1111,10 +1115,6 @@ static int q6v5_start(struct rproc *rproc)
>         return 0;
>
>  reclaim_mpss:
> -       xfermemop_ret = q6v5_xfer_mem_ownership(qproc, &qproc->mpss_perm,
> -                                               false, qproc->mpss_phys,
> -                                               qproc->mpss_size);
> -       WARN_ON(xfermemop_ret);
>         q6v5_mba_reclaim(qproc);
>
>         return ret;
> --
> 2.23.0
>