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

[Felix Kuehling]

On 2025-08-05 15:56, Deucher, Alexander wrote:
> [Public]
>
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>> Sent: Wednesday, July 9, 2025 5:02 AM
>> To: Koenig, Christian <Christian.Koenig at amd.com>; Deucher, Alexander
>> <Alexander.Deucher at amd.com>; McRae, Geoffrey
>> <Geoffrey.McRae at amd.com>; Kuehling, Felix <Felix.Kuehling at amd.com>
>> Cc: 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
>>
>>
>>
>> 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.
> In this case, I think -ENOTTY makes sense per the documentation.  Patch is:
> Acked-by: Alex Deucher <alexander.deucher at amd.com>

I agree.

Reviewed-by: Felix Kuehling <felix.kuehling at amd.com>


>
>
>> 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);
>>>>>>