Re: [PATCH V1] accel/amdxdna: Support submit commands without arguments

[Lizhi Hou <lizhi.hou@amd.com>]

On 5/7/25 09:29, Mario Limonciello wrote:
> On 5/7/2025 11:15 AM, Lizhi Hou wrote:
>> The latest userspace runtime allows generating commands which do not
>> have any argument. Remove the corresponding check in driver IOCTL to
>> enable this use case.
>>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>
> Can the userspace handle discovery of the difference?  Or does this 
> need any sort of ABI discovery command introduced too?
>
> The code change itself below looks good to me though.

Runtime will capture and handle the error with current driver.

Even if runtime have a way to explicitly discovery the different, it 
will just report an error and fail the case.


Thanks,

Lizhi

>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>
>> ---
>>   drivers/accel/amdxdna/amdxdna_ctx.c | 22 ++++++++++++----------
>>   1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c 
>> b/drivers/accel/amdxdna/amdxdna_ctx.c
>> index 43442b9e273b..be073224bd69 100644
>> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
>> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
>> @@ -496,11 +496,11 @@ static int amdxdna_drm_submit_execbuf(struct 
>> amdxdna_client *client,
>>                         struct amdxdna_drm_exec_cmd *args)
>>   {
>>       struct amdxdna_dev *xdna = client->xdna;
>> -    u32 *arg_bo_hdls;
>> +    u32 *arg_bo_hdls = NULL;
>>       u32 cmd_bo_hdl;
>>       int ret;
>>   -    if (!args->arg_count || args->arg_count > MAX_ARG_COUNT) {
>> +    if (args->arg_count > MAX_ARG_COUNT) {
>>           XDNA_ERR(xdna, "Invalid arg bo count %d", args->arg_count);
>>           return -EINVAL;
>>       }
>> @@ -512,14 +512,16 @@ static int amdxdna_drm_submit_execbuf(struct 
>> amdxdna_client *client,
>>       }
>>         cmd_bo_hdl = (u32)args->cmd_handles;
>> -    arg_bo_hdls = kcalloc(args->arg_count, sizeof(u32), GFP_KERNEL);
>> -    if (!arg_bo_hdls)
>> -        return -ENOMEM;
>> -    ret = copy_from_user(arg_bo_hdls, u64_to_user_ptr(args->args),
>> -                 args->arg_count * sizeof(u32));
>> -    if (ret) {
>> -        ret = -EFAULT;
>> -        goto free_cmd_bo_hdls;
>> +    if (args->arg_count) {
>> +        arg_bo_hdls = kcalloc(args->arg_count, sizeof(u32), 
>> GFP_KERNEL);
>> +        if (!arg_bo_hdls)
>> +            return -ENOMEM;
>> +        ret = copy_from_user(arg_bo_hdls, u64_to_user_ptr(args->args),
>> +                     args->arg_count * sizeof(u32));
>> +        if (ret) {
>> +            ret = -EFAULT;
>> +            goto free_cmd_bo_hdls;
>> +        }
>>       }
>>         ret = amdxdna_cmd_submit(client, cmd_bo_hdl, arg_bo_hdls,
>