Re: [PATCH 4/4] drm/amdgpu/vcn: prevent silent fence drop on 64-bit flag mismatch

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

On 4/26/26 23:52, jbmoore wrote:
> From: "John B. Moore" <jbmoore61@gmail.com>
> 
> VCN, UVD, and VCE encoder/decoder ring fence emission callbacks only
> support 32-bit fence writes.  When AMDGPU_FENCE_FLAG_64BIT is passed,
> the existing bare WARN_ON() fires but execution continues, emitting
> a truncated fence that causes the VCN hardware unit to issue a
> no-retry UTCL2 page fault at NULL address (0x0).
> 
> The hardware fault is non-recoverable: the VCNU client is permanently
> stalled, the VCN ring stops processing jobs, and all pending fences
> on the affected ring never signal.
> 
> Convert WARN_ON() to WARN_ON_ONCE() and add an early return to
> prevent the corrupted fence emission.  The early return is safe
> because the WARN_ON fires before any ring buffer writes in all five
> affected callsites:
>   - vcn_v1_0_dec_ring_emit_fence()
>   - vcn_v1_0_enc_ring_emit_fence()
>   - vcn_v2_0_dec_ring_emit_fence()
>   - vcn_v2_0_enc_ring_emit_fence()
>   - vcn_dec_sw_ring_emit_fence()
> 
> The missing fence will be caught by the scheduler timeout mechanism,
> which will clean up the job without hardware damage.
> 
> Using WARN_ON_ONCE instead of the bare WARN_ON also prevents kernel
> log flooding if the condition is triggered repeatedly by a fuzzer.
> 
> Found by a custom amdgpu DRM ioctl fuzzer.

Absolutely clear NAK. Not emitting the fence is even worse than the page fault.

Question is rather why that isn't filtered upfront by the CS IOCTL?

Regards,
Christian.

> 
> Fixes: 8ace845ff0e8 ("drm/amdgpu: add vcn enc ring type and functions")
> Fixes: cca69fe8ff98 ("drm/amdgpu: add vcn decode ring type and functions")
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 6 ++++--
>  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c    | 6 ++++--
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c b/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> index 2b9ddb3d2..aa0022deb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> @@ -27,7 +27,8 @@
>  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);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  
>  	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..2acf6e621 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1548,7 +1548,8 @@ 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);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  
>  	amdgpu_ring_write(ring,
>  		PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_CONTEXT_ID), 0));
> @@ -1724,7 +1725,8 @@ 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);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  
>  	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..6cfb5aedd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -1537,7 +1537,8 @@ 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);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  	amdgpu_ring_write(ring, PACKET0(adev->vcn.inst[ring->me].internal.context_id, 0));
>  	amdgpu_ring_write(ring, seq);
>  
> @@ -1722,7 +1723,8 @@ 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);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  
>  	amdgpu_ring_write(ring, VCN_ENC_CMD_FENCE);
>  	amdgpu_ring_write(ring, addr);