[PATCH 1/4] drm/msm: Split the a5xx preemption record

[PATCH 1/4] drm/msm: Split the a5xx preemption record

[Jordan Crouse]

The main a5xx preemption record can be marked as privileged to
protect it from user access but the counters storage needs to be
remain unprivileged. Split the buffers and mark the critical memory
as privileged.

Cc: stable@vger.kernel.org
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a5xx_gpu.h     |  1 +
 drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 25 ++++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
index 54868d4e3958..1e5b1a15a70f 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
@@ -31,6 +31,7 @@ struct a5xx_gpu {
 	struct msm_ringbuffer *next_ring;
 
 	struct drm_gem_object *preempt_bo[MSM_GPU_MAX_RINGS];
+	struct drm_gem_object *preempt_counters_bo[MSM_GPU_MAX_RINGS];
 	struct a5xx_preempt_record *preempt[MSM_GPU_MAX_RINGS];
 	uint64_t preempt_iova[MSM_GPU_MAX_RINGS];
 
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
index 9cf9353a7ff1..9f3fe177b00e 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
@@ -226,19 +226,31 @@ static int preempt_init_ring(struct a5xx_gpu *a5xx_gpu,
 	struct adreno_gpu *adreno_gpu = &a5xx_gpu->base;
 	struct msm_gpu *gpu = &adreno_gpu->base;
 	struct a5xx_preempt_record *ptr;
-	struct drm_gem_object *bo = NULL;
-	u64 iova = 0;
+	void *counters;
+	struct drm_gem_object *bo = NULL, *counters_bo = NULL;
+	u64 iova = 0, counters_iova = 0;
 
 	ptr = msm_gem_kernel_new(gpu->dev,
 		A5XX_PREEMPT_RECORD_SIZE + A5XX_PREEMPT_COUNTER_SIZE,
-		MSM_BO_UNCACHED, gpu->aspace, &bo, &iova);
+		MSM_BO_UNCACHED | MSM_BO_MAP_PRIV, gpu->aspace, &bo, &iova);
 
 	if (IS_ERR(ptr))
 		return PTR_ERR(ptr);
 
+	/* The buffer to store counters needs to be unprivileged */
+	counters = msm_gem_kernel_new(gpu->dev,
+		A5XX_PREEMPT_COUNTER_SIZE,
+		MSM_BO_UNCACHED, gpu->aspace, &counters_bo, &counters_iova);
+	if (IS_ERR(counters)) {
+		msm_gem_kernel_put(bo, gpu->aspace, true);
+		return PTR_ERR(counters);
+	}
+
 	msm_gem_object_set_name(bo, "preempt");
+	msm_gem_object_set_name(counters_bo, "preempt_counters");
 
 	a5xx_gpu->preempt_bo[ring->id] = bo;
+	a5xx_gpu->preempt_counters_bo[ring->id] = counters_bo;
 	a5xx_gpu->preempt_iova[ring->id] = iova;
 	a5xx_gpu->preempt[ring->id] = ptr;
 
@@ -249,7 +261,7 @@ static int preempt_init_ring(struct a5xx_gpu *a5xx_gpu,
 	ptr->data = 0;
 	ptr->cntl = MSM_GPU_RB_CNTL_DEFAULT;
 	ptr->rptr_addr = rbmemptr(ring, rptr);
-	ptr->counter = iova + A5XX_PREEMPT_RECORD_SIZE;
+	ptr->counter = counters_iova;
 
 	return 0;
 }
@@ -260,8 +272,11 @@ void a5xx_preempt_fini(struct msm_gpu *gpu)
 	struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu);
 	int i;
 
-	for (i = 0; i < gpu->nr_rings; i++)
+	for (i = 0; i < gpu->nr_rings; i++) {
 		msm_gem_kernel_put(a5xx_gpu->preempt_bo[i], gpu->aspace, true);
+		msm_gem_kernel_put(a5xx_gpu->preempt_counters_bo[i],
+			gpu->aspace, true);
+	}
 }
 
 void a5xx_preempt_init(struct msm_gpu *gpu)
-- 
2.25.1

Re: [PATCH 1/4] drm/msm: Split the a5xx preemption record

[Sasha Levin]

Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.8.6, v5.4.62, v4.19.143, v4.14.196, v4.9.235, v4.4.235.

v5.8.6: Build OK!
v5.4.62: Build failed! Errors:
    drivers/gpu/drm/msm/adreno/a5xx_preempt.c:235:21: error: 'MSM_BO_MAP_PRIV' undeclared (first use in this function); did you mean 'MSM_BO_CACHED'?

v4.19.143: Failed to apply! Possible dependencies:
    0815d7749a68 ("drm/msm: Add a name field for gem objects")
    1e29dff00400 ("drm/msm: Add a common function to free kernel buffer objects")
    64686886bbff ("drm/msm: Replace drm_gem_object_{un/reference} with put, get functions")
    6a41da17e87d ("drm: msm: Use DRM_DEV_* instead of dev_*")
    c97ea6a61b5e ("drm: msm: adreno: Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) +PTR_ERR")
    df0dff132905 ("drm/msm/a6xx: Poll for HFI responses")
    f384d7d514d1 ("drm: Convert to using %pOFn instead of device_node.name")
    feb085ec8a3d ("drm/msm: dsi: Return errors whan dt parsing fails")

v4.14.196: Failed to apply! Possible dependencies:
    b1fc2839d2f9 ("drm/msm: Implement preemption for A5XX targets")
    cd414f3d9316 ("drm/msm: Move memptrs to msm_gpu")
    e8f3de96a9d3 ("drm/msm/adreno: split out helper to load fw")
    eec874ce5ff1 ("drm/msm/adreno: load gpu at probe/bind time")
    f7de15450e90 ("drm/msm: Add per-instance submit queues")
    f97decac5f4c ("drm/msm: Support multiple ringbuffers")

v4.9.235: Failed to apply! Possible dependencies:
    1cec20f0ea0e ("dma-buf: Restart reservation_object_wait_timeout_rcu() after writes")
    667ce33e57d0 ("drm/msm: support multiple address spaces")
    78010cd9736e ("dma-buf/fence: add an lockdep_assert_held()")
    89d777a57245 ("drm/msm: Remove 'src_clk' from adreno configuration")
    b1fc2839d2f9 ("drm/msm: Implement preemption for A5XX targets")
    b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
    f54d1867005c ("dma-buf: Rename struct fence to dma_fence")
    fb0399819239 ("drm/msm: Add adreno_gpu_write64()")
    fedf54132d24 ("dma-buf: Restart reservation_object_get_fences_rcu() after writes")

v4.4.235: Failed to apply! Possible dependencies:
    01c8f1c44b83 ("mm, dax, gpu: convert vm_insert_mixed to pfn_t")
    0caeef63e6d2 ("libnvdimm: Add a poison list and export badblocks")
    0e749e54244e ("dax: increase granularity of dax_clear_blocks() operations")
    34c0fd540e79 ("mm, dax, pmem: introduce pfn_t")
    357ff00b08d6 ("drm/msm/adreno: support for adreno 430.")
    52db400fcd50 ("pmem, dax: clean up clear_pmem()")
    667ce33e57d0 ("drm/msm: support multiple address spaces")
    68209390f116 ("drm/msm: shrinker support")
    7d0c5ee9f077 ("drm/msm/adreno: get CP_RPTR from register instead of shadow memory")
    87ba05dff351 ("libnvdimm: don't fail init for full badblocks list")
    89d777a57245 ("drm/msm: Remove 'src_clk' from adreno configuration")
    a5725ab0497a ("drm/msm/adreno: move function declarations to header file")
    b1fc2839d2f9 ("drm/msm: Implement preemption for A5XX targets")
    b2e0d1625e19 ("dax: fix lifetime of in-kernel dax mappings with dax_map_atomic()")
    b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
    b95f5f4391fa ("libnvdimm: convert to statically allocated badblocks")
    edcd60ce243d ("drm/msm: move debugfs code to it's own file")
    fb0399819239 ("drm/msm: Add adreno_gpu_write64()")
    fde5de6cb461 ("drm/msm: move fence code to it's own file")
    fe683adabfe6 ("dax: guarantee page aligned results from bdev_direct_access()")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

Re: [PATCH 1/4] drm/msm: Split the a5xx preemption record

[Jordan Crouse]

