[PATCH v5 0/5] accel/amdxdna: SYNC_BO correctness fixes

[PATCH v5 0/5] accel/amdxdna: SYNC_BO correctness fixes

[Taimuraz Kaitmazov]

Same five fixes as v4, with both review comments addressed.

Changes in v5:
  - patch 3: XDNA_DBG rather than XDNA_ERR, per Lizhi. An unprivileged
    caller can repeat it, which is the same reason patch 1 does not log
    at error level.
  - patch 5: reworked as Lizhi asked. The assigned_hwctx test moves into
    amdxdna_drm_sync_bo_ioctl(), which has the object already, so
    amdxdna_hwctx_sync_debug_bo() keeps -EINVAL for a context that is
    named but gone, and the handle is not resolved twice.
  - patches 1, 2 and 4 carry Lizhi's Reviewed-by, otherwise unchanged.

One behaviour change userspace can see, spelled out in patch 4: SYNC_BO
answers -EOPNOTSUPP for an imported BO, which on a carveout device is every
AMDXDNA_BO_DEV.

Patch 4 still needs "accel/amdxdna: return early from a zero-length
flush", which is now in drm-misc-fixes, so it wants that backmerged into
drm-misc-next. Patches 1, 2, 3 and 5 apply without it.

  https://lore.kernel.org/all/20260817230655.356785-1-taimuraz@kaitmazov.com/

v4: https://lore.kernel.org/all/20260817230707.356828-1-taimuraz@kaitmazov.com/

Built on drm-misc-next plus that commit, each commit on its own: x86_64
with DRM_ACCEL_AMDXDNA=m, clang 22.1.8, W=1, no warnings, checkpatch
--strict clean.

Taimuraz Kaitmazov (5):
  accel/amdxdna: refuse an I/O memory mapping of an imported BO
  accel/amdxdna: check the sync range for overflow on a device BO
  accel/amdxdna: do not warn when a sync request is rejected
  accel/amdxdna: refuse to flush an imported BO
  accel/amdxdna: do not fail a sync for a BO with no debug context

 drivers/accel/amdxdna/amdxdna_gem.c | 30 ++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

-- 
2.55.0

[PATCH v5 1/5] accel/amdxdna: refuse an I/O memory mapping of an imported BO

[Taimuraz Kaitmazov]

amdxdna_gem_vmap() flattens the iosys_map drm_gem_vmap() fills in down to
the void * in abo->mem.kva, and iosys_map is discriminated by is_iomem, so
an exporter answering with an I/O mapping leaves a void __iomem pointer
there, which amdxdna_cmd_set_error() memsets and memcpys through.

amdxdna_drm_va_tbl takes a dmabuf_fd, so such a BO can be any exporter's
buffer. amdgpu cannot reach this: its .pin forces GTT for a non peer to
peer attachment like ours. An exporter on drm_gem_prime_dmabuf_ops has
no .pin, and drm_gem_ttm_vmap() answers iomem for a VRAM resident
object, so an NPU paired with nouveau or radeon does.

Drop such a mapping and answer NULL. Checking here rather than in the
.vmap callback leaves that callback's iosys_map contract intact for a
caller equipped to read I/O memory, and covers everything that takes a
plain kernel address through this helper. vmw_gem_vmap() refuses the
same case; unlike that one this path is reachable from an unprivileged
ioctl, so it neither warns nor logs at error level.

Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index cca84fa07e9d..f88b5349cd4b 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -209,10 +209,15 @@ void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo)
 
 	if (!abo->mem.kva) {
 		ret = drm_gem_vmap(to_gobj(abo), &map);
-		if (ret)
+		if (ret) {
 			XDNA_ERR(abo->client->xdna, "Vmap bo failed, ret %d", ret);
-		else
+		} else if (map.is_iomem) {
+			/* Callers use the result as an ordinary kernel address. */
+			XDNA_DBG(abo->client->xdna, "Vmap bo returned I/O memory");
+			drm_gem_vunmap(to_gobj(abo), &map);
+		} else {
 			abo->mem.kva = map.vaddr;
+		}
 	}
 	return abo->mem.kva;
 }
-- 
2.55.0

[PATCH v5 2/5] accel/amdxdna: check the sync range for overflow on a device BO

[Taimuraz Kaitmazov]

amdxdna_drm_sync_bo_ioctl() forms the range for a device BO by adding the
caller's offset and size to the BO address without checking either, while
amdxdna_flush_bo() one call down guards the same arithmetic with
check_add_overflow().

A size that wraps flush_end leaves it below the heap it is clamped
against, so every heap fails the start >= end test, and a sync that asked
for more than the address space holds reports success having flushed
nothing. Reject it instead.

Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index f88b5349cd4b..77a9493cd7ba 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -1274,8 +1274,13 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device *dev,
 		struct amdxdna_gem_obj *heap;
 		unsigned long heap_id;
 		u64 bo_start = amdxdna_gem_dev_addr(abo);
-		u64 flush_start = bo_start + args->offset;
-		u64 flush_end = flush_start + args->size;
+		u64 flush_start, flush_end;
+
+		if (check_add_overflow(bo_start, args->offset, &flush_start) ||
+		    check_add_overflow(flush_start, args->size, &flush_end)) {
+			ret = -EINVAL;
+			goto put_obj;
+		}
 
 		xa_for_each_range(&client->dev_heap_xa, heap_id, heap,
 				  abo->heap_start_id, abo->heap_end_id) {
-- 
2.55.0

[PATCH v5 3/5] accel/amdxdna: do not warn when a sync request is rejected

[Taimuraz Kaitmazov]

amdxdna_drm_sync_bo_ioctl() answers a failed amdxdna_flush_bo() with
drm_WARN(). Both of that function's error returns are decided by the
ioctl's arguments, so SYNC_BO with an offset past the end of the BO
splats and taints the kernel from an unprivileged caller.

Log it at debug level, since the same caller can repeat it.

Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 77a9493cd7ba..0e0f844526ca 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -1310,7 +1310,7 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device *dev,
 		amdxdna_gem_unpin(abo);
 
 		if (ret) {
-			drm_WARN(&xdna->ddev, 1, "Can not get flush memory");
+			XDNA_DBG(xdna, "Flush BO %d failed, ret %d", args->handle, ret);
 			goto put_obj;
 		}
 	}
-- 
2.55.0

[PATCH v5 4/5] accel/amdxdna: refuse to flush an imported BO

[Taimuraz Kaitmazov]

SYNC_BO clflushes an imported BO's scatterlist. An importer may not do
that: the memory belongs to the exporter, and dma-buf gives the importer
no interface to ask for maintenance on it. Refuse the request instead.

is_import_bo() is (obj)->attach, which covers more than foreign buffers.
A userptr BO arrives through a ubuf, and on a carveout device every share
BO and the device heap arrive through a cbuf, so SYNC_BO answers
-EOPNOTSUPP for those too, including the AMDXDNA_BO_DEV path that flushes
through its heap.

Only the ubuf case gives up maintenance it was getting: on a 64 MiB
userptr BO a 4 KiB sync and a full sync both cost 659 us, this arm having
ignored the range. amdxdna_cbuf_map() fills in only the DMA address and
length, so drm_clflush_sg() already walks zero pages on carveout memory.
Userspace maintains these through the mapping it already holds, as XRT's
buffer::sync() does unless it is told to sync through the driver.

Suggested-by: Lizhi Hou <lizhi.hou@amd.com>
Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 0e0f844526ca..4be5298d1062 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -1224,6 +1224,9 @@ static int amdxdna_flush_bo(struct amdxdna_gem_obj *abo, u64 offset, u64 size)
 {
 	u64 end;
 
+	if (is_import_bo(abo))
+		return -EOPNOTSUPP;
+
 	if (offset >= abo->mem.size)
 		return -EINVAL;
 
@@ -1234,9 +1237,7 @@ static int amdxdna_flush_bo(struct amdxdna_gem_obj *abo, u64 offset, u64 size)
 	if (!size)
 		return 0;
 
-	if (is_import_bo(abo))
-		drm_clflush_sg(abo->base.sgt);
-	else if (amdxdna_gem_vmap(abo))
+	if (amdxdna_gem_vmap(abo))
 		drm_clflush_virt_range(amdxdna_gem_vmap(abo) + offset, size);
 	else if (abo->base.pages)
 		drm_clflush_pages(abo->base.pages, abo->mem.size >> PAGE_SHIFT);
-- 
2.55.0

[PATCH v5 5/5] accel/amdxdna: do not fail a sync for a BO with no debug context

[Taimuraz Kaitmazov]

amdxdna_drm_sync_bo_ioctl() calls amdxdna_hwctx_sync_debug_bo() for every
FROM_DEVICE sync, which answers -EINVAL when the BO's assigned_hwctx names
no context. Only a BO attached with ATTACH_DEBUG_BO is ever given one, so
an ordinary read-back sync reports failure after its flush has already run.

Ask for the debug sync only when the BO has a context. An unattached BO
carries AMDXDNA_INVALID_CTX_HANDLE and hwctx ids are allocated above it, so
the test is exact, -EINVAL keeps meaning that the named context is gone,
and the handle is not resolved twice. The field is written under dev_lock
and read here without it; the context is still resolved under that lock, so
a racing attach only decides whether this sync sees the buffer.

Suggested-by: Lizhi Hou <lizhi.hou@amd.com>
Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 4be5298d1062..2613c94dd842 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -1319,7 +1319,8 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device *dev,
 	XDNA_DBG(xdna, "Sync bo %d offset 0x%llx, size 0x%llx\n",
 		 args->handle, args->offset, args->size);
 
-	if (args->direction == SYNC_DIRECT_FROM_DEVICE)
+	if (abo->assigned_hwctx != AMDXDNA_INVALID_CTX_HANDLE &&
+	    args->direction == SYNC_DIRECT_FROM_DEVICE)
 		ret = amdxdna_hwctx_sync_debug_bo(client, args->handle);
 
 put_obj:
-- 
2.55.0

Re: [PATCH v5 4/5] accel/amdxdna: refuse to flush an imported BO

[Lizhi Hou]

Applied this to drm-misc-fixes.

On 8/19/26 15:44, Taimuraz Kaitmazov wrote:
> SYNC_BO clflushes an imported BO's scatterlist. An importer may not do
> that: the memory belongs to the exporter, and dma-buf gives the importer
> no interface to ask for maintenance on it. Refuse the request instead.
>
> is_import_bo() is (obj)->attach, which covers more than foreign buffers.
> A userptr BO arrives through a ubuf, and on a carveout device every share
> BO and the device heap arrive through a cbuf, so SYNC_BO answers
> -EOPNOTSUPP for those too, including the AMDXDNA_BO_DEV path that flushes
> through its heap.
>
> Only the ubuf case gives up maintenance it was getting: on a 64 MiB
> userptr BO a 4 KiB sync and a full sync both cost 659 us, this arm having
> ignored the range. amdxdna_cbuf_map() fills in only the DMA address and
> length, so drm_clflush_sg() already walks zero pages on carveout memory.
> Userspace maintains these through the mapping it already holds, as XRT's
> buffer::sync() does unless it is told to sync through the driver.
>
> Suggested-by: Lizhi Hou <lizhi.hou@amd.com>
> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
> Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 0e0f844526ca..4be5298d1062 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -1224,6 +1224,9 @@ static int amdxdna_flush_bo(struct amdxdna_gem_obj *abo, u64 offset, u64 size)
>   {
>   	u64 end;
>   
> +	if (is_import_bo(abo))
> +		return -EOPNOTSUPP;
> +
>   	if (offset >= abo->mem.size)
>   		return -EINVAL;
>   
> @@ -1234,9 +1237,7 @@ static int amdxdna_flush_bo(struct amdxdna_gem_obj *abo, u64 offset, u64 size)
>   	if (!size)
>   		return 0;
>   
> -	if (is_import_bo(abo))
> -		drm_clflush_sg(abo->base.sgt);
> -	else if (amdxdna_gem_vmap(abo))
> +	if (amdxdna_gem_vmap(abo))
>   		drm_clflush_virt_range(amdxdna_gem_vmap(abo) + offset, size);
>   	else if (abo->base.pages)
>   		drm_clflush_pages(abo->base.pages, abo->mem.size >> PAGE_SHIFT);

Re: [PATCH v5 3/5] accel/amdxdna: do not warn when a sync request is rejected

[Lizhi Hou]

On 8/19/26 15:44, Taimuraz Kaitmazov wrote:
> amdxdna_drm_sync_bo_ioctl() answers a failed amdxdna_flush_bo() with
> drm_WARN(). Both of that function's error returns are decided by the
> ioctl's arguments, so SYNC_BO with an offset past the end of the BO
> splats and taints the kernel from an unprivileged caller.
>
> Log it at debug level, since the same caller can repeat it.
>
> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 77a9493cd7ba..0e0f844526ca 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -1310,7 +1310,7 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device *dev,
>   		amdxdna_gem_unpin(abo);
>   
>   		if (ret) {
> -			drm_WARN(&xdna->ddev, 1, "Can not get flush memory");
> +			XDNA_DBG(xdna, "Flush BO %d failed, ret %d", args->handle, ret);
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>   			goto put_obj;
>   		}
>   	}

Re: [PATCH v5 5/5] accel/amdxdna: do not fail a sync for a BO with no debug context

[Lizhi Hou]

On 8/19/26 15:44, Taimuraz Kaitmazov wrote:
> amdxdna_drm_sync_bo_ioctl() calls amdxdna_hwctx_sync_debug_bo() for every
> FROM_DEVICE sync, which answers -EINVAL when the BO's assigned_hwctx names
> no context. Only a BO attached with ATTACH_DEBUG_BO is ever given one, so
> an ordinary read-back sync reports failure after its flush has already run.
>
> Ask for the debug sync only when the BO has a context. An unattached BO
> carries AMDXDNA_INVALID_CTX_HANDLE and hwctx ids are allocated above it, so
> the test is exact, -EINVAL keeps meaning that the named context is gone,
> and the handle is not resolved twice. The field is written under dev_lock
> and read here without it; the context is still resolved under that lock, so
> a racing attach only decides whether this sync sees the buffer.
>
> Suggested-by: Lizhi Hou <lizhi.hou@amd.com>
> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 4be5298d1062..2613c94dd842 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -1319,7 +1319,8 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device *dev,
>   	XDNA_DBG(xdna, "Sync bo %d offset 0x%llx, size 0x%llx\n",
>   		 args->handle, args->offset, args->size);
>   
> -	if (args->direction == SYNC_DIRECT_FROM_DEVICE)
> +	if (abo->assigned_hwctx != AMDXDNA_INVALID_CTX_HANDLE &&
> +	    args->direction == SYNC_DIRECT_FROM_DEVICE)
>   		ret = amdxdna_hwctx_sync_debug_bo(client, args->handle);
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>   
>   put_obj:

Re: [PATCH v5 3/5] accel/amdxdna: do not warn when a sync request is rejected

[Lizhi Hou]

Applied to drm-misc-next

On 9/17/26 08:55, Lizhi Hou wrote:
>
> On 8/19/26 15:44, Taimuraz Kaitmazov wrote:
>> amdxdna_drm_sync_bo_ioctl() answers a failed amdxdna_flush_bo() with
>> drm_WARN(). Both of that function's error returns are decided by the
>> ioctl's arguments, so SYNC_BO with an offset past the end of the BO
>> splats and taints the kernel from an unprivileged caller.
>>
>> Log it at debug level, since the same caller can repeat it.
>>
>> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
>> ---
>>   drivers/accel/amdxdna/amdxdna_gem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c 
>> b/drivers/accel/amdxdna/amdxdna_gem.c
>> index 77a9493cd7ba..0e0f844526ca 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.c
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
>> @@ -1310,7 +1310,7 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device 
>> *dev,
>>           amdxdna_gem_unpin(abo);
>>             if (ret) {
>> -            drm_WARN(&xdna->ddev, 1, "Can not get flush memory");
>> +            XDNA_DBG(xdna, "Flush BO %d failed, ret %d", 
>> args->handle, ret);
> Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>>               goto put_obj;
>>           }
>>       }

