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

[Deucher, Alexander]

[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>


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