[PATCH] accel/qaic: Fix dma_free_attrs() buffer size

[PATCH] accel/qaic: Fix dma_free_attrs() buffer size

[Thomas Fourier]

The gpummu->table buffer is alloc'd with size TABLE_SIZE + 32 in
a2xx_gpummu_new() but freed with size TABLE_SIZE in
a2xx_gpummu_destroy().

Change the free size to match the allocation.

Fixes: c2052a4e5c99 ("drm/msm: implement a2xx mmu")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
 drivers/gpu/drm/msm/adreno/a2xx_gpummu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpummu.c b/drivers/gpu/drm/msm/adreno/a2xx_gpummu.c
index 0407c9bc8c1b..4467b04527cd 100644
--- a/drivers/gpu/drm/msm/adreno/a2xx_gpummu.c
+++ b/drivers/gpu/drm/msm/adreno/a2xx_gpummu.c
@@ -78,7 +78,7 @@ static void a2xx_gpummu_destroy(struct msm_mmu *mmu)
 {
 	struct a2xx_gpummu *gpummu = to_a2xx_gpummu(mmu);
 
-	dma_free_attrs(mmu->dev, TABLE_SIZE, gpummu->table, gpummu->pt_base,
+	dma_free_attrs(mmu->dev, TABLE_SIZE + 32, gpummu->table, gpummu->pt_base,
 		DMA_ATTR_FORCE_CONTIGUOUS);
 
 	kfree(gpummu);
-- 
2.43.0

Claude review: accel/qaic: Fix dma_free_attrs() buffer size

[Claude Code Review Bot]

Patch Review

**Subject Line Issue:**
The subject prefix is incorrect. The patch modifies `drivers/gpu/drm/msm/adreno/a2xx_gpummu.c`, which is the MSM Adreno driver, not the QAIC accelerator driver. The subject should be:
```
drm/msm/a2xx: Fix dma_free_attrs() buffer size
```

**Commit Message:**
The commit message clearly describes the problem:
```
The gpummu->table buffer is alloc'd with size TABLE_SIZE + 32 in
a2xx_gpummu_new() but freed with size TABLE_SIZE in
a2xx_gpummu_destroy().
```

However, it would be helpful to explain:
1. Why the extra 32 bytes are allocated
2. What the potential impact of this mismatch is (memory leak, corruption, etc.)
3. Whether this has been observed to cause issues in practice

**Technical Analysis:**

The change itself:
```c
-	dma_free_attrs(mmu->dev, TABLE_SIZE, gpummu->table, gpummu->pt_base,
+	dma_free_attrs(mmu->dev, TABLE_SIZE + 32, gpummu->table, gpummu->pt_base,
 		DMA_ATTR_FORCE_CONTIGUOUS);
```

**Questions requiring verification:**

1. **Allocation site verification:** The commit message claims the allocation in `a2xx_gpummu_new()` uses `TABLE_SIZE + 32`. This should be verified by examining that function to ensure the sizes truly match.

2. **Purpose of +32 bytes:** The extra 32 bytes suggests alignment padding or guard space. Without seeing the allocation code, it's unclear if:
   - This is for cache line alignment (32 bytes = typical cache line size)
   - This is guard space for overflow detection
   - This is for some hardware-specific requirement

3. **TABLE_SIZE definition:** What is the value of TABLE_SIZE? Is it already aligned, or does the +32 ensure proper alignment?

4. **DMA API behavior:** While the DMA API typically requires matching allocation/free sizes, the behavior with mismatched sizes may vary by architecture. The impact should be documented.

**Potential Issues:**

1. **Memory leak:** Depending on the DMA allocator implementation, freeing with a smaller size than allocated could leak the remaining bytes (unlikely but possible).

2. **Reference counting:** If the DMA layer tracks allocations by size, this mismatch could cause issues.

3. **Debugging complications:** Memory debugging tools (KASAN, KMSAN) may flag this mismatch.

**Missing Context:**

To fully validate this patch, reviewers need to see:
```c
// From a2xx_gpummu_new() - allocation site
gpummu->table = dma_alloc_attrs(dev, TABLE_SIZE + 32, &gpummu->pt_base, 
                                GFP_KERNEL, DMA_ATTR_FORCE_CONTIGUOUS);
```

**Fixes Tag:**
```
Fixes: c2052a4e5c99 ("drm/msm: implement a2xx mmu")
```
The Fixes tag is present and appears reasonable. The commit should be verified to exist in the tree.

**Recommendations:**

1. **Correct the subject line** to use `drm/msm/a2xx:` prefix instead of `accel/qaic:`

2. **Enhance the commit message** to explain:
   - Why +32 bytes (alignment requirement?)
   - Potential impact (leak, corruption, debug tool warnings?)
   - How this was discovered (code inspection, tool report, bug observed?)

3. **Verify the allocation** by including a reference to the allocation code or confirming line numbers

4. **Consider adding a comment** in the code explaining the +32 bytes:
   ```c
   /* TABLE_SIZE + 32 for cache line alignment */
   ```

**Code correctness:** If the allocation truly uses `TABLE_SIZE + 32`, then this fix is correct and necessary.

**Testing needed:**
- Verify no memory leaks with kmemleak
- Test on actual A2XX hardware if available
- Run with KASAN/KMSAN to ensure no warnings

**Verdict:** The fix appears correct but the patch needs revision for the incorrect subject line. The commit message could be more detailed about the rationale and impact.

---
Generated by Claude Code Patch Reviewer

Claude review: accel/qaic: Fix dma_free_attrs() buffer size

[Claude Code Review Bot]

Overall Series Review

Subject: accel/qaic: Fix dma_free_attrs() buffer size
Author: Thomas Fourier <fourier.thomas@gmail.com>
Patches: 1
Reviewed: 2026-02-11T16:32:49.659898

---

This is a single-patch series fixing a buffer size mismatch in the MSM Adreno A2XX GPU MMU driver. The patch addresses a memory management bug where the buffer allocation size differs from the deallocation size, which could lead to memory leaks or corruption.

**Series Assessment:**
- **Severity:** Medium - Memory management bugs can cause resource leaks or undefined behavior
- **Correctness:** The fix appears correct based on the code context
- **Scope:** Minimal and focused - single line change
- **Concerns:** Need to verify the allocation context and whether the +32 bytes serves a specific purpose

The patch is tagged for stable backport and includes proper Fixes tag, which is appropriate for this type of bug fix.

---
Generated by Claude Code Patch Reviewer