Re: [PATCH v4 4/4] drm/amdgpu: do not pass AMDGPU_FENCE_FLAG_64BIT to media rings

["Christian König" <christian.koenig@amd.com>]

On 4/27/26 18:30, jbmoore wrote:
> From: "John B. Moore" <jbmoore61@gmail.com>
> 
> amdgpu_ib_schedule() unconditionally ORs AMDGPU_FENCE_FLAG_64BIT into
> the flags when emitting the user fence for every ring type:
> 
>   amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
>                          fence_flags | AMDGPU_FENCE_FLAG_64BIT);
> 
> VCN, UVD, VCE, and JPEG encoder/decoder rings only support 32-bit
> fence values.  Their emit_fence callbacks contain bare WARN_ON()
> assertions for this flag, but the flag should never reach them in
> the first place.
> 
> The VCN_ENC_CMD_FENCE hardware packet writes a single 32-bit
> sequence value to a 64-bit GPU address.  There is no 64-bit fence
> variant in the VCN/UVD/VCE/JPEG command sets.
> 
> Filter AMDGPU_FENCE_FLAG_64BIT at the call site in
> amdgpu_ib_schedule(), only setting it for ring types whose hardware
> supports 64-bit fence writes: GFX, compute, SDMA, KIQ, MES, and VPE.
> 
> Also convert the bare WARN_ON() guards in the five affected VCN
> callbacks to WARN_ON_ONCE() to prevent kernel log flooding if
> the condition is somehow triggered via another path.
> 
> Found by a custom amdgpu DRM ioctl fuzzer.

Well again clear NAK.

First of all this shouldn't be handled by the IB scheduler code and second that is already fixed upstream.

Regards,
Christian.

> 
> Fixes: c660f40b1ef3 ("drm/amdgpu: fix user fence write race condition")
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   | 18 +++++++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c |  2 +-
>  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c    |  4 ++--
>  4 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index f1ed4a436..3c32a6197 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -297,8 +297,24 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned int num_ibs,
>  
>  	/* wrap the last IB with fence */
>  	if (job && job->uf_addr) {
> +		unsigned int uf_flags = fence_flags;
> +
> +		/*
> +		 * Only request 64-bit fence writes on rings whose hardware
> +		 * supports them.  VCN/UVD/VCE/JPEG rings only support 32-bit
> +		 * fence values; passing AMDGPU_FENCE_FLAG_64BIT causes their
> +		 * emit_fence callbacks to WARN and emit a truncated fence.
> +		 */
> +		if (ring->funcs->type == AMDGPU_RING_TYPE_GFX ||
> +		    ring->funcs->type == AMDGPU_RING_TYPE_COMPUTE ||
> +		    ring->funcs->type == AMDGPU_RING_TYPE_SDMA ||
> +		    ring->funcs->type == AMDGPU_RING_TYPE_KIQ ||
> +		    ring->funcs->type == AMDGPU_RING_TYPE_MES ||
> +		    ring->funcs->type == AMDGPU_RING_TYPE_VPE)
> +			uf_flags |= AMDGPU_FENCE_FLAG_64BIT;
> +
>  		amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
> -				       fence_flags | AMDGPU_FENCE_FLAG_64BIT);
> +				       uf_flags);
>  	}
>  
>  	if (ring->funcs->emit_gfx_shadow && ring->funcs->init_cond_exec &&
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c b/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> index 2b9ddb3d2..9adc7607c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> @@ -27,7 +27,7 @@
>  void vcn_dec_sw_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>  	u64 seq, uint32_t flags)
>  {
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT);
>  
>  	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_FENCE);
>  	amdgpu_ring_write(ring, addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index e9d790914..729c1c378 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1548,7 +1548,7 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>  {
>  	struct amdgpu_device *adev = ring->adev;
>  
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT);
>  
>  	amdgpu_ring_write(ring,
>  		PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_CONTEXT_ID), 0));
> @@ -1724,7 +1724,7 @@ static void vcn_v1_0_enc_ring_set_wptr(struct amdgpu_ring *ring)
>  static void vcn_v1_0_enc_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>  			u64 seq, unsigned flags)
>  {
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT);
>  
>  	amdgpu_ring_write(ring, VCN_ENC_CMD_FENCE);
>  	amdgpu_ring_write(ring, addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> index e35fae9cd..a020140fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -1537,7 +1537,7 @@ void vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
>  {
>  	struct amdgpu_device *adev = ring->adev;
>  
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT);
>  	amdgpu_ring_write(ring, PACKET0(adev->vcn.inst[ring->me].internal.context_id, 0));
>  	amdgpu_ring_write(ring, seq);
>  
> @@ -1722,7 +1722,7 @@ static void vcn_v2_0_enc_ring_set_wptr(struct amdgpu_ring *ring)
>  void vcn_v2_0_enc_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>  				u64 seq, unsigned flags)
>  {
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT);
>  
>  	amdgpu_ring_write(ring, VCN_ENC_CMD_FENCE);
>  	amdgpu_ring_write(ring, addr);