Linux-remoteproc Archive mirror
 help / color / mirror / Atom feed
* [PATCH] remoteproc: qcom_q6v5_mss: Re-order writes to the IMEM region
@ 2024-08-19  7:30 Sibi Sankar
  2024-08-19 23:40 ` Doug Anderson
  0 siblings, 1 reply; 3+ messages in thread
From: Sibi Sankar @ 2024-08-19  7:30 UTC (permalink / raw)
  To: andersson, mathieu.poirier, dianders
  Cc: linux-arm-msm, linux-remoteproc, linux-kernel, Sibi Sankar

Any write access to the IMEM region when the Q6 is setting up XPU
protection on it will result in a XPU violation. Fix this by ensuring
IMEM writes related to the MBA post-mortem logs happen before the Q6
is brought out of reset.

Fixes: 318130cc9362 ("remoteproc: qcom_q6v5_mss: Add MBA log extraction support")
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/remoteproc/qcom_q6v5_mss.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 2a42215ce8e0..32c3531b20c7 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -1162,6 +1162,9 @@ static int q6v5_mba_load(struct q6v5 *qproc)
 		goto disable_active_clks;
 	}
 
+	if (qproc->has_mba_logs)
+		qcom_pil_info_store("mba", qproc->mba_phys, MBA_LOG_SIZE);
+
 	writel(qproc->mba_phys, qproc->rmb_base + RMB_MBA_IMAGE_REG);
 	if (qproc->dp_size) {
 		writel(qproc->mba_phys + SZ_1M, qproc->rmb_base + RMB_PMI_CODE_START_REG);
@@ -1172,9 +1175,6 @@ static int q6v5_mba_load(struct q6v5 *qproc)
 	if (ret)
 		goto reclaim_mba;
 
-	if (qproc->has_mba_logs)
-		qcom_pil_info_store("mba", qproc->mba_phys, MBA_LOG_SIZE);
-
 	ret = q6v5_rmb_mba_wait(qproc, 0, 5000);
 	if (ret == -ETIMEDOUT) {
 		dev_err(qproc->dev, "MBA boot timed out\n");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] remoteproc: qcom_q6v5_mss: Re-order writes to the IMEM region
  2024-08-19  7:30 [PATCH] remoteproc: qcom_q6v5_mss: Re-order writes to the IMEM region Sibi Sankar
@ 2024-08-19 23:40 ` Doug Anderson
  2024-10-16 16:01   ` Doug Anderson
  0 siblings, 1 reply; 3+ messages in thread
From: Doug Anderson @ 2024-08-19 23:40 UTC (permalink / raw)
  To: Sibi Sankar
  Cc: andersson, mathieu.poirier, linux-arm-msm, linux-remoteproc,
	linux-kernel

Hi,

On Mon, Aug 19, 2024 at 12:30 AM Sibi Sankar <quic_sibis@quicinc.com> wrote:
>
> Any write access to the IMEM region when the Q6 is setting up XPU
> protection on it will result in a XPU violation. Fix this by ensuring
> IMEM writes related to the MBA post-mortem logs happen before the Q6
> is brought out of reset.
>
> Fixes: 318130cc9362 ("remoteproc: qcom_q6v5_mss: Add MBA log extraction support")
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
>  drivers/remoteproc/qcom_q6v5_mss.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

As discussed offlist, this isn't a perfect fix since writes to this
IMEM could happen by other drivers and those could still cause things
to go boom if they run in parallel with this driver. That being said:
* It seems like a more proper fix needs a coordinated effort between a
device's built-in firmware and the modem firmware. This is difficult /
near impossible to get done properly.
* Even if we do a more proper fix, making this change won't hurt.
* This change will immediately improve things by avoiding the XPU
violation in the most common case.

I've confirmed that the test case I had where things were going boom
is fixed. Thus:

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] remoteproc: qcom_q6v5_mss: Re-order writes to the IMEM region
  2024-08-19 23:40 ` Doug Anderson
@ 2024-10-16 16:01   ` Doug Anderson
  0 siblings, 0 replies; 3+ messages in thread
From: Doug Anderson @ 2024-10-16 16:01 UTC (permalink / raw)
  To: Sibi Sankar
  Cc: andersson, mathieu.poirier, linux-arm-msm, linux-remoteproc,
	linux-kernel

Hi,

On Mon, Aug 19, 2024 at 4:40 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Aug 19, 2024 at 12:30 AM Sibi Sankar <quic_sibis@quicinc.com> wrote:
> >
> > Any write access to the IMEM region when the Q6 is setting up XPU
> > protection on it will result in a XPU violation. Fix this by ensuring
> > IMEM writes related to the MBA post-mortem logs happen before the Q6
> > is brought out of reset.
> >
> > Fixes: 318130cc9362 ("remoteproc: qcom_q6v5_mss: Add MBA log extraction support")
> > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> > ---
> >  drivers/remoteproc/qcom_q6v5_mss.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
>
> As discussed offlist, this isn't a perfect fix since writes to this
> IMEM could happen by other drivers and those could still cause things
> to go boom if they run in parallel with this driver. That being said:
> * It seems like a more proper fix needs a coordinated effort between a
> device's built-in firmware and the modem firmware. This is difficult /
> near impossible to get done properly.
> * Even if we do a more proper fix, making this change won't hurt.
> * This change will immediately improve things by avoiding the XPU
> violation in the most common case.
>
> I've confirmed that the test case I had where things were going boom
> is fixed. Thus:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Tested-by: Douglas Anderson <dianders@chromium.org>

Just checking in to see if there's anything else needed for this patch
to land. Thanks! :-)

-Doug

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-10-16 16:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-19  7:30 [PATCH] remoteproc: qcom_q6v5_mss: Re-order writes to the IMEM region Sibi Sankar
2024-08-19 23:40 ` Doug Anderson
2024-10-16 16:01   ` Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).