Re: [PATCH v5 5/5] accel/amdxdna: do not fail a sync for a BO with no debug context

[Lizhi Hou]

Applied to drm-misc-next

On 9/17/26 09:11, Lizhi Hou wrote:
>
> On 8/19/26 15:44, Taimuraz Kaitmazov wrote:
>> amdxdna_drm_sync_bo_ioctl() calls amdxdna_hwctx_sync_debug_bo() for 
>> every
>> FROM_DEVICE sync, which answers -EINVAL when the BO's assigned_hwctx 
>> names
>> no context. Only a BO attached with ATTACH_DEBUG_BO is ever given 
>> one, so
>> an ordinary read-back sync reports failure after its flush has 
>> already run.
>>
>> Ask for the debug sync only when the BO has a context. An unattached BO
>> carries AMDXDNA_INVALID_CTX_HANDLE and hwctx ids are allocated above 
>> it, so
>> the test is exact, -EINVAL keeps meaning that the named context is gone,
>> and the handle is not resolved twice. The field is written under 
>> dev_lock
>> and read here without it; the context is still resolved under that 
>> lock, so
>> a racing attach only decides whether this sync sees the buffer.
>>
>> Suggested-by: Lizhi Hou <lizhi.hou@amd.com>
>> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
>> ---
>>   drivers/accel/amdxdna/amdxdna_gem.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c 
>> b/drivers/accel/amdxdna/amdxdna_gem.c
>> index 4be5298d1062..2613c94dd842 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.c
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
>> @@ -1319,7 +1319,8 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device 
>> *dev,
>>       XDNA_DBG(xdna, "Sync bo %d offset 0x%llx, size 0x%llx\n",
>>            args->handle, args->offset, args->size);
>>   -    if (args->direction == SYNC_DIRECT_FROM_DEVICE)
>> +    if (abo->assigned_hwctx != AMDXDNA_INVALID_CTX_HANDLE &&
>> +        args->direction == SYNC_DIRECT_FROM_DEVICE)
>>           ret = amdxdna_hwctx_sync_debug_bo(client, args->handle);
> Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>>     put_obj: