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

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 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]

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 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: