Re: [Qemu-devel] [PATCH v4] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

[Arnd Bergmann]

On Fri, Jul 12, 2019 at 2:17 PM Laurent Vivier <address@hidden> wrote:
>
> Le 11/07/2019 à 23:05, Arnd Bergmann a écrit :
> > On Thu, Jul 11, 2019 at 7:32 PM Laurent Vivier <address@hidden> 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.

> I've added the SH4 variant.

What is special about SH4?

> 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
};

         Arnd