[Public] > -----Original Message----- > From: Lazar, Lijo <lijo.la...@amd.com> > Sent: Wednesday, July 9, 2025 5:02 AM > To: Koenig, Christian <christian.koe...@amd.com>; Deucher, Alexander > <alexander.deuc...@amd.com>; McRae, Geoffrey > <geoffrey.mc...@amd.com>; Kuehling, Felix <felix.kuehl...@amd.com> > Cc: amd-...@lists.freedesktop.org; dri-devel@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.deuc...@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.mc...@amd.com> > >>> *Sent:* Tuesday, July 8, 2025 5:13 AM > >>> *To:* Koenig, Christian <christian.koe...@amd.com>; Kuehling, Felix > >>> <felix.kuehl...@amd.com> > >>> *Cc:* Deucher, Alexander <alexander.deuc...@amd.com>; amd- > >>> g...@lists.freedesktop.org; dri-devel@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.koe...@amd.com > >>> <mailto:christian.koe...@amd.com>> > >>> *Sent:* Tuesday, 8 July 2025 5:01 PM > >>> *To:* McRae, Geoffrey <geoffrey.mc...@amd.com > >>> <mailto:geoffrey.mc...@amd.com>>; Kuehling, Felix > >>> <felix.kuehl...@amd.com <mailto:felix.kuehl...@amd.com>> > >>> *Cc:* Deucher, Alexander <alexander.deuc...@amd.com > >>> <mailto:alexander.deuc...@amd.com>>; amd-...@lists.freedesktop.org > >>> <mailto:amd-...@lists.freedesktop.org> > >>> <amd-...@lists.freedesktop.org > >>> <mailto:amd-...@lists.freedesktop.org>>; > >>> dri-devel@lists.freedesktop.org > >>> <mailto:dri-devel@lists.freedesktop.org> <dri- > >>> de...@lists.freedesktop.org > >>> <mailto:dri-devel@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.mc...@amd.com > >>>> <mailto:geoffrey.mc...@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); > >>>> > >>> > >> > >
