* [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
@ 2020-02-13 0:54 Luben Tuikov
2020-02-13 10:30 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Luben Tuikov @ 2020-02-13 0:54 UTC (permalink / raw)
To: amd-gfx; +Cc: Luben Tuikov
Move from a per-CS secure flag (TMZ) to a per-IB
secure flag.
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ++++++++++++++++++++---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 ++++-----
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++++++----------------
drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 20 ++++++--------------
include/uapi/drm/amdgpu_drm.h | 7 ++++---
10 files changed, 44 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 80ba6dfc54e2..f9fa6e104fef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
if (ret)
goto free_all_kdata;
- p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
-
if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
ret = -ECANCELED;
goto free_all_kdata;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 6e0f97afb030..cae81914c821 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
uint64_t fence_ctx;
uint32_t status = 0, alloc_size;
unsigned fence_flags = 0;
+ bool secure;
unsigned i;
int r = 0;
@@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
if (job && ring->funcs->emit_cntxcntl) {
status |= job->preamble_status;
status |= job->preemption_status;
- amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
+ amdgpu_ring_emit_cntxcntl(ring, status);
}
+ secure = false;
for (i = 0; i < num_ibs; ++i) {
ib = &ibs[i];
@@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
continue;
+ /* If this IB is TMZ, add frame TMZ start packet,
+ * else, turn off TMZ.
+ */
+ if (ib->flags & AMDGPU_IB_FLAGS_SECURE && ring->funcs->emit_tmz) {
+ if (!secure) {
+ secure = true;
+ amdgpu_ring_emit_tmz(ring, true);
+ }
+ } else if (secure) {
+ secure = false;
+ amdgpu_ring_emit_tmz(ring, false);
+ }
+
amdgpu_ring_emit_ib(ring, job, ib, status);
status &= ~AMDGPU_HAVE_CTX_SWITCH;
}
- if (ring->funcs->emit_tmz)
- amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
+ if (secure) {
+ secure = false;
+ amdgpu_ring_emit_tmz(ring, false);
+ }
#ifdef CONFIG_X86_64
if (!(adev->flags & AMD_IS_APU))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 31c4444b0203..2e2110dddb76 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -61,9 +61,6 @@ struct amdgpu_job {
/* user fence handling */
uint64_t uf_addr;
uint64_t uf_sequence;
-
- /* the job is due to a secure command submission */
- bool secure;
};
int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 5134d0dd6dc2..930316e60155 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
void (*begin_use)(struct amdgpu_ring *ring);
void (*end_use)(struct amdgpu_ring *ring);
void (*emit_switch_buffer) (struct amdgpu_ring *ring);
- void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
- bool trusted);
+ void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
@@ -167,7 +166,7 @@ struct amdgpu_ring_funcs {
void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
uint32_t reg0, uint32_t reg1,
uint32_t ref, uint32_t mask);
- void (*emit_tmz)(struct amdgpu_ring *ring, bool start, bool trusted);
+ void (*emit_tmz)(struct amdgpu_ring *ring, bool start);
/* priority functions */
void (*set_priority) (struct amdgpu_ring *ring,
enum drm_sched_priority priority);
@@ -243,12 +242,12 @@ struct amdgpu_ring {
#define amdgpu_ring_emit_gds_switch(r, v, db, ds, wb, ws, ab, as) (r)->funcs->emit_gds_switch((r), (v), (db), (ds), (wb), (ws), (ab), (as))
#define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
#define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
-#define amdgpu_ring_emit_cntxcntl(r, d, s) (r)->funcs->emit_cntxcntl((r), (d), (s))
+#define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
#define amdgpu_ring_emit_rreg(r, d) (r)->funcs->emit_rreg((r), (d))
#define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
#define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
#define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) (r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
-#define amdgpu_ring_emit_tmz(r, b, s) (r)->funcs->emit_tmz((r), (b), (s))
+#define amdgpu_ring_emit_tmz(r, b) (r)->funcs->emit_tmz((r), (b))
#define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
#define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
#define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 4e25b39ac14f..1a9787a0177e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -256,8 +256,7 @@ static int gfx_v10_0_rlc_backdoor_autoload_enable(struct amdgpu_device *adev);
static int gfx_v10_0_wait_for_rlc_autoload_complete(struct amdgpu_device *adev);
static void gfx_v10_0_ring_emit_ce_meta(struct amdgpu_ring *ring, bool resume);
static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume);
-static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
- bool trusted);
+static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start);
static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)
{
@@ -4564,8 +4563,7 @@ static void gfx_v10_0_ring_emit_sb(struct amdgpu_ring *ring)
}
static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
- uint32_t flags,
- bool trusted)
+ uint32_t flags)
{
uint32_t dw2 = 0;
@@ -4573,8 +4571,6 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
gfx_v10_0_ring_emit_ce_meta(ring,
flags & AMDGPU_IB_PREEMPTED ? true : false);
- gfx_v10_0_ring_emit_tmz(ring, true, trusted);
-
dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
if (flags & AMDGPU_HAVE_CTX_SWITCH) {
/* set load_global_config & load_global_uconfig */
@@ -4731,17 +4727,12 @@ static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume)
sizeof(de_payload) >> 2);
}
-static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
- bool trusted)
+static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
{
- amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
- /*
- * cmd = 0: frame begin
- * cmd = 1: frame end
- */
- amdgpu_ring_write(ring,
- ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
- | FRAME_CMD(start ? 0 : 1));
+ if (amdgpu_is_tmz(ring->adev)) {
+ amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
+ amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
+ }
}
static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 95bb2422b27c..31f44d05e606 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -2973,8 +2973,7 @@ static uint64_t gfx_v6_0_get_gpu_clock_counter(struct amdgpu_device *adev)
return clock;
}
-static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
- bool trusted)
+static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
{
if (flags & AMDGPU_HAVE_CTX_SWITCH)
gfx_v6_0_ring_emit_vgt_flush(ring);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 648d767d14e7..8f20a5dd44fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2320,8 +2320,7 @@ static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
amdgpu_ring_write(ring, control);
}
-static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
- bool trusted)
+static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
{
uint32_t dw2 = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index a46ec1c9846e..fa245973de12 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6386,8 +6386,7 @@ static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
amdgpu_ring_write(ring, 0);
}
-static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
- bool trusted)
+static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
{
uint32_t dw2 = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index e5753746cbf9..9d3805488832 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -5212,29 +5212,21 @@ static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring)
amdgpu_ring_write_multiple(ring, (void *)&de_payload, sizeof(de_payload) >> 2);
}
-static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
- bool trusted)
+static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
{
- amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
- /*
- * cmd = 0: frame begin
- * cmd = 1: frame end
- */
- amdgpu_ring_write(ring,
- ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
- | FRAME_CMD(start ? 0 : 1));
+ if (amdgpu_is_tmz(ring->adev)) {
+ amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
+ amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
+ }
}
-static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
- bool trusted)
+static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
{
uint32_t dw2 = 0;
if (amdgpu_sriov_vf(ring->adev))
gfx_v9_0_ring_emit_ce_meta(ring);
- gfx_v9_0_ring_emit_tmz(ring, true, trusted);
-
dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
if (flags & AMDGPU_HAVE_CTX_SWITCH) {
/* set load_global_config & load_global_uconfig */
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 918ac3548cd3..eaf94a421901 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -556,9 +556,6 @@ struct drm_amdgpu_cs_chunk {
__u64 chunk_data;
};
-/* Flag the command submission as secure */
-#define AMDGPU_CS_FLAGS_SECURE (1 << 0)
-
struct drm_amdgpu_cs_in {
/** Rendering context id */
__u32 ctx_id;
@@ -599,6 +596,10 @@ union drm_amdgpu_cs {
*/
#define AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID (1 << 4)
+/* Flag the IB as secure (TMZ)
+ */
+#define AMDGPU_IB_FLAGS_SECURE (1 << 5)
+
struct drm_amdgpu_cs_chunk_ib {
__u32 _pad;
/** AMDGPU_IB_FLAG_* */
--
2.25.0.232.gd8437c57fa
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
2020-02-13 0:54 [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ) Luben Tuikov
@ 2020-02-13 10:30 ` Christian König
2020-02-13 14:57 ` Huang Rui
2020-02-13 22:00 ` Luben Tuikov
0 siblings, 2 replies; 6+ messages in thread
From: Christian König @ 2020-02-13 10:30 UTC (permalink / raw)
To: Luben Tuikov, amd-gfx
Am 13.02.20 um 01:54 schrieb Luben Tuikov:
> Move from a per-CS secure flag (TMZ) to a per-IB
> secure flag.
Well that seems to make a lot of sense to me, but need to bit of time to
read into the PM4 handling of TMZ.
Especially what is the "start" parameter good for?
Regards,
Christian.
>
> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ++++++++++++++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 ++++-----
> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++++++----------------
> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 3 +--
> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 +--
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 +--
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 20 ++++++--------------
> include/uapi/drm/amdgpu_drm.h | 7 ++++---
> 10 files changed, 44 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 80ba6dfc54e2..f9fa6e104fef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
> if (ret)
> goto free_all_kdata;
>
> - p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
> -
> if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
> ret = -ECANCELED;
> goto free_all_kdata;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 6e0f97afb030..cae81914c821 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
> uint64_t fence_ctx;
> uint32_t status = 0, alloc_size;
> unsigned fence_flags = 0;
> + bool secure;
>
> unsigned i;
> int r = 0;
> @@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
> if (job && ring->funcs->emit_cntxcntl) {
> status |= job->preamble_status;
> status |= job->preemption_status;
> - amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
> + amdgpu_ring_emit_cntxcntl(ring, status);
> }
>
> + secure = false;
> for (i = 0; i < num_ibs; ++i) {
> ib = &ibs[i];
>
> @@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
> continue;
>
> + /* If this IB is TMZ, add frame TMZ start packet,
> + * else, turn off TMZ.
> + */
> + if (ib->flags & AMDGPU_IB_FLAGS_SECURE && ring->funcs->emit_tmz) {
> + if (!secure) {
> + secure = true;
> + amdgpu_ring_emit_tmz(ring, true);
> + }
> + } else if (secure) {
> + secure = false;
> + amdgpu_ring_emit_tmz(ring, false);
> + }
> +
> amdgpu_ring_emit_ib(ring, job, ib, status);
> status &= ~AMDGPU_HAVE_CTX_SWITCH;
> }
>
> - if (ring->funcs->emit_tmz)
> - amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
> + if (secure) {
> + secure = false;
> + amdgpu_ring_emit_tmz(ring, false);
> + }
>
> #ifdef CONFIG_X86_64
> if (!(adev->flags & AMD_IS_APU))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index 31c4444b0203..2e2110dddb76 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -61,9 +61,6 @@ struct amdgpu_job {
> /* user fence handling */
> uint64_t uf_addr;
> uint64_t uf_sequence;
> -
> - /* the job is due to a secure command submission */
> - bool secure;
> };
>
> int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 5134d0dd6dc2..930316e60155 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
> void (*begin_use)(struct amdgpu_ring *ring);
> void (*end_use)(struct amdgpu_ring *ring);
> void (*emit_switch_buffer) (struct amdgpu_ring *ring);
> - void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
> - bool trusted);
> + void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
> void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
> void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
> void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
> @@ -167,7 +166,7 @@ struct amdgpu_ring_funcs {
> void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
> uint32_t reg0, uint32_t reg1,
> uint32_t ref, uint32_t mask);
> - void (*emit_tmz)(struct amdgpu_ring *ring, bool start, bool trusted);
> + void (*emit_tmz)(struct amdgpu_ring *ring, bool start);
> /* priority functions */
> void (*set_priority) (struct amdgpu_ring *ring,
> enum drm_sched_priority priority);
> @@ -243,12 +242,12 @@ struct amdgpu_ring {
> #define amdgpu_ring_emit_gds_switch(r, v, db, ds, wb, ws, ab, as) (r)->funcs->emit_gds_switch((r), (v), (db), (ds), (wb), (ws), (ab), (as))
> #define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
> #define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
> -#define amdgpu_ring_emit_cntxcntl(r, d, s) (r)->funcs->emit_cntxcntl((r), (d), (s))
> +#define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
> #define amdgpu_ring_emit_rreg(r, d) (r)->funcs->emit_rreg((r), (d))
> #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
> #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
> #define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) (r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
> -#define amdgpu_ring_emit_tmz(r, b, s) (r)->funcs->emit_tmz((r), (b), (s))
> +#define amdgpu_ring_emit_tmz(r, b) (r)->funcs->emit_tmz((r), (b))
> #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
> #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
> #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 4e25b39ac14f..1a9787a0177e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -256,8 +256,7 @@ static int gfx_v10_0_rlc_backdoor_autoload_enable(struct amdgpu_device *adev);
> static int gfx_v10_0_wait_for_rlc_autoload_complete(struct amdgpu_device *adev);
> static void gfx_v10_0_ring_emit_ce_meta(struct amdgpu_ring *ring, bool resume);
> static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume);
> -static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
> - bool trusted);
> +static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start);
>
> static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)
> {
> @@ -4564,8 +4563,7 @@ static void gfx_v10_0_ring_emit_sb(struct amdgpu_ring *ring)
> }
>
> static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
> - uint32_t flags,
> - bool trusted)
> + uint32_t flags)
> {
> uint32_t dw2 = 0;
>
> @@ -4573,8 +4571,6 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
> gfx_v10_0_ring_emit_ce_meta(ring,
> flags & AMDGPU_IB_PREEMPTED ? true : false);
>
> - gfx_v10_0_ring_emit_tmz(ring, true, trusted);
> -
> dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
> if (flags & AMDGPU_HAVE_CTX_SWITCH) {
> /* set load_global_config & load_global_uconfig */
> @@ -4731,17 +4727,12 @@ static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume)
> sizeof(de_payload) >> 2);
> }
>
> -static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
> - bool trusted)
> +static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
> {
> - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
> - /*
> - * cmd = 0: frame begin
> - * cmd = 1: frame end
> - */
> - amdgpu_ring_write(ring,
> - ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
> - | FRAME_CMD(start ? 0 : 1));
> + if (amdgpu_is_tmz(ring->adev)) {
> + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
> + amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
> + }
> }
>
> static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index 95bb2422b27c..31f44d05e606 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -2973,8 +2973,7 @@ static uint64_t gfx_v6_0_get_gpu_clock_counter(struct amdgpu_device *adev)
> return clock;
> }
>
> -static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
> - bool trusted)
> +static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
> {
> if (flags & AMDGPU_HAVE_CTX_SWITCH)
> gfx_v6_0_ring_emit_vgt_flush(ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index 648d767d14e7..8f20a5dd44fe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2320,8 +2320,7 @@ static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> amdgpu_ring_write(ring, control);
> }
>
> -static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
> - bool trusted)
> +static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
> {
> uint32_t dw2 = 0;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index a46ec1c9846e..fa245973de12 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6386,8 +6386,7 @@ static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
> amdgpu_ring_write(ring, 0);
> }
>
> -static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
> - bool trusted)
> +static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
> {
> uint32_t dw2 = 0;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index e5753746cbf9..9d3805488832 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -5212,29 +5212,21 @@ static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring)
> amdgpu_ring_write_multiple(ring, (void *)&de_payload, sizeof(de_payload) >> 2);
> }
>
> -static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
> - bool trusted)
> +static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
> {
> - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
> - /*
> - * cmd = 0: frame begin
> - * cmd = 1: frame end
> - */
> - amdgpu_ring_write(ring,
> - ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
> - | FRAME_CMD(start ? 0 : 1));
> + if (amdgpu_is_tmz(ring->adev)) {
> + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
> + amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
> + }
> }
>
> -static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
> - bool trusted)
> +static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
> {
> uint32_t dw2 = 0;
>
> if (amdgpu_sriov_vf(ring->adev))
> gfx_v9_0_ring_emit_ce_meta(ring);
>
> - gfx_v9_0_ring_emit_tmz(ring, true, trusted);
> -
> dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
> if (flags & AMDGPU_HAVE_CTX_SWITCH) {
> /* set load_global_config & load_global_uconfig */
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 918ac3548cd3..eaf94a421901 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -556,9 +556,6 @@ struct drm_amdgpu_cs_chunk {
> __u64 chunk_data;
> };
>
> -/* Flag the command submission as secure */
> -#define AMDGPU_CS_FLAGS_SECURE (1 << 0)
> -
> struct drm_amdgpu_cs_in {
> /** Rendering context id */
> __u32 ctx_id;
> @@ -599,6 +596,10 @@ union drm_amdgpu_cs {
> */
> #define AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID (1 << 4)
>
> +/* Flag the IB as secure (TMZ)
> + */
> +#define AMDGPU_IB_FLAGS_SECURE (1 << 5)
> +
> struct drm_amdgpu_cs_chunk_ib {
> __u32 _pad;
> /** AMDGPU_IB_FLAG_* */
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
2020-02-13 10:30 ` Christian König
@ 2020-02-13 14:57 ` Huang Rui
2020-02-13 22:03 ` Luben Tuikov
2020-02-13 22:00 ` Luben Tuikov
1 sibling, 1 reply; 6+ messages in thread
From: Huang Rui @ 2020-02-13 14:57 UTC (permalink / raw)
To: christian.koenig; +Cc: Luben Tuikov, amd-gfx
On Thu, Feb 13, 2020 at 11:30:43AM +0100, Christian König wrote:
> Am 13.02.20 um 01:54 schrieb Luben Tuikov:
> > Move from a per-CS secure flag (TMZ) to a per-IB
> > secure flag.
>
> Well that seems to make a lot of sense to me, but need to bit of time to
> read into the PM4 handling of TMZ.
>
> Especially what is the "start" parameter good for?
I see this patch just now. And yes, that's my purpose for the discussion.
Thanks!
Reviewed-by: Huang Rui <ray.huang@amd.com>
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ++++++++++++++++++++---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 ++++-----
> > drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++++++----------------
> > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 3 +--
> > drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 +--
> > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 +--
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 20 ++++++--------------
> > include/uapi/drm/amdgpu_drm.h | 7 ++++---
> > 10 files changed, 44 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 80ba6dfc54e2..f9fa6e104fef 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
> > if (ret)
> > goto free_all_kdata;
> > - p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
> > -
> > if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
> > ret = -ECANCELED;
> > goto free_all_kdata;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > index 6e0f97afb030..cae81914c821 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > @@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
> > uint64_t fence_ctx;
> > uint32_t status = 0, alloc_size;
> > unsigned fence_flags = 0;
> > + bool secure;
> > unsigned i;
> > int r = 0;
> > @@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
> > if (job && ring->funcs->emit_cntxcntl) {
> > status |= job->preamble_status;
> > status |= job->preemption_status;
> > - amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
> > + amdgpu_ring_emit_cntxcntl(ring, status);
> > }
> > + secure = false;
> > for (i = 0; i < num_ibs; ++i) {
> > ib = &ibs[i];
> > @@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
> > !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
> > continue;
> > + /* If this IB is TMZ, add frame TMZ start packet,
> > + * else, turn off TMZ.
> > + */
> > + if (ib->flags & AMDGPU_IB_FLAGS_SECURE && ring->funcs->emit_tmz) {
> > + if (!secure) {
> > + secure = true;
> > + amdgpu_ring_emit_tmz(ring, true);
> > + }
> > + } else if (secure) {
> > + secure = false;
> > + amdgpu_ring_emit_tmz(ring, false);
> > + }
> > +
> > amdgpu_ring_emit_ib(ring, job, ib, status);
> > status &= ~AMDGPU_HAVE_CTX_SWITCH;
> > }
> > - if (ring->funcs->emit_tmz)
> > - amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
> > + if (secure) {
> > + secure = false;
> > + amdgpu_ring_emit_tmz(ring, false);
> > + }
> > #ifdef CONFIG_X86_64
> > if (!(adev->flags & AMD_IS_APU))
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > index 31c4444b0203..2e2110dddb76 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > @@ -61,9 +61,6 @@ struct amdgpu_job {
> > /* user fence handling */
> > uint64_t uf_addr;
> > uint64_t uf_sequence;
> > -
> > - /* the job is due to a secure command submission */
> > - bool secure;
> > };
> > int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > index 5134d0dd6dc2..930316e60155 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
> > void (*begin_use)(struct amdgpu_ring *ring);
> > void (*end_use)(struct amdgpu_ring *ring);
> > void (*emit_switch_buffer) (struct amdgpu_ring *ring);
> > - void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
> > - bool trusted);
> > + void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
> > void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
> > void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
> > void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
> > @@ -167,7 +166,7 @@ struct amdgpu_ring_funcs {
> > void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
> > uint32_t reg0, uint32_t reg1,
> > uint32_t ref, uint32_t mask);
> > - void (*emit_tmz)(struct amdgpu_ring *ring, bool start, bool trusted);
> > + void (*emit_tmz)(struct amdgpu_ring *ring, bool start);
> > /* priority functions */
> > void (*set_priority) (struct amdgpu_ring *ring,
> > enum drm_sched_priority priority);
> > @@ -243,12 +242,12 @@ struct amdgpu_ring {
> > #define amdgpu_ring_emit_gds_switch(r, v, db, ds, wb, ws, ab, as) (r)->funcs->emit_gds_switch((r), (v), (db), (ds), (wb), (ws), (ab), (as))
> > #define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
> > #define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
> > -#define amdgpu_ring_emit_cntxcntl(r, d, s) (r)->funcs->emit_cntxcntl((r), (d), (s))
> > +#define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
> > #define amdgpu_ring_emit_rreg(r, d) (r)->funcs->emit_rreg((r), (d))
> > #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
> > #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
> > #define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) (r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
> > -#define amdgpu_ring_emit_tmz(r, b, s) (r)->funcs->emit_tmz((r), (b), (s))
> > +#define amdgpu_ring_emit_tmz(r, b) (r)->funcs->emit_tmz((r), (b))
> > #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
> > #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
> > #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > index 4e25b39ac14f..1a9787a0177e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> > @@ -256,8 +256,7 @@ static int gfx_v10_0_rlc_backdoor_autoload_enable(struct amdgpu_device *adev);
> > static int gfx_v10_0_wait_for_rlc_autoload_complete(struct amdgpu_device *adev);
> > static void gfx_v10_0_ring_emit_ce_meta(struct amdgpu_ring *ring, bool resume);
> > static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume);
> > -static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
> > - bool trusted);
> > +static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start);
> > static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)
> > {
> > @@ -4564,8 +4563,7 @@ static void gfx_v10_0_ring_emit_sb(struct amdgpu_ring *ring)
> > }
> > static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
> > - uint32_t flags,
> > - bool trusted)
> > + uint32_t flags)
> > {
> > uint32_t dw2 = 0;
> > @@ -4573,8 +4571,6 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
> > gfx_v10_0_ring_emit_ce_meta(ring,
> > flags & AMDGPU_IB_PREEMPTED ? true : false);
> > - gfx_v10_0_ring_emit_tmz(ring, true, trusted);
> > -
> > dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
> > if (flags & AMDGPU_HAVE_CTX_SWITCH) {
> > /* set load_global_config & load_global_uconfig */
> > @@ -4731,17 +4727,12 @@ static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume)
> > sizeof(de_payload) >> 2);
> > }
> > -static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
> > - bool trusted)
> > +static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
> > {
> > - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
> > - /*
> > - * cmd = 0: frame begin
> > - * cmd = 1: frame end
> > - */
> > - amdgpu_ring_write(ring,
> > - ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
> > - | FRAME_CMD(start ? 0 : 1));
> > + if (amdgpu_is_tmz(ring->adev)) {
> > + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
> > + amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
> > + }
> > }
> > static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > index 95bb2422b27c..31f44d05e606 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > @@ -2973,8 +2973,7 @@ static uint64_t gfx_v6_0_get_gpu_clock_counter(struct amdgpu_device *adev)
> > return clock;
> > }
> > -static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
> > - bool trusted)
> > +static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
> > {
> > if (flags & AMDGPU_HAVE_CTX_SWITCH)
> > gfx_v6_0_ring_emit_vgt_flush(ring);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > index 648d767d14e7..8f20a5dd44fe 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -2320,8 +2320,7 @@ static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> > amdgpu_ring_write(ring, control);
> > }
> > -static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
> > - bool trusted)
> > +static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
> > {
> > uint32_t dw2 = 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index a46ec1c9846e..fa245973de12 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -6386,8 +6386,7 @@ static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
> > amdgpu_ring_write(ring, 0);
> > }
> > -static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
> > - bool trusted)
> > +static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
> > {
> > uint32_t dw2 = 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index e5753746cbf9..9d3805488832 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -5212,29 +5212,21 @@ static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring)
> > amdgpu_ring_write_multiple(ring, (void *)&de_payload, sizeof(de_payload) >> 2);
> > }
> > -static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
> > - bool trusted)
> > +static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
> > {
> > - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
> > - /*
> > - * cmd = 0: frame begin
> > - * cmd = 1: frame end
> > - */
> > - amdgpu_ring_write(ring,
> > - ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
> > - | FRAME_CMD(start ? 0 : 1));
> > + if (amdgpu_is_tmz(ring->adev)) {
> > + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
> > + amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
> > + }
> > }
> > -static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
> > - bool trusted)
> > +static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
> > {
> > uint32_t dw2 = 0;
> > if (amdgpu_sriov_vf(ring->adev))
> > gfx_v9_0_ring_emit_ce_meta(ring);
> > - gfx_v9_0_ring_emit_tmz(ring, true, trusted);
> > -
> > dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
> > if (flags & AMDGPU_HAVE_CTX_SWITCH) {
> > /* set load_global_config & load_global_uconfig */
> > diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> > index 918ac3548cd3..eaf94a421901 100644
> > --- a/include/uapi/drm/amdgpu_drm.h
> > +++ b/include/uapi/drm/amdgpu_drm.h
> > @@ -556,9 +556,6 @@ struct drm_amdgpu_cs_chunk {
> > __u64 chunk_data;
> > };
> > -/* Flag the command submission as secure */
> > -#define AMDGPU_CS_FLAGS_SECURE (1 << 0)
> > -
> > struct drm_amdgpu_cs_in {
> > /** Rendering context id */
> > __u32 ctx_id;
> > @@ -599,6 +596,10 @@ union drm_amdgpu_cs {
> > */
> > #define AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID (1 << 4)
> > +/* Flag the IB as secure (TMZ)
> > + */
> > +#define AMDGPU_IB_FLAGS_SECURE (1 << 5)
> > +
> > struct drm_amdgpu_cs_chunk_ib {
> > __u32 _pad;
> > /** AMDGPU_IB_FLAG_* */
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cray.huang%40amd.com%7C1749e47fe6294a1c615808d7b06fcb17%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171866515740964&sdata=Il9zrmo2G5zBlQW6BSRIl2qbDQkine7Kdf%2B%2BN7SQA%2Bk%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
2020-02-13 14:57 ` Huang Rui
@ 2020-02-13 22:03 ` Luben Tuikov
0 siblings, 0 replies; 6+ messages in thread
From: Luben Tuikov @ 2020-02-13 22:03 UTC (permalink / raw)
To: Huang Rui, christian.koenig, Deucher, Alexander; +Cc: amd-gfx
On 2020-02-13 09:57, Huang Rui wrote:
> On Thu, Feb 13, 2020 at 11:30:43AM +0100, Christian König wrote:
>> Am 13.02.20 um 01:54 schrieb Luben Tuikov:
>>> Move from a per-CS secure flag (TMZ) to a per-IB
>>> secure flag.
>>
>> Well that seems to make a lot of sense to me, but need to bit of time to
>> read into the PM4 handling of TMZ.
>>
>> Especially what is the "start" parameter good for?
>
> I see this patch just now. And yes, that's my purpose for the discussion.
Yes, I did discuss this implementation with Alex and we seem to be thinking
this would be the most flexible and accomodating way at the moment.
>
> Thanks!
>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
Thans Ray, I'll wheel it off to submit into amdgpu-staging-drm-next.
Regards,
Luben
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ++++++++++++++++++++---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 ++++-----
>>> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++++++----------------
>>> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 3 +--
>>> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 +--
>>> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 +--
>>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 20 ++++++--------------
>>> include/uapi/drm/amdgpu_drm.h | 7 ++++---
>>> 10 files changed, 44 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> index 80ba6dfc54e2..f9fa6e104fef 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> @@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
>>> if (ret)
>>> goto free_all_kdata;
>>> - p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
>>> -
>>> if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
>>> ret = -ECANCELED;
>>> goto free_all_kdata;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> index 6e0f97afb030..cae81914c821 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> @@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>>> uint64_t fence_ctx;
>>> uint32_t status = 0, alloc_size;
>>> unsigned fence_flags = 0;
>>> + bool secure;
>>> unsigned i;
>>> int r = 0;
>>> @@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>>> if (job && ring->funcs->emit_cntxcntl) {
>>> status |= job->preamble_status;
>>> status |= job->preemption_status;
>>> - amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
>>> + amdgpu_ring_emit_cntxcntl(ring, status);
>>> }
>>> + secure = false;
>>> for (i = 0; i < num_ibs; ++i) {
>>> ib = &ibs[i];
>>> @@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
>>> continue;
>>> + /* If this IB is TMZ, add frame TMZ start packet,
>>> + * else, turn off TMZ.
>>> + */
>>> + if (ib->flags & AMDGPU_IB_FLAGS_SECURE && ring->funcs->emit_tmz) {
>>> + if (!secure) {
>>> + secure = true;
>>> + amdgpu_ring_emit_tmz(ring, true);
>>> + }
>>> + } else if (secure) {
>>> + secure = false;
>>> + amdgpu_ring_emit_tmz(ring, false);
>>> + }
>>> +
>>> amdgpu_ring_emit_ib(ring, job, ib, status);
>>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>>> }
>>> - if (ring->funcs->emit_tmz)
>>> - amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
>>> + if (secure) {
>>> + secure = false;
>>> + amdgpu_ring_emit_tmz(ring, false);
>>> + }
>>> #ifdef CONFIG_X86_64
>>> if (!(adev->flags & AMD_IS_APU))
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>> index 31c4444b0203..2e2110dddb76 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>> @@ -61,9 +61,6 @@ struct amdgpu_job {
>>> /* user fence handling */
>>> uint64_t uf_addr;
>>> uint64_t uf_sequence;
>>> -
>>> - /* the job is due to a secure command submission */
>>> - bool secure;
>>> };
>>> int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> index 5134d0dd6dc2..930316e60155 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> @@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
>>> void (*begin_use)(struct amdgpu_ring *ring);
>>> void (*end_use)(struct amdgpu_ring *ring);
>>> void (*emit_switch_buffer) (struct amdgpu_ring *ring);
>>> - void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
>>> - bool trusted);
>>> + void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
>>> void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
>>> void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
>>> void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
>>> @@ -167,7 +166,7 @@ struct amdgpu_ring_funcs {
>>> void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
>>> uint32_t reg0, uint32_t reg1,
>>> uint32_t ref, uint32_t mask);
>>> - void (*emit_tmz)(struct amdgpu_ring *ring, bool start, bool trusted);
>>> + void (*emit_tmz)(struct amdgpu_ring *ring, bool start);
>>> /* priority functions */
>>> void (*set_priority) (struct amdgpu_ring *ring,
>>> enum drm_sched_priority priority);
>>> @@ -243,12 +242,12 @@ struct amdgpu_ring {
>>> #define amdgpu_ring_emit_gds_switch(r, v, db, ds, wb, ws, ab, as) (r)->funcs->emit_gds_switch((r), (v), (db), (ds), (wb), (ws), (ab), (as))
>>> #define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
>>> #define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
>>> -#define amdgpu_ring_emit_cntxcntl(r, d, s) (r)->funcs->emit_cntxcntl((r), (d), (s))
>>> +#define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
>>> #define amdgpu_ring_emit_rreg(r, d) (r)->funcs->emit_rreg((r), (d))
>>> #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
>>> #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
>>> #define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) (r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
>>> -#define amdgpu_ring_emit_tmz(r, b, s) (r)->funcs->emit_tmz((r), (b), (s))
>>> +#define amdgpu_ring_emit_tmz(r, b) (r)->funcs->emit_tmz((r), (b))
>>> #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
>>> #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
>>> #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>> index 4e25b39ac14f..1a9787a0177e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>> @@ -256,8 +256,7 @@ static int gfx_v10_0_rlc_backdoor_autoload_enable(struct amdgpu_device *adev);
>>> static int gfx_v10_0_wait_for_rlc_autoload_complete(struct amdgpu_device *adev);
>>> static void gfx_v10_0_ring_emit_ce_meta(struct amdgpu_ring *ring, bool resume);
>>> static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume);
>>> -static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
>>> - bool trusted);
>>> +static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start);
>>> static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)
>>> {
>>> @@ -4564,8 +4563,7 @@ static void gfx_v10_0_ring_emit_sb(struct amdgpu_ring *ring)
>>> }
>>> static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
>>> - uint32_t flags,
>>> - bool trusted)
>>> + uint32_t flags)
>>> {
>>> uint32_t dw2 = 0;
>>> @@ -4573,8 +4571,6 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
>>> gfx_v10_0_ring_emit_ce_meta(ring,
>>> flags & AMDGPU_IB_PREEMPTED ? true : false);
>>> - gfx_v10_0_ring_emit_tmz(ring, true, trusted);
>>> -
>>> dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
>>> if (flags & AMDGPU_HAVE_CTX_SWITCH) {
>>> /* set load_global_config & load_global_uconfig */
>>> @@ -4731,17 +4727,12 @@ static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume)
>>> sizeof(de_payload) >> 2);
>>> }
>>> -static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
>>> - bool trusted)
>>> +static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
>>> {
>>> - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
>>> - /*
>>> - * cmd = 0: frame begin
>>> - * cmd = 1: frame end
>>> - */
>>> - amdgpu_ring_write(ring,
>>> - ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
>>> - | FRAME_CMD(start ? 0 : 1));
>>> + if (amdgpu_is_tmz(ring->adev)) {
>>> + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
>>> + amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
>>> + }
>>> }
>>> static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> index 95bb2422b27c..31f44d05e606 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> @@ -2973,8 +2973,7 @@ static uint64_t gfx_v6_0_get_gpu_clock_counter(struct amdgpu_device *adev)
>>> return clock;
>>> }
>>> -static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
>>> - bool trusted)
>>> +static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>>> {
>>> if (flags & AMDGPU_HAVE_CTX_SWITCH)
>>> gfx_v6_0_ring_emit_vgt_flush(ring);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> index 648d767d14e7..8f20a5dd44fe 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> @@ -2320,8 +2320,7 @@ static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>>> amdgpu_ring_write(ring, control);
>>> }
>>> -static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
>>> - bool trusted)
>>> +static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>>> {
>>> uint32_t dw2 = 0;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> index a46ec1c9846e..fa245973de12 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> @@ -6386,8 +6386,7 @@ static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
>>> amdgpu_ring_write(ring, 0);
>>> }
>>> -static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
>>> - bool trusted)
>>> +static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>>> {
>>> uint32_t dw2 = 0;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> index e5753746cbf9..9d3805488832 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> @@ -5212,29 +5212,21 @@ static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring)
>>> amdgpu_ring_write_multiple(ring, (void *)&de_payload, sizeof(de_payload) >> 2);
>>> }
>>> -static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
>>> - bool trusted)
>>> +static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
>>> {
>>> - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
>>> - /*
>>> - * cmd = 0: frame begin
>>> - * cmd = 1: frame end
>>> - */
>>> - amdgpu_ring_write(ring,
>>> - ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
>>> - | FRAME_CMD(start ? 0 : 1));
>>> + if (amdgpu_is_tmz(ring->adev)) {
>>> + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
>>> + amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
>>> + }
>>> }
>>> -static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
>>> - bool trusted)
>>> +static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>>> {
>>> uint32_t dw2 = 0;
>>> if (amdgpu_sriov_vf(ring->adev))
>>> gfx_v9_0_ring_emit_ce_meta(ring);
>>> - gfx_v9_0_ring_emit_tmz(ring, true, trusted);
>>> -
>>> dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
>>> if (flags & AMDGPU_HAVE_CTX_SWITCH) {
>>> /* set load_global_config & load_global_uconfig */
>>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>>> index 918ac3548cd3..eaf94a421901 100644
>>> --- a/include/uapi/drm/amdgpu_drm.h
>>> +++ b/include/uapi/drm/amdgpu_drm.h
>>> @@ -556,9 +556,6 @@ struct drm_amdgpu_cs_chunk {
>>> __u64 chunk_data;
>>> };
>>> -/* Flag the command submission as secure */
>>> -#define AMDGPU_CS_FLAGS_SECURE (1 << 0)
>>> -
>>> struct drm_amdgpu_cs_in {
>>> /** Rendering context id */
>>> __u32 ctx_id;
>>> @@ -599,6 +596,10 @@ union drm_amdgpu_cs {
>>> */
>>> #define AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID (1 << 4)
>>> +/* Flag the IB as secure (TMZ)
>>> + */
>>> +#define AMDGPU_IB_FLAGS_SECURE (1 << 5)
>>> +
>>> struct drm_amdgpu_cs_chunk_ib {
>>> __u32 _pad;
>>> /** AMDGPU_IB_FLAG_* */
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cray.huang%40amd.com%7C1749e47fe6294a1c615808d7b06fcb17%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171866515740964&sdata=Il9zrmo2G5zBlQW6BSRIl2qbDQkine7Kdf%2B%2BN7SQA%2Bk%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
2020-02-13 10:30 ` Christian König
2020-02-13 14:57 ` Huang Rui
@ 2020-02-13 22:00 ` Luben Tuikov
1 sibling, 0 replies; 6+ messages in thread
From: Luben Tuikov @ 2020-02-13 22:00 UTC (permalink / raw)
To: christian.koenig, amd-gfx
On 2020-02-13 05:30, Christian König wrote:
> Am 13.02.20 um 01:54 schrieb Luben Tuikov:
>> Move from a per-CS secure flag (TMZ) to a per-IB
>> secure flag.
>
> Well that seems to make a lot of sense to me, but need to bit of time to
> read into the PM4 handling of TMZ.
Note that this patch is _not_ adding new functionality,
or anything new. It is not adding new PM3 or even PM4
functionality. Nothing new here.
It only moves the hierarchy from CS to IB of the
secure flag. That's all.
I think it is safe to submit it now, as it would alleviate
libdrm secure buffer submission.
>
> Especially what is the "start" parameter good for?
Note that this is existing code. It must've been reviewed
by someone when it was submitted and git-blame is good
with that.
The "start" parameter controls whether the packet
switches the ring to TMZ or non-TMZ processing.
Regards,
Luben
>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ++++++++++++++++++++---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 ++++-----
>> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++++++----------------
>> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 3 +--
>> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 +--
>> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 +--
>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 20 ++++++--------------
>> include/uapi/drm/amdgpu_drm.h | 7 ++++---
>> 10 files changed, 44 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index 80ba6dfc54e2..f9fa6e104fef 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
>> if (ret)
>> goto free_all_kdata;
>>
>> - p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
>> -
>> if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
>> ret = -ECANCELED;
>> goto free_all_kdata;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 6e0f97afb030..cae81914c821 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>> uint64_t fence_ctx;
>> uint32_t status = 0, alloc_size;
>> unsigned fence_flags = 0;
>> + bool secure;
>>
>> unsigned i;
>> int r = 0;
>> @@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>> if (job && ring->funcs->emit_cntxcntl) {
>> status |= job->preamble_status;
>> status |= job->preemption_status;
>> - amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
>> + amdgpu_ring_emit_cntxcntl(ring, status);
>> }
>>
>> + secure = false;
>> for (i = 0; i < num_ibs; ++i) {
>> ib = &ibs[i];
>>
>> @@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
>> continue;
>>
>> + /* If this IB is TMZ, add frame TMZ start packet,
>> + * else, turn off TMZ.
>> + */
>> + if (ib->flags & AMDGPU_IB_FLAGS_SECURE && ring->funcs->emit_tmz) {
>> + if (!secure) {
>> + secure = true;
>> + amdgpu_ring_emit_tmz(ring, true);
>> + }
>> + } else if (secure) {
>> + secure = false;
>> + amdgpu_ring_emit_tmz(ring, false);
>> + }
>> +
>> amdgpu_ring_emit_ib(ring, job, ib, status);
>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>> }
>>
>> - if (ring->funcs->emit_tmz)
>> - amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
>> + if (secure) {
>> + secure = false;
>> + amdgpu_ring_emit_tmz(ring, false);
>> + }
>>
>> #ifdef CONFIG_X86_64
>> if (!(adev->flags & AMD_IS_APU))
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>> index 31c4444b0203..2e2110dddb76 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>> @@ -61,9 +61,6 @@ struct amdgpu_job {
>> /* user fence handling */
>> uint64_t uf_addr;
>> uint64_t uf_sequence;
>> -
>> - /* the job is due to a secure command submission */
>> - bool secure;
>> };
>>
>> int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> index 5134d0dd6dc2..930316e60155 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> @@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
>> void (*begin_use)(struct amdgpu_ring *ring);
>> void (*end_use)(struct amdgpu_ring *ring);
>> void (*emit_switch_buffer) (struct amdgpu_ring *ring);
>> - void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
>> - bool trusted);
>> + void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
>> void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
>> void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
>> void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
>> @@ -167,7 +166,7 @@ struct amdgpu_ring_funcs {
>> void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
>> uint32_t reg0, uint32_t reg1,
>> uint32_t ref, uint32_t mask);
>> - void (*emit_tmz)(struct amdgpu_ring *ring, bool start, bool trusted);
>> + void (*emit_tmz)(struct amdgpu_ring *ring, bool start);
>> /* priority functions */
>> void (*set_priority) (struct amdgpu_ring *ring,
>> enum drm_sched_priority priority);
>> @@ -243,12 +242,12 @@ struct amdgpu_ring {
>> #define amdgpu_ring_emit_gds_switch(r, v, db, ds, wb, ws, ab, as) (r)->funcs->emit_gds_switch((r), (v), (db), (ds), (wb), (ws), (ab), (as))
>> #define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
>> #define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
>> -#define amdgpu_ring_emit_cntxcntl(r, d, s) (r)->funcs->emit_cntxcntl((r), (d), (s))
>> +#define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
>> #define amdgpu_ring_emit_rreg(r, d) (r)->funcs->emit_rreg((r), (d))
>> #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
>> #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
>> #define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) (r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
>> -#define amdgpu_ring_emit_tmz(r, b, s) (r)->funcs->emit_tmz((r), (b), (s))
>> +#define amdgpu_ring_emit_tmz(r, b) (r)->funcs->emit_tmz((r), (b))
>> #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
>> #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
>> #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> index 4e25b39ac14f..1a9787a0177e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> @@ -256,8 +256,7 @@ static int gfx_v10_0_rlc_backdoor_autoload_enable(struct amdgpu_device *adev);
>> static int gfx_v10_0_wait_for_rlc_autoload_complete(struct amdgpu_device *adev);
>> static void gfx_v10_0_ring_emit_ce_meta(struct amdgpu_ring *ring, bool resume);
>> static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume);
>> -static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
>> - bool trusted);
>> +static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start);
>>
>> static void gfx10_kiq_set_resources(struct amdgpu_ring *kiq_ring, uint64_t queue_mask)
>> {
>> @@ -4564,8 +4563,7 @@ static void gfx_v10_0_ring_emit_sb(struct amdgpu_ring *ring)
>> }
>>
>> static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
>> - uint32_t flags,
>> - bool trusted)
>> + uint32_t flags)
>> {
>> uint32_t dw2 = 0;
>>
>> @@ -4573,8 +4571,6 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
>> gfx_v10_0_ring_emit_ce_meta(ring,
>> flags & AMDGPU_IB_PREEMPTED ? true : false);
>>
>> - gfx_v10_0_ring_emit_tmz(ring, true, trusted);
>> -
>> dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
>> if (flags & AMDGPU_HAVE_CTX_SWITCH) {
>> /* set load_global_config & load_global_uconfig */
>> @@ -4731,17 +4727,12 @@ static void gfx_v10_0_ring_emit_de_meta(struct amdgpu_ring *ring, bool resume)
>> sizeof(de_payload) >> 2);
>> }
>>
>> -static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
>> - bool trusted)
>> +static void gfx_v10_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
>> {
>> - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
>> - /*
>> - * cmd = 0: frame begin
>> - * cmd = 1: frame end
>> - */
>> - amdgpu_ring_write(ring,
>> - ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
>> - | FRAME_CMD(start ? 0 : 1));
>> + if (amdgpu_is_tmz(ring->adev)) {
>> + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
>> + amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
>> + }
>> }
>>
>> static void gfx_v10_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>> index 95bb2422b27c..31f44d05e606 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>> @@ -2973,8 +2973,7 @@ static uint64_t gfx_v6_0_get_gpu_clock_counter(struct amdgpu_device *adev)
>> return clock;
>> }
>>
>> -static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
>> - bool trusted)
>> +static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>> {
>> if (flags & AMDGPU_HAVE_CTX_SWITCH)
>> gfx_v6_0_ring_emit_vgt_flush(ring);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> index 648d767d14e7..8f20a5dd44fe 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> @@ -2320,8 +2320,7 @@ static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>> amdgpu_ring_write(ring, control);
>> }
>>
>> -static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
>> - bool trusted)
>> +static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>> {
>> uint32_t dw2 = 0;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index a46ec1c9846e..fa245973de12 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -6386,8 +6386,7 @@ static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
>> amdgpu_ring_write(ring, 0);
>> }
>>
>> -static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
>> - bool trusted)
>> +static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>> {
>> uint32_t dw2 = 0;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index e5753746cbf9..9d3805488832 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -5212,29 +5212,21 @@ static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring)
>> amdgpu_ring_write_multiple(ring, (void *)&de_payload, sizeof(de_payload) >> 2);
>> }
>>
>> -static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
>> - bool trusted)
>> +static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start)
>> {
>> - amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
>> - /*
>> - * cmd = 0: frame begin
>> - * cmd = 1: frame end
>> - */
>> - amdgpu_ring_write(ring,
>> - ((amdgpu_is_tmz(ring->adev) && trusted) ? FRAME_TMZ : 0)
>> - | FRAME_CMD(start ? 0 : 1));
>> + if (amdgpu_is_tmz(ring->adev)) {
>> + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0));
>> + amdgpu_ring_write(ring, FRAME_TMZ | FRAME_CMD(start ? 0 : 1));
>> + }
>> }
>>
>> -static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
>> - bool trusted)
>> +static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
>> {
>> uint32_t dw2 = 0;
>>
>> if (amdgpu_sriov_vf(ring->adev))
>> gfx_v9_0_ring_emit_ce_meta(ring);
>>
>> - gfx_v9_0_ring_emit_tmz(ring, true, trusted);
>> -
>> dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
>> if (flags & AMDGPU_HAVE_CTX_SWITCH) {
>> /* set load_global_config & load_global_uconfig */
>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>> index 918ac3548cd3..eaf94a421901 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -556,9 +556,6 @@ struct drm_amdgpu_cs_chunk {
>> __u64 chunk_data;
>> };
>>
>> -/* Flag the command submission as secure */
>> -#define AMDGPU_CS_FLAGS_SECURE (1 << 0)
>> -
>> struct drm_amdgpu_cs_in {
>> /** Rendering context id */
>> __u32 ctx_id;
>> @@ -599,6 +596,10 @@ union drm_amdgpu_cs {
>> */
>> #define AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID (1 << 4)
>>
>> +/* Flag the IB as secure (TMZ)
>> + */
>> +#define AMDGPU_IB_FLAGS_SECURE (1 << 5)
>> +
>> struct drm_amdgpu_cs_chunk_ib {
>> __u32 _pad;
>> /** AMDGPU_IB_FLAG_* */
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CH2PR12MB39125C58ABA40856F1CBF8DAF7190@CH2PR12MB3912.namprd12.prod.outlook.com>]
* [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
[not found] <CH2PR12MB39125C58ABA40856F1CBF8DAF7190@CH2PR12MB3912.namprd12.prod.outlook.com>
@ 2020-02-10 23:25 ` Luben Tuikov
0 siblings, 0 replies; 6+ messages in thread
From: Luben Tuikov @ 2020-02-10 23:25 UTC (permalink / raw)
To: Alexander Deucher, Christian Koenig, amd-gfx, Ray Huang,
Marek Olsak, Pierre-Eric Pelloux-Prayer
Cc: Luben Tuikov, Leo Liu, Aaron Liu
Move from a per-CS secure flag (TMZ) to a per-IB
secure flag.
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ++++++++++++++++++++---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 5 ++---
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 5 +----
drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 5 +----
include/uapi/drm/amdgpu_drm.h | 7 ++++---
10 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 80ba6dfc54e2..f9fa6e104fef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
if (ret)
goto free_all_kdata;
- p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
-
if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
ret = -ECANCELED;
goto free_all_kdata;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 6e0f97afb030..f9515db2823b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
uint64_t fence_ctx;
uint32_t status = 0, alloc_size;
unsigned fence_flags = 0;
+ bool secure;
unsigned i;
int r = 0;
@@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
if (job && ring->funcs->emit_cntxcntl) {
status |= job->preamble_status;
status |= job->preemption_status;
- amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
+ amdgpu_ring_emit_cntxcntl(ring, status);
}
+ secure = false;
for (i = 0; i < num_ibs; ++i) {
ib = &ibs[i];
@@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE ib must be inserted anyway */
continue;
+ /* If this IB is TMZ, add frame TMZ start packet,
+ * else, turn off TMZ.
+ */
+ if (ib->flags & AMDGPU_IB_FLAGS_SECURE && ring->funcs->emit_tmz) {
+ if (!secure) {
+ secure = true;
+ amdgpu_ring_emit_tmz(ring, true, true);
+ }
+ } else if (secure) {
+ secure = false;
+ amdgpu_ring_emit_tmz(ring, false, false);
+ }
+
amdgpu_ring_emit_ib(ring, job, ib, status);
status &= ~AMDGPU_HAVE_CTX_SWITCH;
}
- if (ring->funcs->emit_tmz)
- amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
+ if (secure) {
+ secure = false;
+ amdgpu_ring_emit_tmz(ring, false, false);
+ }
#ifdef CONFIG_X86_64
if (!(adev->flags & AMD_IS_APU))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 31c4444b0203..2e2110dddb76 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -61,9 +61,6 @@ struct amdgpu_job {
/* user fence handling */
uint64_t uf_addr;
uint64_t uf_sequence;
-
- /* the job is due to a secure command submission */
- bool secure;
};
int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 5134d0dd6dc2..34aa63ad5799 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
void (*begin_use)(struct amdgpu_ring *ring);
void (*end_use)(struct amdgpu_ring *ring);
void (*emit_switch_buffer) (struct amdgpu_ring *ring);
- void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
- bool trusted);
+ void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
@@ -243,7 +242,7 @@ struct amdgpu_ring {
#define amdgpu_ring_emit_gds_switch(r, v, db, ds, wb, ws, ab, as) (r)->funcs->emit_gds_switch((r), (v), (db), (ds), (wb), (ws), (ab), (as))
#define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
#define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
-#define amdgpu_ring_emit_cntxcntl(r, d, s) (r)->funcs->emit_cntxcntl((r), (d), (s))
+#define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
#define amdgpu_ring_emit_rreg(r, d) (r)->funcs->emit_rreg((r), (d))
#define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
#define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), (d), (v), (m))
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 4e25b39ac14f..42bdff7c2add 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4564,8 +4564,7 @@ static void gfx_v10_0_ring_emit_sb(struct amdgpu_ring *ring)
}
static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
- uint32_t flags,
- bool trusted)
+ uint32_t flags)
{
uint32_t dw2 = 0;
@@ -4573,8 +4572,6 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
gfx_v10_0_ring_emit_ce_meta(ring,
flags & AMDGPU_IB_PREEMPTED ? true : false);
- gfx_v10_0_ring_emit_tmz(ring, true, trusted);
-
dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
if (flags & AMDGPU_HAVE_CTX_SWITCH) {
/* set load_global_config & load_global_uconfig */
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 95bb2422b27c..31f44d05e606 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -2973,8 +2973,7 @@ static uint64_t gfx_v6_0_get_gpu_clock_counter(struct amdgpu_device *adev)
return clock;
}
-static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
- bool trusted)
+static void gfx_v6_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
{
if (flags & AMDGPU_HAVE_CTX_SWITCH)
gfx_v6_0_ring_emit_vgt_flush(ring);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 648d767d14e7..8f20a5dd44fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2320,8 +2320,7 @@ static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
amdgpu_ring_write(ring, control);
}
-static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
- bool trusted)
+static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
{
uint32_t dw2 = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index a46ec1c9846e..fa245973de12 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6386,8 +6386,7 @@ static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
amdgpu_ring_write(ring, 0);
}
-static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
- bool trusted)
+static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
{
uint32_t dw2 = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index e5753746cbf9..0a47a5ca5ad1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -5225,16 +5225,13 @@ static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool start,
| FRAME_CMD(start ? 0 : 1));
}
-static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags,
- bool trusted)
+static void gfx_v9_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
{
uint32_t dw2 = 0;
if (amdgpu_sriov_vf(ring->adev))
gfx_v9_0_ring_emit_ce_meta(ring);
- gfx_v9_0_ring_emit_tmz(ring, true, trusted);
-
dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
if (flags & AMDGPU_HAVE_CTX_SWITCH) {
/* set load_global_config & load_global_uconfig */
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 918ac3548cd3..eaf94a421901 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -556,9 +556,6 @@ struct drm_amdgpu_cs_chunk {
__u64 chunk_data;
};
-/* Flag the command submission as secure */
-#define AMDGPU_CS_FLAGS_SECURE (1 << 0)
-
struct drm_amdgpu_cs_in {
/** Rendering context id */
__u32 ctx_id;
@@ -599,6 +596,10 @@ union drm_amdgpu_cs {
*/
#define AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID (1 << 4)
+/* Flag the IB as secure (TMZ)
+ */
+#define AMDGPU_IB_FLAGS_SECURE (1 << 5)
+
struct drm_amdgpu_cs_chunk_ib {
__u32 _pad;
/** AMDGPU_IB_FLAG_* */
--
2.25.0.26.gc7a6207591
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-13 22:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 0:54 [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ) Luben Tuikov
2020-02-13 10:30 ` Christian König
2020-02-13 14:57 ` Huang Rui
2020-02-13 22:03 ` Luben Tuikov
2020-02-13 22:00 ` Luben Tuikov
[not found] <CH2PR12MB39125C58ABA40856F1CBF8DAF7190@CH2PR12MB3912.namprd12.prod.outlook.com>
2020-02-10 23:25 ` Luben Tuikov
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).