[RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[Emil Velikov]

On 20 August 2016 at 23:31, Rob Clark <robdclark@gmail.com> wrote:
> On Sat, Aug 20, 2016 at 1:58 PM, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
>> Cc'ing lkml too.
>>
>> On Fri, Aug 19, 2016 at 11:54:21PM +0100, Emil Velikov wrote:
>>> Story time:
>>> I was dreaming of a day were we can stop installing these headers,
>>> thus making deprecation a bit easier process.
>>> Yet after failing to convince Dave and Daniel on a number of occasions
>>> I've accepted that those headers _are_ here to stay. And yes they
>>> _are_ the UAPI, even though no applications are meant to use them but
>>> the libdrm 'version'.
>>> Thus any changes to the libdrm ones should be a mirror of the ones
>>> here and libdrm should _not_ differ.
>>
>> Another day dream:
>>
>> Wouldn't it be nice if the uapi headers from Linux kernel would pass
>> a simple quality check of compiling in userspace where they are meant to be
>> used? Stand alone. Without magic tricks and additional libraries and their
>> headers. Without glibc or any other libc implementation specific additions.
>> The uapi headers define many parts of the Linux kernel API and ABI, and thus
>> compiling them also without the 'official' GNU/Linux userspace libraries
>> like glibc or libdrm does have some uses. For example API and ABI
>> compatibility checks and API/ABI/system call fuzzers.
>>
>> Many headers required stdint.h types but Linux kernel headers do not
>> define them in userspace, and then Linus has said that uapi headers
>> should use the linux/types.h with double underscores. Thus my patches
>> for fixing trivial compile errors turned into changing several stdint.h
>> definitions to linux/types.h.
>
> The problem is, for the most part, the driver specific gpu related
> ioctl interfaces are not intended for general public consumption.
> They have one consumer, ie. libdrm_$drivername (or perhaps mesa
> directly).  They are complex interfaces, because GPUs are complex.
> They are not intended to be used directly (or for the most part, even
> indirectly) by random userspace applications.  And in fact the uapi
> headers exported from kernel are not actually ever used.  (ie.
> libdrm_$drivername uses it's own copy internally within libdrm.)
>
> So Linus's argument against stdint types, as weak as it is, doesn't
> even apply for gpu driver specific ioctls.
>
Although last time around people leaned towards the __uX types, if we
have a consensus amongst drm (kernel) developers about using stdint
ones everything should be fine.
We just need a handful of acks from the different maintainers.

That said, _note_ that some applications are built with -C89 -pedantic
[1] which means that using stdint.h may or may not work as expected.
So we'll want a __STDC_VESION__ check + #error in case of pre-C99 ?
If the affected programs are proprietary ones we should be safe,
otherwise we want to update them ~alongside the transition.

Thanks
Emil

[1]
https://cgit.freedesktop.org/mesa/drm/commit/?id=0f4452bb51306024fbf4cbf77d8baab20cefba67
https://cgit.freedesktop.org/mesa/drm/commit/?id=d20314d083e533e3b8753192b1846752341afbbe

Re: [RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[Christian König]

Am 22.08.2016 um 10:48 schrieb Emil Velikov:
>
> Although last time around people leaned towards the __uX types, if we
> have a consensus amongst drm (kernel) developers about using stdint
> ones everything should be fine.
> We just need a handful of acks from the different maintainers.

For the record I always clearly voted for the C99 stdint types instead 
of the kernel ones.

> That said, _note_ that some applications are built with -C89 -pedantic
> [1] which means that using stdint.h may or may not work as expected.
> So we'll want a __STDC_VESION__ check + #error in case of pre-C99 ?
> If the affected programs are proprietary ones we should be safe,
> otherwise we want to update them ~alongside the transition.

While it is theoretical possible I don't think we have any applications 
which actually do this, cause they would have been broken before as well.

Regards,
Christian.

>
> Thanks
> Emil
>
> [1]
> https://cgit.freedesktop.org/mesa/drm/commit/?id=0f4452bb51306024fbf4cbf77d8baab20cefba67
> https://cgit.freedesktop.org/mesa/drm/commit/?id=d20314d083e533e3b8753192b1846752341afbbe
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[Mikko Rapeli]

On Mon, Aug 22, 2016 at 09:48:10AM +0100, Emil Velikov wrote:
> On 20 August 2016 at 23:31, Rob Clark <robdclark@gmail.com> wrote:
> > On Sat, Aug 20, 2016 at 1:58 PM, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
> >> Cc'ing lkml too.
> >>
> >> On Fri, Aug 19, 2016 at 11:54:21PM +0100, Emil Velikov wrote:
> >>> Story time:
> >>> I was dreaming of a day were we can stop installing these headers,
> >>> thus making deprecation a bit easier process.
> >>> Yet after failing to convince Dave and Daniel on a number of occasions
> >>> I've accepted that those headers _are_ here to stay. And yes they
> >>> _are_ the UAPI, even though no applications are meant to use them but
> >>> the libdrm 'version'.
> >>> Thus any changes to the libdrm ones should be a mirror of the ones
> >>> here and libdrm should _not_ differ.
> >>
> >> Another day dream:
> >>
> >> Wouldn't it be nice if the uapi headers from Linux kernel would pass
> >> a simple quality check of compiling in userspace where they are meant to be
> >> used? Stand alone. Without magic tricks and additional libraries and their
> >> headers. Without glibc or any other libc implementation specific additions.
> >> The uapi headers define many parts of the Linux kernel API and ABI, and thus
> >> compiling them also without the 'official' GNU/Linux userspace libraries
> >> like glibc or libdrm does have some uses. For example API and ABI
> >> compatibility checks and API/ABI/system call fuzzers.
> >>
> >> Many headers required stdint.h types but Linux kernel headers do not
> >> define them in userspace, and then Linus has said that uapi headers
> >> should use the linux/types.h with double underscores. Thus my patches
> >> for fixing trivial compile errors turned into changing several stdint.h
> >> definitions to linux/types.h.
> >
> > The problem is, for the most part, the driver specific gpu related
> > ioctl interfaces are not intended for general public consumption.
> > They have one consumer, ie. libdrm_$drivername (or perhaps mesa
> > directly).  They are complex interfaces, because GPUs are complex.
> > They are not intended to be used directly (or for the most part, even
> > indirectly) by random userspace applications.  And in fact the uapi
> > headers exported from kernel are not actually ever used.  (ie.
> > libdrm_$drivername uses it's own copy internally within libdrm.)
> >
> > So Linus's argument against stdint types, as weak as it is, doesn't
> > even apply for gpu driver specific ioctls.
> >
> Although last time around people leaned towards the __uX types, if we
> have a consensus amongst drm (kernel) developers about using stdint
> ones everything should be fine.
> We just need a handful of acks from the different maintainers.

Note that drm in not the only kernel subsystem with this wish/requirement.

fuse maintainer has said the same problem and there are others, where
Linux kernel uapi headers come from standards or other external sources.
coda and xen come to my mind due to changes I had to make lately.

> That said, _note_ that some applications are built with -C89 -pedantic
> [1] which means that using stdint.h may or may not work as expected.
> So we'll want a __STDC_VESION__ check + #error in case of pre-C99 ?
> If the affected programs are proprietary ones we should be safe,
> otherwise we want to update them ~alongside the transition.

In the uapi headers side maybe a common kernel subsystem specific compatibility
header could define the policy, e.g. <drm/libdrm-compat.h> would include
<stdint.h>. In similar way, <linux/libfuse-compat.h> and <xen/xen-compat.h>
would handle fuse and xen policies. <linux/libc-compat.h> could define the
default policy of <linux/types.h> as it is now.

-Mikko

Re: [RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[Rob Clark]

On Mon, Aug 22, 2016 at 4:48 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 20 August 2016 at 23:31, Rob Clark <robdclark@gmail.com> wrote:
>> On Sat, Aug 20, 2016 at 1:58 PM, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
>>> Cc'ing lkml too.
>>>
>>> On Fri, Aug 19, 2016 at 11:54:21PM +0100, Emil Velikov wrote:
>>>> Story time:
>>>> I was dreaming of a day were we can stop installing these headers,
>>>> thus making deprecation a bit easier process.
>>>> Yet after failing to convince Dave and Daniel on a number of occasions
>>>> I've accepted that those headers _are_ here to stay. And yes they
>>>> _are_ the UAPI, even though no applications are meant to use them but
>>>> the libdrm 'version'.
>>>> Thus any changes to the libdrm ones should be a mirror of the ones
>>>> here and libdrm should _not_ differ.
>>>
>>> Another day dream:
>>>
>>> Wouldn't it be nice if the uapi headers from Linux kernel would pass
>>> a simple quality check of compiling in userspace where they are meant to be
>>> used? Stand alone. Without magic tricks and additional libraries and their
>>> headers. Without glibc or any other libc implementation specific additions.
>>> The uapi headers define many parts of the Linux kernel API and ABI, and thus
>>> compiling them also without the 'official' GNU/Linux userspace libraries
>>> like glibc or libdrm does have some uses. For example API and ABI
>>> compatibility checks and API/ABI/system call fuzzers.
>>>
>>> Many headers required stdint.h types but Linux kernel headers do not
>>> define them in userspace, and then Linus has said that uapi headers
>>> should use the linux/types.h with double underscores. Thus my patches
>>> for fixing trivial compile errors turned into changing several stdint.h
>>> definitions to linux/types.h.
>>
>> The problem is, for the most part, the driver specific gpu related
>> ioctl interfaces are not intended for general public consumption.
>> They have one consumer, ie. libdrm_$drivername (or perhaps mesa
>> directly).  They are complex interfaces, because GPUs are complex.
>> They are not intended to be used directly (or for the most part, even
>> indirectly) by random userspace applications.  And in fact the uapi
>> headers exported from kernel are not actually ever used.  (ie.
>> libdrm_$drivername uses it's own copy internally within libdrm.)
>>
>> So Linus's argument against stdint types, as weak as it is, doesn't
>> even apply for gpu driver specific ioctls.
>>
> Although last time around people leaned towards the __uX types, if we
> have a consensus amongst drm (kernel) developers about using stdint
> ones everything should be fine.
> We just need a handful of acks from the different maintainers.

maybe I didn't grumble loudly enough at the time (against __uX types)

> That said, _note_ that some applications are built with -C89 -pedantic
> [1] which means that using stdint.h may or may not work as expected.
> So we'll want a __STDC_VESION__ check + #error in case of pre-C99 ?
> If the affected programs are proprietary ones we should be safe,
> otherwise we want to update them ~alongside the transition.

naw, at least for msm_drm.h, just don't build libdrm_freedreno w/
-C89.. problem solved!

BR,
-R

> Thanks
> Emil
>
> [1]
> https://cgit.freedesktop.org/mesa/drm/commit/?id=0f4452bb51306024fbf4cbf77d8baab20cefba67
> https://cgit.freedesktop.org/mesa/drm/commit/?id=d20314d083e533e3b8753192b1846752341afbbe

Re: [RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[Daniel Vetter]

On Mon, Aug 22, 2016 at 12:38 PM, Rob Clark <robdclark@gmail.com> wrote:
>> That said, _note_ that some applications are built with -C89 -pedantic
>> [1] which means that using stdint.h may or may not work as expected.
>> So we'll want a __STDC_VESION__ check + #error in case of pre-C99 ?
>> If the affected programs are proprietary ones we should be safe,
>> otherwise we want to update them ~alongside the transition.
>
> naw, at least for msm_drm.h, just don't build libdrm_freedreno w/
> -C89.. problem solved!

Yeah, I think sprinkling an

#ifdef __kernel___
#include <linux/types.h>
#else
#include <stdtypes.h>
#endif

at the opt of all drm uapi headers should be good enough. Or at least
those which opt to choose stdints. Since our userspace is very
limited, and our headers will never leak to general applications we
can just require c99, at least for driver headers. For kms/general drm
uapi that might not be the best idea.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[Emil Velikov]

On 22 August 2016 at 15:38, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Aug 22, 2016 at 12:38 PM, Rob Clark <robdclark@gmail.com> wrote:
>>> That said, _note_ that some applications are built with -C89 -pedantic
>>> [1] which means that using stdint.h may or may not work as expected.
>>> So we'll want a __STDC_VESION__ check + #error in case of pre-C99 ?
>>> If the affected programs are proprietary ones we should be safe,
>>> otherwise we want to update them ~alongside the transition.
>>
>> naw, at least for msm_drm.h, just don't build libdrm_freedreno w/
>> -C89.. problem solved!
>
> Yeah, I think sprinkling an
>
> #ifdef __kernel___
> #include <linux/types.h>
> #else
> #include <stdtypes.h>
> #endif
>
Guess i was too vague :-]

I was thinking about the following cases:
 - using old/incomplete stdint.h - thus the __STDC_VESION__ check.
 - building non-libdrm software - for libdrm we've (implicitly and
explicitly) required C99 for a long time.

> at the opt of all drm uapi headers should be good enough. Or at least
> those which opt to choose stdints. Since our userspace is very
> limited, and our headers will never leak to general applications we
> can just require c99, at least for driver headers. For kms/general drm
> uapi that might not be the best idea.
Won't doing so bring more confusion to an already convoluted topic ?
If we opt for it, let's have a juicy comment that clarifies things.

-Emil

Re: [RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[Daniel Vetter]

On Mon, Aug 22, 2016 at 04:05:21PM +0100, Emil Velikov wrote:
> On 22 August 2016 at 15:38, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Aug 22, 2016 at 12:38 PM, Rob Clark <robdclark@gmail.com> wrote:
> >>> That said, _note_ that some applications are built with -C89 -pedantic
> >>> [1] which means that using stdint.h may or may not work as expected.
> >>> So we'll want a __STDC_VESION__ check + #error in case of pre-C99 ?
> >>> If the affected programs are proprietary ones we should be safe,
> >>> otherwise we want to update them ~alongside the transition.
> >>
> >> naw, at least for msm_drm.h, just don't build libdrm_freedreno w/
> >> -C89.. problem solved!
> >
> > Yeah, I think sprinkling an
> >
> > #ifdef __kernel___
> > #include <linux/types.h>
> > #else
> > #include <stdtypes.h>
> > #endif
> >
> Guess i was too vague :-]
> 
> I was thinking about the following cases:
>  - using old/incomplete stdint.h - thus the __STDC_VESION__ check.
>  - building non-libdrm software - for libdrm we've (implicitly and
> explicitly) required C99 for a long time.
> 
> > at the opt of all drm uapi headers should be good enough. Or at least
> > those which opt to choose stdints. Since our userspace is very
> > limited, and our headers will never leak to general applications we
> > can just require c99, at least for driver headers. For kms/general drm
> > uapi that might not be the best idea.
> Won't doing so bring more confusion to an already convoluted topic ?
> If we opt for it, let's have a juicy comment that clarifies things.

If we require C99 in libdrm since ages then I think there's no problem
with outright requiring working stdint support in drm uapi headers
everywhere. We still need a bit of #ifdef though I think to impendence
match between kernel and userspace.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC] Using C99 stdint vs kernel __uX types in kernel drmUAPI (was Re: [PATCH 1/2] Revert "include/uapi/drm/amdgpu_drm.h: use __u32 and __u64 from <linux/types.h>")

[randyf]

On Mon, 22 Aug 2016, Emil Velikov wrote:

> Although last time around people leaned towards the __uX types, if we
> have a consensus amongst drm (kernel) developers about using stdint
> ones everything should be fine.
> We just need a handful of acks from the different maintainers.
>

   My opinion (as a non-Linux driver maintainer) is to use the stdint 
variants.  They are (or at least should be) supported by all OS's 
and compilers without any special modifications.


   That said, it really isn't that hard for me to wrap the __uX types 
either.


 	---- Randy