element sizes in input_event struct on riscv32

element sizes in input_event struct on riscv32

[Antonios Salios]

Hi all.

I'm having trouble getting evdev to run in a simulated Buildroot
environment on riscv32. Evtest (and the x11 driver) seems to be
receiving garbage data from input devices.

Analyzing the input_event struct shows that the kernel uses 32-bit (aka
__kernel_ulong_t) values for __sec & __usec.
Evtest on the other hand interprets these variables as 64-bit time_t
values in a timeval struct, resulting in a mismatch between the kernel
and userspace.

What would be the correct size for these values on a 32-bit
architecture that uses 64-bit time_t values?


Kind regards

-- 
Antonios Salios
Student Employee

MachineWare GmbH | www.machineware.de
Hühnermarkt 19, 52062 Aachen, Germany
Amtsgericht Aachen HRB25734

Geschäftsführung
Lukas Jünger
Dr.-Ing. Jan Henrik Weinstock

Re: element sizes in input_event struct on riscv32

[Dmitry Torokhov]

Hi Antonious,

On Thu, Dec 14, 2023 at 11:11:18AM +0100, Antonios Salios wrote:
> Hi all.
> 
> I'm having trouble getting evdev to run in a simulated Buildroot
> environment on riscv32. Evtest (and the x11 driver) seems to be
> receiving garbage data from input devices.
> 
> Analyzing the input_event struct shows that the kernel uses 32-bit (aka
> __kernel_ulong_t) values for __sec & __usec.
> Evtest on the other hand interprets these variables as 64-bit time_t
> values in a timeval struct, resulting in a mismatch between the kernel
> and userspace.
> 
> What would be the correct size for these values on a 32-bit
> architecture that uses 64-bit time_t values?

I think there is misunderstanding - we do not have *2* 64-bit values on
32-but architectures. Here is what was done:

