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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <670ccea1-5315-bbe5-995f-10aefabf9c28@amd.com>
Date: Wed, 7 May 2025 10:31:25 -0700
From: Lizhi Hou <lizhi.hou@....com>
To: Mario Limonciello <mario.limonciello@....com>, <ogabbay@...nel.org>,
	<quic_jhugo@...cinc.com>, <jacek.lawrynowicz@...ux.intel.com>,
	<dri-devel@...ts.freedesktop.org>
CC: <linux-kernel@...r.kernel.org>, <min.ma@....com>, <max.zhen@....com>,
	<sonal.santan@....com>, <king.tam@....com>
Subject: Re: [PATCH V1] accel/amdxdna: Support submit commands without
 arguments


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@....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@....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,
>