drm/vc4: fix binner-overflow corruption and GPU-reset storms#7509
Open
mairacanal wants to merge 3 commits into
Open
drm/vc4: fix binner-overflow corruption and GPU-reset storms#7509mairacanal wants to merge 3 commits into
mairacanal wants to merge 3 commits into
Conversation
…size vc4_overflow_mem_work() points BPOA at a 512KB slot inside the 16MB binner BO, but writes the size of the whole BO to BPOS. On every binner out-of-memory event the PTB is therefore authorized to write tile lists across all the other slots (which may hold the tile state, tile alloc and overflow memory of in-flight jobs) and, for any slot but the first, past the end of the binner BO into unrelated CMA memory. Since CMA pages are recycled into page cache and user allocations, this is arbitrary memory corruption by GPU DMA. In practice it shows up as GPU hangs with corrupted control list pointers, userspace heap corruption, a GPU that stays permanently wedged after the first hang, and occasional full system crashes, whenever a job overflows the initial binner slot. EmulationStation's screen transitions at 1080p on a Raspberry Pi 3 reproduce it within seconds. The bug dates back to the conversion from a dedicated overflow BO (where writing the full BO size was correct) to the slotted binner BO. Fixes: 553c942 ("drm/vc4: Allow using more than 256MB of CMA memory.") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-4.8 Signed-off-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Although vc4_irq_reset() tears down the in-flight jobs, it never resets the software binner-overflow allocator or the HW overflow registers (BPOA/BPOS). The current overflow slot is tracked in bin_alloc_overflow and is attached to a job's bin_slots at the *next* out-of-memory event. On reset, the RENDER job that would have read it is force-completed, so it is never attached and never freed: a slot leaks on every reset. BPOA/BPOS are also left pointing at the stale overflow slot, so the re-queued BIN job would consume that stale pointer before raising a new OOM interrupt. Free the pending overflow slot, clear bin_alloc_overflow, and zero BPOA/BPOS under job_lock, so the allocator and hardware return to a clean state before submitting new jobs. Fixes: 553c942 ("drm/vc4: Allow using more than 256MB of CMA memory.") Cc: stable@vger.kernel.org Signed-off-by: Maíra Canal <mcanal@igalia.com>
The binner writes each tile's binned command list into an initial tile-allocation block and, when that list outgrows the block, continues it into a chain of continuation blocks. On VC4 the hardware does not link that chain reliably: the render thread that later walks it desyncs -- it has been seen to fetch an invalid opcode partway through a block (CT1CS reports CTERR) or to follow the chain into never-written memory and stall -- and hangcheck then resets the GPU. The chain is only walked once a tile spills its initial block, so the problem scales with how small that block is. The continuation-block size is not the lever: the hang reproduces with both 32- and 128-byte continuation blocks. Enlarging the *initial* block is what helps, as it keeps more tiles' lists in that block and off the chain entirely, and 256 bytes is the hardware maximum. A fixed 256-byte block cannot be used unconditionally, though. The tile_count initial blocks are laid out contiguously right after the tile state and must fit within the binner slot, and the render thread reaches each tile's list by a fixed per-tile stride. If the blocks did not fit, the later ones would land at a non-contiguous address and that stride would point at the wrong memory. So pick the largest initial block (256, 128, 64 or 32 bytes) that still fits the slot for the job's tile count, and program the RCL branch stride to match. This is a mitigation for a hardware limitation, not a complete fix: a single tile whose list exceeds even a 256-byte block, or a scene dense enough to force a smaller initial block, can still reach the chain and hang. But the case users have reported in EmulationStation's 1080p transitions between consoles, which previously stormed "Resetting GPU", now runs with zero resets. Link: raspberrypi#3221 Link: raspberrypi#5780 Fixes: d5b1a78 ("drm/vc4: Add support for drawing 3D frames.") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-4.8 Signed-off-by: Maíra Canal <mcanal@igalia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This series fixes three independent, long-standing bugs in the vc4 binner overflow path that Raspberry Pi 0-3 users have reported as GPU hangs, "[drm] Resetting GPU" storms, and occasional memory corruption or full system crashes.
If possible, please, allow the workflow to run to let users test the kernel.
Moreover, I believe it will be easier for you if I merge this through stable branches instead of merging the PR and then end up needing to revert the downstream patches. I'm just opening the PR to ease user testing. However, if the tree maintainers note some benefit in merging the PR, I don't see a problem.