[PATCH v2 1/1] drm/amdkfd: return -ENOTTY for unsupported IOCTLs

[Lazar, Lijo]

On 7/9/2025 2:09 PM, Christian König wrote:
> On 09.07.25 06:56, Lazar, Lijo wrote:
>> On 7/8/2025 8:40 PM, Deucher, Alexander wrote:
>>> [Public]
>>>
>>>
>>> I seem to recall -ENOTSUPP being frowned upon for IOCTLs.
>>>
>>>
>> Going by documentation -
>> https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html
>>
> 
> Good point.
> 
>> EOPNOTSUPP:
>> Feature (like PRIME, modesetting, GEM) is not supported by the driver.
>>
>> "Note that ENOTTY has the slightly unintuitive meaning of “this IOCTL
>> does not exist”, and is used exactly as such in DRM"
>>  
>> Since KFD ioctls could eventually be supported in drm node,
> 
> That's certainly not going to happen.
> 
> We are currently in the process of deprecating the KFD IOCTLs and either using the existing DRM render node ones or coming up with new IOCTL/additions to the existing ones.
> 

I really meant to convey this to justify using drm documentation as the
background for picking error codes for KFD ones also. At least for any
new error code returns, definitions will remain consistent across both.

Thanks,
Lijo

> Background is that some of the KFD IOCTLs have design flaws which are unfix able.
> 
> Regards,
> Christian.
> 
>> it seems
>> better to go with ENOTTY.
>>
>> Thanks,
>> Lijo
>>
>>>
>>> Alex
>>>
>>>  
>>>
>>> *From:*McRae, Geoffrey <Geoffrey.McRae at amd.com>
>>> *Sent:* Tuesday, July 8, 2025 5:13 AM
>>> *To:* Koenig, Christian <Christian.Koenig at amd.com>; Kuehling, Felix
>>> <Felix.Kuehling at amd.com>
>>> *Cc:* Deucher, Alexander <Alexander.Deucher at amd.com>; amd-
>>> gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
>>> *Subject:* Re: [PATCH v2 1/1] drm/amdkfd: return -ENOTTY for unsupported
>>> IOCTLs
>>>
>>>  
>>>
>>> [AMD Official Use Only - AMD Internal Distribution Only]
>>>
>>>  
>>>
>>> I am happy to use EOPNOTSUPP but I must point out that this is not the
>>> pattern used across the kernel, the standard is to use ENOTTY, which is
>>> also the default that fs/ioctl.c returns when no handler is present.
>>> Userspace tooling such as strace and glibc specifically expectect ENOTTY
>>> to indicate invalid or unsupported IOCTL.
>>>
>>> ------------------------------------------------------------------------
>>>
>>> *From:*Koenig, Christian <Christian.Koenig at amd.com
>>> <mailto:Christian.Koenig at amd.com>>
>>> *Sent:* Tuesday, 8 July 2025 5:01 PM
>>> *To:* McRae, Geoffrey <Geoffrey.McRae at amd.com
>>> <mailto:Geoffrey.McRae at amd.com>>; Kuehling, Felix
>>> <Felix.Kuehling at amd.com <mailto:Felix.Kuehling at amd.com>>
>>> *Cc:* Deucher, Alexander <Alexander.Deucher at amd.com
>>> <mailto:Alexander.Deucher at amd.com>>; amd-gfx at lists.freedesktop.org
>>> <mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org
>>> <mailto:amd-gfx at lists.freedesktop.org>>; dri-devel at lists.freedesktop.org
>>> <mailto:dri-devel at lists.freedesktop.org> <dri-
>>> devel at lists.freedesktop.org <mailto:dri-devel at lists.freedesktop.org>>
>>> *Subject:* Re: [PATCH v2 1/1] drm/amdkfd: return -ENOTTY for unsupported
>>> IOCTLs
>>>
>>>  
>>>
>>> On 08.07.25 06:22, Geoffrey McRae wrote:
>>>> Some kfd ioctls may not be available depending on the kernel version the
>>>> user is running, as such we need to report -ENOTTY so userland can
>>>> determine the cause of the ioctl failure.
>>>
>>> In general sounds like a good idea, but ENOTTY is potentially a bit
>>> misleading.
>>>
>>> We usually use EOPNOTSUPP for that even if its not the original meaning
>>> of that error code.
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Signed-off-by: Geoffrey McRae <geoffrey.mcrae at amd.com <mailto:geoffrey.mcrae at amd.com>>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 8 ++++++--
>>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>>> index a2149afa5803..36396b7318e7 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>>> @@ -3253,8 +3253,10 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>>>>         int retcode = -EINVAL;
>>>>         bool ptrace_attached = false;
>>>>   
>>>> -     if (nr >= AMDKFD_CORE_IOCTL_COUNT)
>>>> +     if (nr >= AMDKFD_CORE_IOCTL_COUNT) {
>>>> +             retcode = -ENOTTY;
>>>>                 goto err_i1;
>>>> +     }
>>>>   
>>>>         if ((nr >= AMDKFD_COMMAND_START) && (nr < AMDKFD_COMMAND_END)) {
>>>>                 u32 amdkfd_size;
>>>> @@ -3267,8 +3269,10 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>>>>                         asize = amdkfd_size;
>>>>   
>>>>                 cmd = ioctl->cmd;
>>>> -     } else
>>>> +     } else {
>>>> +             retcode = -ENOTTY;
>>>>                 goto err_i1;
>>>> +     }
>>>>   
>>>>         dev_dbg(kfd_device, "ioctl cmd 0x%x (#0x%x), arg 0x%lx\n", cmd, nr, arg);
>>>>   
>>>
>>
>