commit 152194fe9c3f03232b9c0d0264793a7fa4af82f8
Author: Deepa Dinamani <deepa.kernel@gmail.com>
Date:   Sun Jan 7 17:44:42 2018 -0800

    Input: extend usable life of event timestamps to 2106 on 32 bit systems

    The input events use struct timeval to store event time, unfortunately
    this structure is not y2038 safe and is being replaced in kernel with
    y2038 safe structures.

    Because of ABI concerns we can not change the size or the layout of
    structure input_event, so we opt to re-interpreting the 'seconds' part
    of timestamp as an unsigned value, effectively doubling the range of
    values, to year 2106.

    Newer glibc that has support for 32 bit applications to use 64 bit
    time_t supplies __USE_TIME_BITS64 define [1], that we can use to present
    the userspace with updated input_event layout. The updated layout will
    cause the compile time breakage, alerting applications and distributions
    maintainers to the issue. Existing 32 binaries will continue working
    without any changes until 2038.

    Ultimately userspace applications should switch to using monotonic or
    boot time clocks, as realtime clock is not very well suited for input
    event timestamps as it can go backwards (see a80b83b7b8 "Input: evdev -
    add CLOCK_BOOTTIME support" by by John Stultz). With monotonic clock the
    practical range of reported times will always fit into the pair of 32
    bit values, as we do not expect any system to stay up for a hundred
    years without a single reboot.

    [1] https://sourceware.org/glibc/wiki/Y2038ProofnessDesign

I'll let Arnd and Deepa to comment further.

Thanks.

-- 
Dmitry

Re: element sizes in input_event struct on riscv32

[Arnd Bergmann]

On Tue, Dec 19, 2023, at 02:50, Dmitry Torokhov wrote:
> Hi Antonious,
>
> On Thu, Dec 14, 2023 at 11:11:18AM +0100, Antonios Salios wrote:
>> Hi all.
>> 
>> I'm having trouble getting evdev to run in a simulated Buildroot
>> environment on riscv32. Evtest (and the x11 driver) seems to be
>> receiving garbage data from input devices.
>> 
>> Analyzing the input_event struct shows that the kernel uses 32-bit (aka
>> __kernel_ulong_t) values for __sec & __usec.
>> Evtest on the other hand interprets these variables as 64-bit time_t
>> values in a timeval struct, resulting in a mismatch between the kernel
>> and userspace.
>> 
>> What would be the correct size for these values on a 32-bit
>> architecture that uses 64-bit time_t values?
>
> I think there is misunderstanding - we do not have *2* 64-bit values on
> 32-but architectures. Here is what was done:
>
>     Input: extend usable life of event timestamps to 2106 on 32 bit systems

Thanks for forwarding this to me. You are definitely right that
the user-space structure is intended to use a pair of __kernel_ulong_t
for the timestamp. Usually if an application gets this wrong, it is the
result of having copied old kernel headers the source directory that
need to be updated.

For evtest in particular, I don't see how that is possible, the source
code at [1] shows that it just includes the global linux/input.h,
which on riscv32 would have to be at least from linux-5.6 anyway
because older versions are too old to build a time64 glibc.

Antonios, can you check which header was used to build your copy
of evtest, and in case this came from /usr/include/linux, which
version it corresponds to?

      Arnd

[1] https://gitlab.freedesktop.org/libevdev/evtest/-/blob/master/evtest.c?ref_type=heads

Re: element sizes in input_event struct on riscv32

[Antonios Salios]

On Tue, 2023-12-19 at 13:53 +0000, Arnd Bergmann wrote:
> On Tue, Dec 19, 2023, at 02:50, Dmitry Torokhov wrote:
> > Hi Antonious,
> > 
> > On Thu, Dec 14, 2023 at 11:11:18AM +0100, Antonios Salios wrote:
> > > Hi all.
> > > 
> > > I'm having trouble getting evdev to run in a simulated Buildroot
> > > environment on riscv32. Evtest (and the x11 driver) seems to be
> > > receiving garbage data from input devices.
> > > 
> > > Analyzing the input_event struct shows that the kernel uses 32-
> > > bit (aka
> > > __kernel_ulong_t) values for __sec & __usec.
> > > Evtest on the other hand interprets these variables as 64-bit
> > > time_t
> > > values in a timeval struct, resulting in a mismatch between the
> > > kernel
> > > and userspace.
> > > 
> > > What would be the correct size for these values on a 32-bit
> > > architecture that uses 64-bit time_t values?
> > 
> > I think there is misunderstanding - we do not have *2* 64-bit
> > values on
> > 32-but architectures. Here is what was done:
> > 
> >     Input: extend usable life of event timestamps to 2106 on 32 bit
> > systems
> 
> Thanks for forwarding this to me. You are definitely right that
> the user-space structure is intended to use a pair of
> __kernel_ulong_t
> for the timestamp. Usually if an application gets this wrong, it is
> the
> result of having copied old kernel headers the source directory that
> need to be updated.
> 
> For evtest in particular, I don't see how that is possible, the
> source
> code at [1] shows that it just includes the global linux/input.h,
> which on riscv32 would have to be at least from linux-5.6 anyway
> because older versions are too old to build a time64 glibc.
> 
> Antonios, can you check which header was used to build your copy
> of evtest, and in case this came from /usr/include/linux, which
> version it corresponds to?
> 
>       Arnd
> 
> [1]
> https://gitlab.freedesktop.org/libevdev/evtest/-/blob/master/evtest.c?ref_type=heads

The header is included from the sysroot of the toolchain, using version
6.5.6.
I'm using glibc 2.37 with a toolchain built from Buildroot.

The problem seems to be, that __USE_TIME_BITS64 is not defined even
though riscv32 uses 64-bit time.
__BITS_PER_LONG is set to 32 & __KERNEL__ is (of course) undefined in
userspace.
The userspace therefore uses 64-bit values as opposed to the kernel,
which uses 32-bit values.

__USE_TIME_BITS64 is only set when __TIMESIZE is set to 32. [1]
Under riscv32, the default value of 64 is used. [2]


[1]
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/features-time64.h;h=af9d84daa7dfe4174e9f977b2c76c5c0df1ce47b;hb=refs/tags/glibc-2.37
[2]
https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=bits/timesize.h;hb=refs/tags/glibc-2.37

-- 
Antonios Salios
Software Engineer

MachineWare GmbH | www.machineware.de
Hühnermarkt 19, 52062 Aachen, Germany
Amtsgericht Aachen HRB25734

Geschäftsführung
Lukas Jünger
Dr.-Ing. Jan Henrik Weinstock

Re: element sizes in input_event struct on riscv32

[Arnd Bergmann]

On Thu, Dec 21, 2023, at 08:56, Antonios Salios wrote:
> On Tue, 2023-12-19 at 13:53 +0000, Arnd Bergmann wrote:
>> On Tue, Dec 19, 2023, at 02:50, Dmitry Torokhov wrote:
>
> The header is included from the sysroot of the toolchain, using version
> 6.5.6.
> I'm using glibc 2.37 with a toolchain built from Buildroot.
>
> The problem seems to be, that __USE_TIME_BITS64 is not defined even
> though riscv32 uses 64-bit time.

That sounds like a libc bug. Which C library are you using?

> __BITS_PER_LONG is set to 32 & __KERNEL__ is (of course) undefined in
> userspace.
> The userspace therefore uses 64-bit values as opposed to the kernel,
> which uses 32-bit values.
>
> __USE_TIME_BITS64 is only set when __TIMESIZE is set to 32. [1]
> Under riscv32, the default value of 64 is used. [2]

I don't know what __TIMESIZE is, this is not part of the kernel ABI
as far as I can tell. __USE_TIME_BITS64 should be set by any 32-bit
architecture if the C library defines a 64-bit time_t, otherwise the
kernel headers have no way of picking the correct definitions based
on preprocessor logic.

    Arnd

Re: element sizes in input_event struct on riscv32

[Antonios Salios]

On Thu, 2023-12-21 at 12:28 +0000, Arnd Bergmann wrote:
> On Thu, Dec 21, 2023, at 08:56, Antonios Salios wrote:
> > On Tue, 2023-12-19 at 13:53 +0000, Arnd Bergmann wrote:
> > > On Tue, Dec 19, 2023, at 02:50, Dmitry Torokhov wrote:
> > 
> > The problem seems to be, that __USE_TIME_BITS64 is not defined even
> > though riscv32 uses 64-bit time.
> 
> That sounds like a libc bug. Which C library are you using?

Glibc 2.37.

> > __BITS_PER_LONG is set to 32 & __KERNEL__ is (of course) undefined
> > in
> > userspace.
> > The userspace therefore uses 64-bit values as opposed to the
> > kernel,
> > which uses 32-bit values.
> > 
> > __USE_TIME_BITS64 is only set when __TIMESIZE is set to 32. [1]
> > Under riscv32, the default value of 64 is used. [2]
> 
> I don't know what __TIMESIZE is, this is not part of the kernel ABI
> as far as I can tell. __USE_TIME_BITS64 should be set by any 32-bit
> architecture if the C library defines a 64-bit time_t, otherwise the
> kernel headers have no way of picking the correct definitions based
> on preprocessor logic.

Okay, I agree that this might be a libc problem then. I'll ask the
glibc maintainers.

Thanks for your kind help and happy holidays!

-- 
Antonios Salios
Software Engineer

MachineWare GmbH | www.machineware.de
Hühnermarkt 19, 52062 Aachen, Germany
Amtsgericht Aachen HRB25734

Geschäftsführung
Lukas Jünger
Dr.-Ing. Jan Henrik Weinstock

Re: element sizes in input_event struct on riscv32

[Antonios Salios]

On Thu, 2023-12-21 at 14:38 +0100, Antonios Salios wrote:
> On Thu, 2023-12-21 at 12:28 +0000, Arnd Bergmann wrote:
> > On Thu, Dec 21, 2023, at 08:56, Antonios Salios wrote:
> > > On Tue, 2023-12-19 at 13:53 +0000, Arnd Bergmann wrote:
> > > > On Tue, Dec 19, 2023, at 02:50, Dmitry Torokhov wrote:
> > 
> > I don't know what __TIMESIZE is, this is not part of the kernel ABI
> > as far as I can tell. __USE_TIME_BITS64 should be set by any 32-bit
> > architecture if the C library defines a 64-bit time_t, otherwise
> > the
> > kernel headers have no way of picking the correct definitions based
> > on preprocessor logic.
> 
> Okay, I agree that this might be a libc problem then. I'll ask the
> glibc maintainers.
> 

According to a glibc maintainer, __USE_TIME_BITS64 is not set on
architectures that use 64-bit time_t as default such as riscv32.
This can also be seen here [1].

Perhaps the kernel header needs to check the size of time_t in some
other way?

[1]
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/features-time64.h;hb=glibc-2.37

-- 
Antonios Salios
Software Engineer

MachineWare GmbH | www.machineware.de
Hühnermarkt 19, 52062 Aachen, Germany
Amtsgericht Aachen HRB25734

Geschäftsführung
Lukas Jünger
Dr.-Ing. Jan Henrik Weinstock

Re: element sizes in input_event struct on riscv32

[Arnd Bergmann]

On Mon, Jan 15, 2024, at 16:46, Antonios Salios wrote:
> On Thu, 2023-12-21 at 14:38 +0100, Antonios Salios wrote:
>> On Thu, 2023-12-21 at 12:28 +0000, Arnd Bergmann wrote:
>> > On Thu, Dec 21, 2023, at 08:56, Antonios Salios wrote:
>> > > On Tue, 2023-12-19 at 13:53 +0000, Arnd Bergmann wrote:
>> > > > On Tue, Dec 19, 2023, at 02:50, Dmitry Torokhov wrote:
>> > 
>> > I don't know what __TIMESIZE is, this is not part of the kernel ABI
>> > as far as I can tell. __USE_TIME_BITS64 should be set by any 32-bit
>> > architecture if the C library defines a 64-bit time_t, otherwise
>> > the
>> > kernel headers have no way of picking the correct definitions based
>> > on preprocessor logic.
>> 
>> Okay, I agree that this might be a libc problem then. I'll ask the
>> glibc maintainers.
>> 
>
> According to a glibc maintainer, __USE_TIME_BITS64 is not set on
> architectures that use 64-bit time_t as default such as riscv32.
> This can also be seen here [1].
>
> Perhaps the kernel header needs to check the size of time_t in some
> other way?
>
> [1]
> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/features-time64.h;hb=glibc-2.37

I don't see any better way, the kernel headers started using this
in 2018 based on the glibc design documents and discussions
with glibc maintainers, see the section on ioctls in
https://sourceware.org/glibc/wiki/Y2038ProofnessDesign

The kernel only relies on this macro for the sound and
input subsystem, but there are numerous applications and
libraries that copied the kernel definition because that
was defined as the only reliable method.

Maybe you can work around by patching the glibc sources
yourself?

     Arnd