From: Antonino Maniscalco <antomani103@gmail.com>
To: Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Abhinav Kumar <quic_abhinavk@quicinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
Marijn Suijten <marijn.suijten@somainline.org>,
David Airlie <airlied@gmail.com>,
Daniel Vetter <daniel@ffwll.ch>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
Jonathan Corbet <corbet@lwn.net>
Cc: linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org,
Antonino Maniscalco <antomani103@gmail.com>,
Neil Armstrong <neil.armstrong@linaro.org>
Subject: [PATCH v8 02/12] drm/msm/a6xx: Track current_ctx_seqno per ring
Date: Thu, 03 Oct 2024 18:12:51 +0200 [thread overview]
Message-ID: <20241003-preemption-a750-t-v8-2-5c6cb9f256e0@gmail.com> (raw)
In-Reply-To: <20241003-preemption-a750-t-v8-0-5c6cb9f256e0@gmail.com>
With preemption it is not enough to track the current_ctx_seqno globally
as execution might switch between rings.
This is especially problematic when current_ctx_seqno is used to
determine whether a page table switch is necessary as it might lead to
security bugs.
Track current context per ring.
Tested-by: Rob Clark <robdclark@gmail.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550-QRD
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8450-HDK
Signed-off-by: Antonino Maniscalco <antomani103@gmail.com>
---
drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 6 +++---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 ++++++----
drivers/gpu/drm/msm/msm_gpu.c | 2 +-
drivers/gpu/drm/msm/msm_gpu.h | 11 -----------
drivers/gpu/drm/msm/msm_ringbuffer.h | 10 ++++++++++
8 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
index 0dc255ddf5ceba87090f64d5cb9f078b61104063..379a3d346c300f3ccc9e9bd08ef2a32aa3e24ceb 100644
--- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
@@ -22,7 +22,7 @@ static void a2xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
break;
case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
/* ignore if there has not been a ctx switch: */
- if (gpu->cur_ctx_seqno == submit->queue->ctx->seqno)
+ if (ring->cur_ctx_seqno == submit->queue->ctx->seqno)
break;
fallthrough;
case MSM_SUBMIT_CMD_BUF:
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index 5273dc8498381ce09e878894f4eb56263900be39..945fe64f835cc6094f1880ea20f20584de74a464 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -40,7 +40,7 @@ static void a3xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
break;
case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
/* ignore if there has not been a ctx switch: */
- if (gpu->cur_ctx_seqno == submit->queue->ctx->seqno)
+ if (ring->cur_ctx_seqno == submit->queue->ctx->seqno)
break;
fallthrough;
case MSM_SUBMIT_CMD_BUF:
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index 8b4cdf95f4453bb76e7efb93d86080ef678c9f68..50c490b492f08a1a7ebfe33b2f206cafd91a84ba 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -34,7 +34,7 @@ static void a4xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
break;
case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
/* ignore if there has not been a ctx switch: */
- if (gpu->cur_ctx_seqno == submit->queue->ctx->seqno)
+ if (ring->cur_ctx_seqno == submit->queue->ctx->seqno)
break;
fallthrough;
case MSM_SUBMIT_CMD_BUF:
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index c0b5373e90d7139caa023aec6f272545456acb0a..80b441fe8e3a823c5bd1f74cd8c7bb418d0674fb 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -75,7 +75,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit
case MSM_SUBMIT_CMD_IB_TARGET_BUF:
break;
case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
- if (gpu->cur_ctx_seqno == submit->queue->ctx->seqno)
+ if (ring->cur_ctx_seqno == submit->queue->ctx->seqno)
break;
fallthrough;
case MSM_SUBMIT_CMD_BUF:
@@ -129,7 +129,7 @@ static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
unsigned int i, ibs = 0;
if (IS_ENABLED(CONFIG_DRM_MSM_GPU_SUDO) && submit->in_rb) {
- gpu->cur_ctx_seqno = 0;
+ ring->cur_ctx_seqno = 0;
a5xx_submit_in_rb(gpu, submit);
return;
}
@@ -164,7 +164,7 @@ static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
case MSM_SUBMIT_CMD_IB_TARGET_BUF:
break;
case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
- if (gpu->cur_ctx_seqno == submit->queue->ctx->seqno)
+ if (ring->cur_ctx_seqno == submit->queue->ctx->seqno)
break;
fallthrough;
case MSM_SUBMIT_CMD_BUF:
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 32a4faa93d7f072ea6b8d949f4dc9d2a58cec6b9..6e065500b64d6d95599d89c33e6703c92f210047 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -109,7 +109,7 @@ static void a6xx_set_pagetable(struct a6xx_gpu *a6xx_gpu,
u32 asid;
u64 memptr = rbmemptr(ring, ttbr0);
- if (ctx->seqno == a6xx_gpu->base.base.cur_ctx_seqno)
+ if (ctx->seqno == ring->cur_ctx_seqno)
return;
if (msm_iommu_pagetable_params(ctx->aspace->mmu, &ttbr, &asid))
@@ -219,7 +219,7 @@ static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
case MSM_SUBMIT_CMD_IB_TARGET_BUF:
break;
case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
- if (gpu->cur_ctx_seqno == submit->queue->ctx->seqno)
+ if (ring->cur_ctx_seqno == submit->queue->ctx->seqno)
break;
fallthrough;
case MSM_SUBMIT_CMD_BUF:
@@ -305,7 +305,7 @@ static void a7xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
case MSM_SUBMIT_CMD_IB_TARGET_BUF:
break;
case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
- if (gpu->cur_ctx_seqno == submit->queue->ctx->seqno)
+ if (ring->cur_ctx_seqno == submit->queue->ctx->seqno)
break;
fallthrough;
case MSM_SUBMIT_CMD_BUF:
@@ -843,6 +843,7 @@ static int hw_init(struct msm_gpu *gpu)
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
u64 gmem_range_min;
+ unsigned int i;
int ret;
if (!adreno_has_gmu_wrapper(adreno_gpu)) {
@@ -1138,7 +1139,8 @@ static int hw_init(struct msm_gpu *gpu)
/* Always come up on rb 0 */
a6xx_gpu->cur_ring = gpu->rb[0];
- gpu->cur_ctx_seqno = 0;
+ for (i = 0; i < gpu->nr_rings; i++)
+ gpu->rb[i]->cur_ctx_seqno = 0;
/* Enable the SQE_to start the CP engine */
gpu_write(gpu, REG_A6XX_CP_SQE_CNTL, 1);
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 3666b42b4ecd7f91b24302d7f229eeefdc3c39b7..c063b3896dc1c193e41b8fc380a91a9076376811 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -783,7 +783,7 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
mutex_unlock(&gpu->active_lock);
gpu->funcs->submit(gpu, submit);
- gpu->cur_ctx_seqno = submit->queue->ctx->seqno;
+ submit->ring->cur_ctx_seqno = submit->queue->ctx->seqno;
pm_runtime_put(&gpu->pdev->dev);
hangcheck_timer_reset(gpu);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 1f02bb9956be2720a2760646ccdf92f8bead7dd0..7cabc8480d7c5461ab8d8726fcc21690cbaf7366 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -193,17 +193,6 @@ struct msm_gpu {
*/
refcount_t sysprof_active;
- /**
- * cur_ctx_seqno:
- *
- * The ctx->seqno value of the last context to submit rendering,
- * and the one with current pgtables installed (for generations
- * that support per-context pgtables). Tracked by seqno rather
- * than pointer value to avoid dangling pointers, and cases where
- * a ctx can be freed and a new one created with the same address.
- */
- int cur_ctx_seqno;
-
/**
* lock:
*
diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h
index 40791b2ade46ef0e16e2a4088291a575d3be9e82..174f83137a49940ec80b1fbf548e214fa3c32784 100644
--- a/drivers/gpu/drm/msm/msm_ringbuffer.h
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.h
@@ -100,6 +100,16 @@ struct msm_ringbuffer {
* preemption. Can be aquired from irq context.
*/
spinlock_t preempt_lock;
+
+ /**
+ * cur_ctx_seqno:
+ *
+ * The ctx->seqno value of the last context to submit to this ring
+ * Tracked by seqno rather than pointer value to avoid dangling
+ * pointers, and cases where a ctx can be freed and a new one created
+ * with the same address.
+ */
+ int cur_ctx_seqno;
};
struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu *gpu, int id,
--
2.46.1
next prev parent reply other threads:[~2024-10-03 16:13 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-03 16:12 [PATCH v8 00/12] Preemption support for A7XX Antonino Maniscalco
2024-10-03 16:12 ` [PATCH v8 01/12] drm/msm: Fix bv_fence being used as bv_rptr Antonino Maniscalco
2024-10-03 16:12 ` Antonino Maniscalco [this message]
2024-10-03 16:12 ` [PATCH v8 03/12] drm/msm: Add a `preempt_record_size` field Antonino Maniscalco
2024-10-03 16:12 ` [PATCH v8 04/12] drm/msm: Add CONTEXT_SWITCH_CNTL bitfields Antonino Maniscalco
2024-10-03 16:12 ` [PATCH v8 05/12] drm/msm/a6xx: Add a pwrup_list field to a6xx_info Antonino Maniscalco
2024-10-03 16:12 ` [PATCH v8 06/12] drm/msm/a6xx: Implement preemption for a7xx targets Antonino Maniscalco
2024-10-03 16:12 ` [PATCH v8 07/12] drm/msm/a6xx: Sync relevant adreno_pm4.xml changes Antonino Maniscalco
2024-10-03 16:12 ` [PATCH v8 08/12] drm/msm/a6xx: Use posamble to reset counters on preemption Antonino Maniscalco
2024-10-03 16:12 ` [PATCH v8 09/12] drm/msm/a6xx: Add traces for preemption Antonino Maniscalco
2024-10-08 21:10 ` [v8,09/12] " Kees Bakker
2024-10-16 12:13 ` Antonino Maniscalco
2024-10-16 20:33 ` Rob Clark
2024-10-16 21:28 ` Antonino Maniscalco
2024-10-03 16:12 ` [PATCH v8 10/12] drm/msm/a6xx: Add a flag to allow preemption to submitqueue_create Antonino Maniscalco
2024-10-03 16:13 ` [PATCH v8 11/12] drm/msm/a6xx: Enable preemption for tested a7xx targets Antonino Maniscalco
2024-10-03 16:13 ` [PATCH v8 12/12] Documentation: document adreno preemption Antonino Maniscalco
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241003-preemption-a750-t-v8-2-5c6cb9f256e0@gmail.com \
--to=antomani103@gmail.com \
--cc=airlied@gmail.com \
--cc=corbet@lwn.net \
--cc=daniel@ffwll.ch \
--cc=dmitry.baryshkov@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=konrad.dybcio@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=marijn.suijten@somainline.org \
--cc=mripard@kernel.org \
--cc=neil.armstrong@linaro.org \
--cc=quic_abhinavk@quicinc.com \
--cc=robdclark@gmail.com \
--cc=sean@poorly.run \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).