Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1511114 > unrolled thread

Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106 Arnd Bergmann <arnd@arndb.de> - 2016-10-28 14:50 +0200
    Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106 Deepa Dinamani <deepa.kernel@gmail.com> - 2016-10-28 17:20 +0200
      Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106 Arnd Bergmann <arnd@arndb.de> - 2016-10-28 17:50 +0200
        Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106 Deepa Dinamani <deepa.kernel@gmail.com> - 2016-10-28 23:40 +0200
          Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106 Arnd Bergmann <arnd@arndb.de> - 2016-10-28 23:50 +0200
            Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106 Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-10-29 00:00 +0200
              Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106 Arnd Bergmann <arnd@arndb.de> - 2016-10-29 00:10 +0200

#1511114 — Re: [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

On Monday, October 17, 2016 8:27:32 PM CEST Deepa Dinamani wrote:
> @@ -55,24 +60,24 @@ struct ff_effect_compat {
>  
>  static inline size_t input_event_size(void)
>  {
> -       return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
> -               sizeof(struct input_event_compat) : sizeof(struct input_event);
> +       return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
> +                                    sizeof(struct raw_input_event);
>  }

I think the COMPAT_USE_64BIT_TIME check has to stay here,
it's needed for x32 mode on x86-64.

	Arnd

[toc] | [next] | [standalone]

#1511170

On Fri, Oct 28, 2016 at 5:43 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday, October 17, 2016 8:27:32 PM CEST Deepa Dinamani wrote:
>> @@ -55,24 +60,24 @@ struct ff_effect_compat {
>>
>>  static inline size_t input_event_size(void)
>>  {
>> -       return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
>> -               sizeof(struct input_event_compat) : sizeof(struct input_event);
>> +       return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
>> +                                    sizeof(struct raw_input_event);
>>  }
>
> I think the COMPAT_USE_64BIT_TIME check has to stay here,
> it's needed for x32 mode on x86-64.

There is no time_t anymore in the raw_input_event structure.
The struct uses __kernel_ulong_t type.
This should take care of x32 support.

From this cover letter:
https://www.spinics.net/lists/linux-arch/msg16356.html

I see that that the __kernel types were introduced to address the ABI
issues for x32.

-Deepa

[toc] | [prev] | [next] | [standalone]

#1511189

On Friday, October 28, 2016 8:19:46 AM CEST Deepa Dinamani wrote:
> On Fri, Oct 28, 2016 at 5:43 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday, October 17, 2016 8:27:32 PM CEST Deepa Dinamani wrote:
> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
> >>
> >>  static inline size_t input_event_size(void)
> >>  {
> >> -       return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
> >> -               sizeof(struct input_event_compat) : sizeof(struct input_event);
> >> +       return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
> >> +                                    sizeof(struct raw_input_event);
> >>  }
> >
> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
> > it's needed for x32 mode on x86-64.
> 
> There is no time_t anymore in the raw_input_event structure.
> The struct uses __kernel_ulong_t type.
> This should take care of x32 support.

I don't think it does.

> From this cover letter:
> https://www.spinics.net/lists/linux-arch/msg16356.html
> 
> I see that that the __kernel types were introduced to address the ABI
> issues for x32.

This is a variation of the problem we are trying to solve for
the other architectures in your patch set:

On x32, the kernel uses produces a structure with the 64-bit
layout, using __u64 tv_sec, to match the current user space
that has 64-bit __kernel_ulong_t and 64-bit time_t, but
in_compat_syscall() also returns 'true' here, as this is
mostly a 32-bit ABI (time_t being one of the exceptions).

	ARnd

[toc] | [prev] | [next] | [standalone]

#1511436

>> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
>> >>
>> >>  static inline size_t input_event_size(void)
>> >>  {
>> >> -       return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
>> >> -               sizeof(struct input_event_compat) : sizeof(struct input_event);
>> >> +       return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
>> >> +                                    sizeof(struct raw_input_event);
>> >>  }
>> >
>> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
>> > it's needed for x32 mode on x86-64.
>>
>> There is no time_t anymore in the raw_input_event structure.
>> The struct uses __kernel_ulong_t type.
>> This should take care of x32 support.
>
> I don't think it does.
>
>> From this cover letter:
>> https://www.spinics.net/lists/linux-arch/msg16356.html
>>
>> I see that that the __kernel types were introduced to address the ABI
>> issues for x32.
>
> This is a variation of the problem we are trying to solve for
> the other architectures in your patch set:
>
> On x32, the kernel uses produces a structure with the 64-bit
> layout, using __u64 tv_sec, to match the current user space
> that has 64-bit __kernel_ulong_t and 64-bit time_t, but
> in_compat_syscall() also returns 'true' here, as this is
> mostly a 32-bit ABI (time_t being one of the exceptions).

Yes, I missed this.

in_compat_syscall() is true for x32, this would mean we end up here
even if it is a x32 syscall.
But, wouldn't it be better to use in_x32_syscall() here since there is
no timeval any more?

Thanks,
Deepa

[toc] | [prev] | [next] | [standalone]

#1511439

On Friday, October 28, 2016 2:39:35 PM CEST Deepa Dinamani wrote:
> >> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
> >> >>
> >> >>  static inline size_t input_event_size(void)
> >> >>  {
> >> >> -       return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
> >> >> -               sizeof(struct input_event_compat) : sizeof(struct input_event);
> >> >> +       return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
> >> >> +                                    sizeof(struct raw_input_event);
> >> >>  }
> >> >
> >> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
> >> > it's needed for x32 mode on x86-64.
> >>
> >> There is no time_t anymore in the raw_input_event structure.
> >> The struct uses __kernel_ulong_t type.
> >> This should take care of x32 support.
> >
> > I don't think it does.
> >
> >> From this cover letter:
> >> https://www.spinics.net/lists/linux-arch/msg16356.html
> >>
> >> I see that that the __kernel types were introduced to address the ABI
> >> issues for x32.
> >
> > This is a variation of the problem we are trying to solve for
> > the other architectures in your patch set:
> >
> > On x32, the kernel uses produces a structure with the 64-bit
> > layout, using __u64 tv_sec, to match the current user space
> > that has 64-bit __kernel_ulong_t and 64-bit time_t, but
> > in_compat_syscall() also returns 'true' here, as this is
> > mostly a 32-bit ABI (time_t being one of the exceptions).
> 
> Yes, I missed this.
> 
> in_compat_syscall() is true for x32, this would mean we end up here
> even if it is a x32 syscall.
> But, wouldn't it be better to use in_x32_syscall() here since there is
> no timeval any more?

We have to distinguish four cases on x86:

- native 32-bit, input_event with 32-bit time_t
- compat 32-bit, input_event_compat with 32-bit time_t
- native 64-bit, input_event with 64-bit time_t
- compat x32, input_event with 64-bit time_t

The first three can happen on other architectures too,
the last one is x86 specific. There are probably other ways
to express the condition above, but I can't think of one
that is better than the one we have today.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1511441

On Fri, Oct 28, 2016 at 2:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday, October 28, 2016 2:39:35 PM CEST Deepa Dinamani wrote:
>> >> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
>> >> >>
>> >> >>  static inline size_t input_event_size(void)
>> >> >>  {
>> >> >> -       return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
>> >> >> -               sizeof(struct input_event_compat) : sizeof(struct input_event);
>> >> >> +       return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
>> >> >> +                                    sizeof(struct raw_input_event);
>> >> >>  }
>> >> >
>> >> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
>> >> > it's needed for x32 mode on x86-64.
>> >>
>> >> There is no time_t anymore in the raw_input_event structure.
>> >> The struct uses __kernel_ulong_t type.
>> >> This should take care of x32 support.
>> >
>> > I don't think it does.
>> >
>> >> From this cover letter:
>> >> https://www.spinics.net/lists/linux-arch/msg16356.html
>> >>
>> >> I see that that the __kernel types were introduced to address the ABI
>> >> issues for x32.
>> >
>> > This is a variation of the problem we are trying to solve for
>> > the other architectures in your patch set:
>> >
>> > On x32, the kernel uses produces a structure with the 64-bit
>> > layout, using __u64 tv_sec, to match the current user space
>> > that has 64-bit __kernel_ulong_t and 64-bit time_t, but
>> > in_compat_syscall() also returns 'true' here, as this is
>> > mostly a 32-bit ABI (time_t being one of the exceptions).
>>
>> Yes, I missed this.
>>
>> in_compat_syscall() is true for x32, this would mean we end up here
>> even if it is a x32 syscall.
>> But, wouldn't it be better to use in_x32_syscall() here since there is
>> no timeval any more?
>
> We have to distinguish four cases on x86:
>
> - native 32-bit, input_event with 32-bit time_t
> - compat 32-bit, input_event_compat with 32-bit time_t
> - native 64-bit, input_event with 64-bit time_t
> - compat x32, input_event with 64-bit time_t
>
> The first three can happen on other architectures too,
> the last one is x86 specific. There are probably other ways
> to express the condition above, but I can't think of one
> that is better than the one we have today.

Can we detect if given task is compat x32, like we do for compat
64/32? Or entire userspace has to be x32?

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1511452

On Friday, October 28, 2016 2:56:10 PM CEST Dmitry Torokhov wrote:
> On Fri, Oct 28, 2016 at 2:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday, October 28, 2016 2:39:35 PM CEST Deepa Dinamani wrote:
> >> >> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
> >> >> >>
> >> >> >>  static inline size_t input_event_size(void)
> >> >> >>  {
> >> >> >> -       return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
> >> >> >> -               sizeof(struct input_event_compat) : sizeof(struct input_event);
> >> >> >> +       return in_compat_syscall() ? sizeof(struct raw_input_event_compat) :
> >> >> >> +                                    sizeof(struct raw_input_event);
> >> >> >>  }
> >> >> >
> >> >> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
> >> >> > it's needed for x32 mode on x86-64.
> >> >>
> >
> > We have to distinguish four cases on x86:
> >
> > - native 32-bit, input_event with 32-bit time_t
> > - compat 32-bit, input_event_compat with 32-bit time_t
> > - native 64-bit, input_event with 64-bit time_t
> > - compat x32, input_event with 64-bit time_t
> >
> > The first three can happen on other architectures too,
> > the last one is x86 specific. There are probably other ways
> > to express the condition above, but I can't think of one
> > that is better than the one we have today.
> 
> Can we detect if given task is compat x32, like we do for compat
> 64/32? Or entire userspace has to be x32?

Yes, this works fine per task, with the definition of COMPAT_USE_64BIT_TIME
that is hardcoded to zero everywhere except on x86 where it is

#define COMPAT_USE_64BIT_TIME \
        (!!(task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT))

This is unrelated to the patch in question, the existing code
is correct as long as we don't change the logic and just
replace input_event with raw_input_event (or __kernel_input_event
or whichever you prefer).

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web