Le 12/07/2019 à 14:47, Arnd Bergmann a écrit :
> On Fri, Jul 12, 2019 at 2:17 PM Laurent Vivier <laur...@vivier.eu> wrote:
>>
>> Le 11/07/2019 à 23:05, Arnd Bergmann a écrit :
>>> On Thu, Jul 11, 2019 at 7:32 PM Laurent Vivier <laur...@vivier.eu> wrote:
>>>
>>>>
>>>> Notes:
>>>> v4: [lv] timeval64 and timespec64 are { long long , long }
>>>
>>>>
>>>> +STRUCT(timeval64, TYPE_LONGLONG, TYPE_LONG)
>>>> +
>>>> +STRUCT(timespec64, TYPE_LONGLONG, TYPE_LONG)
>>>> +
>>>
>>> This still doesn't look right, see my earlier comment about padding
>>> on big-endian architectures.
>>>
>>> Note that the in-kernel 'timespec64' is different from the uapi
>>> '__kernel_timespec' exported by the kernel. I also still think you may
>>> need to convert between SIOCGSTAMP_NEW and SIOCGSTAMP_OLD,
>>> e.g. when emulating a 32-bit riscv process (which only use
>>> SIOCGSTAMP_NEW) on a kernel that only understands
>>> SIOCGSTAMP_OLD.
>>
>> I agree.
>> I'm preparing a patch always using SIOCGSTAMP and SIOCGSTAMPNS on the
>> host (converting the structure when needed).
>
> That in turn would have the problem of breaking in 2038 when the
> timestamp overflows.
No, because SIOCGSTAMP and SIOCGSTAMPNS are aliased to the _NEW versions
on system supporting them (yes, we need to rebuild the binary, but we have 19
years to do that).
#define SIOCGSTAMP ((sizeof(struct timeval)) == 8 ? \
SIOCGSTAMP_OLD : SIOCGSTAMP_NEW)
#define SIOCGSTAMPNS ((sizeof(struct timespec)) == 8 ? \
SIOCGSTAMPNS_OLD : SIOCGSTAMPNS_NEW)
>
>> I've added the SH4 variant.> What is special about SH4?
The definition of _OLD is different:
#define SIOCGSTAMP_OLD _IOR('s', 100, struct timeval) /* Get stamp (timeval) */
#define SIOCGSTAMPNS_OLD _IOR('s', 101, struct timespec) /* Get stamp
(timespec) */
>
>> I've added the sparc64 variant too: does it means sparc64 use the same
>> structure internally for the OLD and NEW version?
>
> I had to look it up in the code. Yes, it does, timeval is 64/32/pad32,
> timespec is 64/64 in both OLD and NEW for sparc.
>
>> What about sparc 32bit?
>
> sparc32 is like all other 32-bit targets: OLD uses 32/32, and
> new uses 64/64.
>
>> For big-endian, I didn't find in the kernel where the difference is
>> managed: a byte swapping of the 64bit value is not enough?
>
> No, you don't need to swap. The difference is only in the padding.
> Since the kernel uses a 64/64 structure here, and user space
> may have use 'long tv_nsec', you need to add the padding on
> the correct side, like
>
> struct timeval64 {
> long long tv_sec;
> #if 32bit && big-endian
> long :32; /* anonymous padding */
> #endif
> suseconds_t tv_usec;
> #if (32bit && little-endian) || sparc64
> long :32;
> #endif
> };
We don't do memcopy() but we set each field one by one, so the padding doesn't
seem needed if we define correctly the user structure:
struct target_timeval64 {
abi_llong tv_sec;
abi_long tv_usec;
};
and do something like:
struct target_timeval64 *target_tv;
struct timeval *host_tv;
...
__put_user(host_tv->tv_sec, &target_tv->tv_sec);
__put_user(host_tv->tv_usec, &target_tv->tv_usec);
...
Thanks,
Laurent
