On 2025-08-05 15:56, Deucher, Alexander wrote: > [Public] > >> -----Original Message----- >> From: Lazar, Lijo <[email protected]> >> Sent: Wednesday, July 9, 2025 5:02 AM >> To: Koenig, Christian <[email protected]>; Deucher, Alexander >> <[email protected]>; McRae, Geoffrey >> <[email protected]>; Kuehling, Felix <[email protected]> >> Cc: [email protected]; [email protected] >> 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 <[email protected]>
I agree. Reviewed-by: Felix Kuehling <[email protected]> > > >> 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 <[email protected]> >>>>> *Sent:* Tuesday, July 8, 2025 5:13 AM >>>>> *To:* Koenig, Christian <[email protected]>; Kuehling, Felix >>>>> <[email protected]> >>>>> *Cc:* Deucher, Alexander <[email protected]>; amd- >>>>> [email protected]; [email protected] >>>>> *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 <[email protected] >>>>> <mailto:[email protected]>> >>>>> *Sent:* Tuesday, 8 July 2025 5:01 PM >>>>> *To:* McRae, Geoffrey <[email protected] >>>>> <mailto:[email protected]>>; Kuehling, Felix >>>>> <[email protected] <mailto:[email protected]>> >>>>> *Cc:* Deucher, Alexander <[email protected] >>>>> <mailto:[email protected]>>; [email protected] >>>>> <mailto:[email protected]> >>>>> <[email protected] >>>>> <mailto:[email protected]>>; >>>>> [email protected] >>>>> <mailto:[email protected]> <dri- >>>>> [email protected] >>>>> <mailto:[email protected]>> >>>>> *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 <[email protected] >>>>>> <mailto:[email protected]>> >>>>>> --- >>>>>> 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); >>>>>>
