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

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

Merged to drm-misc-next

On 5/7/25 10:31, Lizhi Hou wrote:
>
> 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,
>>