Well, not through this particular piece of code, since this explicitly sets it. But I would imagine someone could set the bit in userspace and insert KMD commands in the BO as part of some IB instructions - I’ll have a look.
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Friday, July 26, 2019 3:17:19 AM
To: Thai, Thong <Thong.Thai@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH 1/2] drm/amd/amdgpu/vcn_v2_0: Mark RB commands as KMD commands
Sent: Friday, July 26, 2019 3:17:19 AM
To: Thai, Thong <Thong.Thai@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH 1/2] drm/amd/amdgpu/vcn_v2_0: Mark RB commands as KMD commands
Am 25.07.19 um 17:52 schrieb Thai, Thong:
> Sets the CMD_SOURCE bit for VCN 2.0 Decoder Ring Buffer commands. This
> bit was previously set by the RBC HW on older firmware. Newer firmware
> uses a SW RBC and this bit has to be set by the driver.
Mhm, another question came to my mind: Would it now be possible for user
space to set this flag and and gain access to the kernel driver commands?
Cause that could be a security problem.
Christian.
>
> Signed-off-by: Thong Thai <thong.thai@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
> drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 12 ++++++------
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 5e2453ee6b29..4d3bf4adf1eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -30,6 +30,7 @@
> #define AMDGPU_VCN_FIRMWARE_OFFSET 256
> #define AMDGPU_VCN_MAX_ENC_RINGS 3
>
> +#define VCN_DEC_KMD_CMD 0x80000000
> #define VCN_DEC_CMD_FENCE 0x00000000
> #define VCN_DEC_CMD_TRAP 0x00000001
> #define VCN_DEC_CMD_WRITE_REG 0x00000004
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> index bc9726787c97..7091aef95ff0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -1488,7 +1488,7 @@ static void vcn_v2_0_dec_ring_insert_start(struct amdgpu_ring *ring)
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));
> amdgpu_ring_write(ring, 0);
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_START << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_PACKET_START << 1));
> }
>
> /**
> @@ -1501,7 +1501,7 @@ static void vcn_v2_0_dec_ring_insert_start(struct amdgpu_ring *ring)
> static void vcn_v2_0_dec_ring_insert_end(struct amdgpu_ring *ring)
> {
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_END << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_PACKET_END << 1));
> }
>
> /**
> @@ -1546,7 +1546,7 @@ static void vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
> amdgpu_ring_write(ring, upper_32_bits(addr) & 0xff);
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_FENCE << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_FENCE << 1));
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));
> amdgpu_ring_write(ring, 0);
> @@ -1556,7 +1556,7 @@ static void vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_TRAP << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_TRAP << 1));
> }
>
> /**
> @@ -1600,7 +1600,7 @@ static void vcn_v2_0_dec_ring_emit_reg_wait(struct amdgpu_ring *ring,
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_REG_READ_COND_WAIT << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_REG_READ_COND_WAIT << 1));
> }
>
> static void vcn_v2_0_dec_ring_emit_vm_flush(struct amdgpu_ring *ring,
> @@ -1629,7 +1629,7 @@ static void vcn_v2_0_dec_ring_emit_wreg(struct amdgpu_ring *ring,
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_WRITE_REG << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_WRITE_REG << 1));
> }
>
> /**
> Sets the CMD_SOURCE bit for VCN 2.0 Decoder Ring Buffer commands. This
> bit was previously set by the RBC HW on older firmware. Newer firmware
> uses a SW RBC and this bit has to be set by the driver.
Mhm, another question came to my mind: Would it now be possible for user
space to set this flag and and gain access to the kernel driver commands?
Cause that could be a security problem.
Christian.
>
> Signed-off-by: Thong Thai <thong.thai@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
> drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 12 ++++++------
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 5e2453ee6b29..4d3bf4adf1eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -30,6 +30,7 @@
> #define AMDGPU_VCN_FIRMWARE_OFFSET 256
> #define AMDGPU_VCN_MAX_ENC_RINGS 3
>
> +#define VCN_DEC_KMD_CMD 0x80000000
> #define VCN_DEC_CMD_FENCE 0x00000000
> #define VCN_DEC_CMD_TRAP 0x00000001
> #define VCN_DEC_CMD_WRITE_REG 0x00000004
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> index bc9726787c97..7091aef95ff0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -1488,7 +1488,7 @@ static void vcn_v2_0_dec_ring_insert_start(struct amdgpu_ring *ring)
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));
> amdgpu_ring_write(ring, 0);
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_START << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_PACKET_START << 1));
> }
>
> /**
> @@ -1501,7 +1501,7 @@ static void vcn_v2_0_dec_ring_insert_start(struct amdgpu_ring *ring)
> static void vcn_v2_0_dec_ring_insert_end(struct amdgpu_ring *ring)
> {
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_END << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_PACKET_END << 1));
> }
>
> /**
> @@ -1546,7 +1546,7 @@ static void vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
> amdgpu_ring_write(ring, upper_32_bits(addr) & 0xff);
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_FENCE << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_FENCE << 1));
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));
> amdgpu_ring_write(ring, 0);
> @@ -1556,7 +1556,7 @@ static void vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_TRAP << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_TRAP << 1));
> }
>
> /**
> @@ -1600,7 +1600,7 @@ static void vcn_v2_0_dec_ring_emit_reg_wait(struct amdgpu_ring *ring,
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_REG_READ_COND_WAIT << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_REG_READ_COND_WAIT << 1));
> }
>
> static void vcn_v2_0_dec_ring_emit_vm_flush(struct amdgpu_ring *ring,
> @@ -1629,7 +1629,7 @@ static void vcn_v2_0_dec_ring_emit_wreg(struct amdgpu_ring *ring,
>
> amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_WRITE_REG << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_WRITE_REG << 1));
> }
>
> /**
_______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx