From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure
Date: Wed, 25 Mar 2026 07:38:19 +1000
Message-ID: <review-patch1-20260323173719.2311474-1-lizhi.hou@amd.com>
In-Reply-To: <20260323173719.2311474-1-lizhi.hou@amd.com>
References: <20260323173719.2311474-1-lizhi.hou@amd.com> <20260323173719.2311474-1-lizhi.hou@amd.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Correctness: Correct.**

The commit message clearly explains the problem: `dma_alloc_noncoherent()` =
returns NULL on failure, but all callers check with `IS_ERR()`. The existin=
g code at lines 63-64 already returns `ERR_PTR(-EINVAL)` for the order chec=
k, and line 68 returns the result of `amdxdna_iommu_alloc()` which presumab=
ly also returns ERR_PTR on failure. Only the `dma_alloc_noncoherent()` path=
 at line 70-71 was inconsistent.

Confirmed all four call sites use `IS_ERR()`:
- `aie2_message.c:399`: `if (IS_ERR(buff_addr))`
- `aie2_message.c:455`: `if (IS_ERR(addr))`
- `aie2_message.c:1206`: `if (IS_ERR(buf))`
- `aie2_error.c:358`: `if (IS_ERR(events->buf))`

Without this fix, a NULL return from `dma_alloc_noncoherent()` would pass t=
he `IS_ERR()` check (since NULL is not in the ERR_PTR range) and lead to a =
NULL pointer dereference when the buffer is subsequently used (e.g., at `dr=
m_clflush_virt_range(buff_addr, size)`).

**The fix itself:**
```c
+	vaddr =3D dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
+				      DMA_FROM_DEVICE, GFP_KERNEL);
+	if (!vaddr)
+		return ERR_PTR(-ENOMEM);
+
+	return vaddr;
```

This is clean and follows the standard kernel pattern. `-ENOMEM` is the app=
ropriate error code for a DMA allocation failure.

**Minor nits:**
- The Fixes tag references `ece3e8980907` which introduced the `amdxdna_iom=
mu_alloc()` path that created the inconsistency =E2=80=94 this is correct.
- The Signed-off-by chain (Wendy Liang as author, Lizhi Hou as submitter) i=
s proper.

**No issues found.**

---
Generated by Claude Code Patch Reviewer