Re: [PATCH 2/4] drm/amdgpu/gfx9: replace BUG_ON/BUG with WARN_ON_ONCE in ring emission

Christian König
2026-04-27T07:24:49Z

> Replace all BUG_ON() and BUG() assertions in the gfx_v9_0 ring
> emission paths with WARN_ON_ONCE() and graceful recovery. Ten sites
> are converted across wait_reg_mem, gpu_early_init, parse_ind_reg_list,
> init_rlc_save_restore_list, kiq_read_clock, emit_ib_gfx,
> emit_ib_compute, emit_fence, get_wptr_compute, and set_wptr_compute.
>
> These assertions guard conditions that are either:
> - Address alignment checks on a deprecated byte-swap encoding from
> legacy pre-amdgpu hardware (bits [1:0] must be zero), or
> - Switch-case defaults that should be unreachable but are better
> handled with dev_err + return -EINVAL than a kernel panic.
>
> Several of the address alignment BUG_ON sites in the IB emission
> paths (emit_ib_gfx, emit_ib_compute) are reachable from unprivileged
> userspace via crafted DRM_IOCTL_AMDGPU_CS submissions, causing a
> fatal kernel panic in a scheduler worker thread.
>
> For address checks, clear the reserved bits and proceed. For
> unreachable switch defaults, log the error and return. For the
> doorbell-only wptr paths, log with WARN_ONCE and return zero /
> no-op. Ring emission callbacks return void, so force-aligning
> and proceeding is the accepted pattern.
>
> Found by a custom amdgpu DRM ioctl fuzzer.
>
> Fixes: b1023571479020e9 ("drm/amdgpu: implement GFX 9.0 support (v2)")
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 50 +++++++++++++++++----------
> 1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 2eb32f92a..47e81c33d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1182,8 +1182,8 @@ static void gfx_v9_0_wait_reg_mem(struct amdgpu_ring *ring, int eng_sel,
> WAIT_REG_MEM_FUNCTION(3) | /* equal */
> WAIT_REG_MEM_ENGINE(eng_sel)));
>
> - if (mem_space)
> - BUG_ON(addr0 & 0x3); /* Dword align */
> + if (mem_space && WARN_ON_ONCE(addr0 & 0x3))
> + addr0 &= ~0x3; /* Force dword align */

Same comment as with the SDMA, please use only WARN_ON() and not WARN_ON_ONCE() and don't mask the bits.

> amdgpu_ring_write(ring, addr0);
> amdgpu_ring_write(ring, addr1);
> amdgpu_ring_write(ring, ref);
> @@ -2107,8 +2107,10 @@ static int gfx_v9_0_gpu_early_init(struct amdgpu_device *adev)
> return err;
> break;
> default:
> - BUG();
> - break;
> + dev_err(adev->dev,
> + "unsupported GFX IP version 0x%x for gfx_v9_0\n",
> + amdgpu_ip_version(adev, GC_HWIP, 0));
> + return -EINVAL;

Mhm, that is most likely a bad idea.

The BUG() here is perfectly justified because the system will crash later on anyway and this way we at least stop at the earliest possible time.

Regards,
Christian.

> }
>
> adev->gfx.config.gb_addr_config = gb_addr_config;
> @@ -2808,7 +2810,8 @@ static void gfx_v9_1_parse_ind_reg_list(int *register_list_format,
> break;
> }
>
> - BUG_ON(idx >= unique_indirect_reg_count);
> + if (WARN_ON_ONCE(idx >= unique_indirect_reg_count))
> + break;
>
> if (!unique_indirect_regs[idx])
> unique_indirect_regs[idx] = register_list_format[indirect_offset];
> @@ -2885,7 +2888,8 @@ static int gfx_v9_1_init_rlc_save_restore_list(struct amdgpu_device *adev)
> }
> }
>
> - BUG_ON(j >= unique_indirect_reg_count);
> + if (WARN_ON_ONCE(j >= unique_indirect_reg_count))
> + break;
>
> i++;
> }
> @@ -4209,7 +4213,8 @@ static uint64_t gfx_v9_0_kiq_read_clock(struct amdgpu_device *adev)
> struct amdgpu_kiq *kiq = &adev->gfx.kiq[0];
> struct amdgpu_ring *ring = &kiq->ring;
>
> - BUG_ON(!ring->funcs->emit_rreg);
> + if (WARN_ON_ONCE(!ring->funcs->emit_rreg))
> + return 0;
>
> spin_lock_irqsave(&kiq->ring_lock, flags);
> if (amdgpu_device_wb_get(adev, &reg_val_offs)) {
> @@ -5431,7 +5436,8 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
> }
>
> amdgpu_ring_write(ring, header);
> - BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> + if (WARN_ON_ONCE(ib->gpu_addr & 0x3)) /* Dword align */
> + ib->gpu_addr &= ~0x3ULL;
> amdgpu_ring_write(ring,
> #ifdef __BIG_ENDIAN
> (2 << 0) |
> @@ -5527,7 +5533,8 @@ static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
> }
>
> amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> - BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> + if (WARN_ON_ONCE(ib->gpu_addr & 0x3)) /* Dword align */
> + ib->gpu_addr &= ~0x3ULL;
> amdgpu_ring_write(ring,
> #ifdef __BIG_ENDIAN
> (2 << 0) |
> @@ -5567,10 +5574,13 @@ static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> * the address should be Qword aligned if 64bit write, Dword
> * aligned if only send 32bit data low (discard data high)
> */
> - if (write64bit)
> - BUG_ON(addr & 0x7);
> - else
> - BUG_ON(addr & 0x3);
> + if (write64bit) {
> + if (WARN_ON_ONCE(addr & 0x7))
> + addr &= ~0x7ULL;
> + } else {
> + if (WARN_ON_ONCE(addr & 0x3))
> + addr &= ~0x3ULL;
> + }
> amdgpu_ring_write(ring, lower_32_bits(addr));
> amdgpu_ring_write(ring, upper_32_bits(addr));
> amdgpu_ring_write(ring, lower_32_bits(seq));
> @@ -5639,10 +5649,13 @@ static u64 gfx_v9_0_ring_get_wptr_compute(struct amdgpu_ring *ring)
> u64 wptr;
>
> /* XXX check if swapping is necessary on BE */
> - if (ring->use_doorbell)
> + if (ring->use_doorbell) {
> wptr = atomic64_read((atomic64_t *)ring->wptr_cpu_addr);
> - else
> - BUG();
> + } else {
> + WARN_ONCE(1, "gfx_v9_0: non-doorbell wptr read on ring %s, only doorbell method supported on gfx9\n",
> + ring->name);
> + wptr = 0;
> + }
> return wptr;
> }
>
> @@ -5654,8 +5667,9 @@ static void gfx_v9_0_ring_set_wptr_compute(struct amdgpu_ring *ring)
> if (ring->use_doorbell) {
> atomic64_set((atomic64_t *)ring->wptr_cpu_addr, ring->wptr);
> WDOORBELL64(ring->doorbell_index, ring->wptr);
> - } else{
> - BUG(); /* only DOORBELL method supported on gfx9 now */
> + } else {
> + WARN_ONCE(1, "gfx_v9_0: non-doorbell wptr write on ring %s, only doorbell method supported on gfx9\n",
> + ring->name);
> }
> }
>