On Sun, Sep 06, 2020 at 03:16:10AM +0000, Sasha Levin wrote:
> Hi
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v5.8.6, v5.4.62, v4.19.143, v4.14.196, v4.9.235, v4.4.235.
> 
> v5.8.6: Build OK!
> v5.4.62: Build failed! Errors:
>     drivers/gpu/drm/msm/adreno/a5xx_preempt.c:235:21: error: 'MSM_BO_MAP_PRIV' undeclared (first use in this function); did you mean 'MSM_BO_CACHED'?
> 
> v4.19.143: Failed to apply! Possible dependencies:
>     0815d7749a68 ("drm/msm: Add a name field for gem objects")
>     1e29dff00400 ("drm/msm: Add a common function to free kernel buffer objects")
>     64686886bbff ("drm/msm: Replace drm_gem_object_{un/reference} with put, get functions")
>     6a41da17e87d ("drm: msm: Use DRM_DEV_* instead of dev_*")
>     c97ea6a61b5e ("drm: msm: adreno: Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) +PTR_ERR")
>     df0dff132905 ("drm/msm/a6xx: Poll for HFI responses")
>     f384d7d514d1 ("drm: Convert to using %pOFn instead of device_node.name")
>     feb085ec8a3d ("drm/msm: dsi: Return errors whan dt parsing fails")
> 
> v4.14.196: Failed to apply! Possible dependencies:
>     b1fc2839d2f9 ("drm/msm: Implement preemption for A5XX targets")
>     cd414f3d9316 ("drm/msm: Move memptrs to msm_gpu")
>     e8f3de96a9d3 ("drm/msm/adreno: split out helper to load fw")
>     eec874ce5ff1 ("drm/msm/adreno: load gpu at probe/bind time")
>     f7de15450e90 ("drm/msm: Add per-instance submit queues")
>     f97decac5f4c ("drm/msm: Support multiple ringbuffers")
> 
> v4.9.235: Failed to apply! Possible dependencies:
>     1cec20f0ea0e ("dma-buf: Restart reservation_object_wait_timeout_rcu() after writes")
>     667ce33e57d0 ("drm/msm: support multiple address spaces")
>     78010cd9736e ("dma-buf/fence: add an lockdep_assert_held()")
>     89d777a57245 ("drm/msm: Remove 'src_clk' from adreno configuration")
>     b1fc2839d2f9 ("drm/msm: Implement preemption for A5XX targets")
>     b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
>     f54d1867005c ("dma-buf: Rename struct fence to dma_fence")
>     fb0399819239 ("drm/msm: Add adreno_gpu_write64()")
>     fedf54132d24 ("dma-buf: Restart reservation_object_get_fences_rcu() after writes")
> 
> v4.4.235: Failed to apply! Possible dependencies:
>     01c8f1c44b83 ("mm, dax, gpu: convert vm_insert_mixed to pfn_t")
>     0caeef63e6d2 ("libnvdimm: Add a poison list and export badblocks")
>     0e749e54244e ("dax: increase granularity of dax_clear_blocks() operations")
>     34c0fd540e79 ("mm, dax, pmem: introduce pfn_t")
>     357ff00b08d6 ("drm/msm/adreno: support for adreno 430.")
>     52db400fcd50 ("pmem, dax: clean up clear_pmem()")
>     667ce33e57d0 ("drm/msm: support multiple address spaces")
>     68209390f116 ("drm/msm: shrinker support")
>     7d0c5ee9f077 ("drm/msm/adreno: get CP_RPTR from register instead of shadow memory")
>     87ba05dff351 ("libnvdimm: don't fail init for full badblocks list")
>     89d777a57245 ("drm/msm: Remove 'src_clk' from adreno configuration")
>     a5725ab0497a ("drm/msm/adreno: move function declarations to header file")
>     b1fc2839d2f9 ("drm/msm: Implement preemption for A5XX targets")
>     b2e0d1625e19 ("dax: fix lifetime of in-kernel dax mappings with dax_map_atomic()")
>     b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
>     b95f5f4391fa ("libnvdimm: convert to statically allocated badblocks")
>     edcd60ce243d ("drm/msm: move debugfs code to it's own file")
>     fb0399819239 ("drm/msm: Add adreno_gpu_write64()")
>     fde5de6cb461 ("drm/msm: move fence code to it's own file")
>     fe683adabfe6 ("dax: guarantee page aligned results from bdev_direct_access()")
> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?

For 5.4 it looks like all you need is:

0b462d7a71c07e96b ("drm/msm: add internal MSM_BO_MAP_PRIV flag")

For 4.19 it looks a bit more complicated. I can try to build a stack that would
work.

This patch isn't needed before 4.19 because preemption isn't available (as far
as I can tell from the dependencies list).

Jordan
> 
> -- 
> Thanks
> Sasha

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project