* Re: [PATCH V1] accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure
2026-03-23 17:37 [PATCH V1] accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure Lizhi Hou
@ 2026-03-23 17:53 ` Karol Wachowski
2026-03-23 17:56 ` Mario Limonciello (AMD) (kernel.org)
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Karol Wachowski @ 2026-03-23 17:53 UTC (permalink / raw)
To: Lizhi Hou, ogabbay, quic_jhugo, dri-devel, mario.limonciello,
maciej.falkowski
Cc: Wendy Liang, linux-kernel, max.zhen, sonal.santan
On 3/23/2026 6:37 PM, Lizhi Hou wrote:
> From: Wendy Liang <wendy.liang@amd.com>
>
> dma_alloc_noncoherent() returns NULL on failure, but callers of
> aie2_alloc_msg_buffer() check for IS_ERR(). Return ERR_PTR(-ENOMEM)
> instead of NULL to match the amdxdna_iommu_alloc() path and the
> caller's error checking convention.
>
> Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via module parameter")
> Signed-off-by: Wendy Liang <wendy.liang@amd.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> ---
> drivers/accel/amdxdna/aie2_message.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
> index 7e219a5eda56..a1c546c3e81c 100644
> --- a/drivers/accel/amdxdna/aie2_message.c
> +++ b/drivers/accel/amdxdna/aie2_message.c
> @@ -56,6 +56,7 @@ void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl *ndev, u32 *size,
> dma_addr_t *dma_addr)
> {
> struct amdxdna_dev *xdna = ndev->xdna;
> + void *vaddr;
> int order;
>
> *size = max(*size, SZ_8K);
> @@ -67,8 +68,12 @@ void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl *ndev, u32 *size,
> if (amdxdna_iova_on(xdna))
> return amdxdna_iommu_alloc(xdna, *size, dma_addr);
>
> - return dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
> + vaddr = dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
> DMA_FROM_DEVICE, GFP_KERNEL);
> + if (!vaddr)
> + return ERR_PTR(-ENOMEM);
> +
> + return vaddr;
> }
>
> void aie2_free_msg_buffer(struct amdxdna_dev_hdl *ndev, size_t size,
Reviewed-by: Karol Wachowski <karol.wachowski@linux.intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH V1] accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure
2026-03-23 17:37 [PATCH V1] accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure Lizhi Hou
2026-03-23 17:53 ` Karol Wachowski
@ 2026-03-23 17:56 ` Mario Limonciello (AMD) (kernel.org)
2026-03-23 20:36 ` Lizhi Hou
2026-03-24 21:38 ` Claude review: " Claude Code Review Bot
2026-03-24 21:38 ` Claude Code Review Bot
3 siblings, 1 reply; 6+ messages in thread
From: Mario Limonciello (AMD) (kernel.org) @ 2026-03-23 17:56 UTC (permalink / raw)
To: Lizhi Hou, ogabbay, quic_jhugo, dri-devel, maciej.falkowski
Cc: Wendy Liang, linux-kernel, max.zhen, sonal.santan
On 3/23/2026 12:37 PM, Lizhi Hou wrote:
> From: Wendy Liang <wendy.liang@amd.com>
>
> dma_alloc_noncoherent() returns NULL on failure, but callers of
> aie2_alloc_msg_buffer() check for IS_ERR(). Return ERR_PTR(-ENOMEM)
> instead of NULL to match the amdxdna_iommu_alloc() path and the
> caller's error checking convention.
>
> Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via module parameter")
> Signed-off-by: Wendy Liang <wendy.liang@amd.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> ---
> drivers/accel/amdxdna/aie2_message.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
> index 7e219a5eda56..a1c546c3e81c 100644
> --- a/drivers/accel/amdxdna/aie2_message.c
> +++ b/drivers/accel/amdxdna/aie2_message.c
> @@ -56,6 +56,7 @@ void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl *ndev, u32 *size,
> dma_addr_t *dma_addr)
> {
> struct amdxdna_dev *xdna = ndev->xdna;
> + void *vaddr;
> int order;
>
> *size = max(*size, SZ_8K);
> @@ -67,8 +68,12 @@ void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl *ndev, u32 *size,
> if (amdxdna_iova_on(xdna))
> return amdxdna_iommu_alloc(xdna, *size, dma_addr);
>
> - return dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
> + vaddr = dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
> DMA_FROM_DEVICE, GFP_KERNEL);
> + if (!vaddr)
> + return ERR_PTR(-ENOMEM);
> +
> + return vaddr;
> }
>
> void aie2_free_msg_buffer(struct amdxdna_dev_hdl *ndev, size_t size,
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH V1] accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure
2026-03-23 17:56 ` Mario Limonciello (AMD) (kernel.org)
@ 2026-03-23 20:36 ` Lizhi Hou
0 siblings, 0 replies; 6+ messages in thread
From: Lizhi Hou @ 2026-03-23 20:36 UTC (permalink / raw)
To: Mario Limonciello (AMD) (kernel.org), ogabbay, quic_jhugo,
dri-devel, maciej.falkowski
Cc: Wendy Liang, linux-kernel, max.zhen, sonal.santan
Applied to drm-misc-next
On 3/23/26 10:56, Mario Limonciello (AMD) (kernel.org) wrote:
>
>
> On 3/23/2026 12:37 PM, Lizhi Hou wrote:
>> From: Wendy Liang <wendy.liang@amd.com>
>>
>> dma_alloc_noncoherent() returns NULL on failure, but callers of
>> aie2_alloc_msg_buffer() check for IS_ERR(). Return ERR_PTR(-ENOMEM)
>> instead of NULL to match the amdxdna_iommu_alloc() path and the
>> caller's error checking convention.
>>
>> Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via
>> module parameter")
>> Signed-off-by: Wendy Liang <wendy.liang@amd.com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>> ---
>> drivers/accel/amdxdna/aie2_message.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_message.c
>> b/drivers/accel/amdxdna/aie2_message.c
>> index 7e219a5eda56..a1c546c3e81c 100644
>> --- a/drivers/accel/amdxdna/aie2_message.c
>> +++ b/drivers/accel/amdxdna/aie2_message.c
>> @@ -56,6 +56,7 @@ void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl
>> *ndev, u32 *size,
>> dma_addr_t *dma_addr)
>> {
>> struct amdxdna_dev *xdna = ndev->xdna;
>> + void *vaddr;
>> int order;
>> *size = max(*size, SZ_8K);
>> @@ -67,8 +68,12 @@ void *aie2_alloc_msg_buffer(struct amdxdna_dev_hdl
>> *ndev, u32 *size,
>> if (amdxdna_iova_on(xdna))
>> return amdxdna_iommu_alloc(xdna, *size, dma_addr);
>> - return dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
>> + vaddr = dma_alloc_noncoherent(xdna->ddev.dev, *size, dma_addr,
>> DMA_FROM_DEVICE, GFP_KERNEL);
>> + if (!vaddr)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + return vaddr;
>> }
>> void aie2_free_msg_buffer(struct amdxdna_dev_hdl *ndev, size_t size,
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure
2026-03-23 17:37 [PATCH V1] accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure Lizhi Hou
2026-03-23 17:53 ` Karol Wachowski
2026-03-23 17:56 ` Mario Limonciello (AMD) (kernel.org)
@ 2026-03-24 21:38 ` Claude Code Review Bot
2026-03-24 21:38 ` Claude Code Review Bot
3 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-24 21:38 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure
Author: Lizhi Hou <lizhi.hou@amd.com>
Patches: 4
Reviewed: 2026-03-25T07:38:18.945083
---
This is a single-patch bug fix for a real NULL-vs-ERR_PTR mismatch in the amdxdna driver. The fix is correct, minimal, and well-targeted. All four callers of `aie2_alloc_msg_buffer()` use `IS_ERR()` to check the return value, but the `dma_alloc_noncoherent()` path returns NULL on failure rather than an ERR_PTR. This means allocation failures on that path would slip past the error check, leading to a NULL pointer dereference. The patch properly converts the NULL to `ERR_PTR(-ENOMEM)`.
**Verdict: Good patch, should be accepted.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure
2026-03-23 17:37 [PATCH V1] accel/amdxdna: Return ERR_PTR on dma_alloc_noncoherent failure Lizhi Hou
` (2 preceding siblings ...)
2026-03-24 21:38 ` Claude review: " Claude Code Review Bot
@ 2026-03-24 21:38 ` Claude Code Review Bot
3 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-24 21:38 UTC (permalink / raw)
To: dri-devel-reviews
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 existing code at lines 63-64 already returns `ERR_PTR(-EINVAL)` for the order check, and line 68 returns the result of `amdxdna_iommu_alloc()` which presumably 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 the `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 `drm_clflush_virt_range(buff_addr, size)`).
**The fix itself:**
```c
+ vaddr = 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 appropriate error code for a DMA allocation failure.
**Minor nits:**
- The Fixes tag references `ece3e8980907` which introduced the `amdxdna_iommu_alloc()` path that created the inconsistency — this is correct.
- The Signed-off-by chain (Wendy Liang as author, Lizhi Hou as submitter) is proper.
**No issues found.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread