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

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

[Daniel P. Berrangé]

The SIOCGSTAMP symbol was previously defined in the
asm-generic/sockios.h header file. QEMU sees that header
indirectly via sys/socket.h

In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
Instead it provides only SIOCGSTAMP_OLD, which only uses a
32-bit time_t on 32-bit architectures.

The linux/sockios.h header then defines SIOCGSTAMP using
either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
on 32-bit architectures

To cope with this we must now define two separate syscalls,
with corresponding old and new sizes, as well as including
the new linux/sockios.h header.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 linux-user/ioctls.h        | 15 +++++++++++++++
 linux-user/syscall.c       |  1 +
 linux-user/syscall_defs.h  |  5 +++++
 linux-user/syscall_types.h |  4 ++++
 4 files changed, 25 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 5e84dc7c3a..5a6d6def7e 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -222,8 +222,23 @@
   IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
   IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
   IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
+
+#ifdef SIOCGSTAMP_OLD
+  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
+#else
   IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
+#endif
+#ifdef SIOCGSTAMPNS_OLD
+  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
+#else
   IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
+#endif
+#ifdef SIOCGSTAMP_NEW
+  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
+#endif
+#ifdef SIOCGSTAMPNS_NEW
+  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
+#endif
 
   IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
   IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b187c1281d..f13e260b02 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -37,6 +37,7 @@
 #include <sched.h>
 #include <sys/timex.h>
 #include <sys/socket.h>
+#include <linux/sockios.h>
 #include <sys/un.h>
 #include <sys/uio.h>
 #include <poll.h>
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 7f141f699c..7830b600e7 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -750,6 +750,11 @@ struct target_pollfd {
 
 #define TARGET_SIOCGSTAMP      0x8906          /* Get stamp (timeval) */
 #define TARGET_SIOCGSTAMPNS    0x8907          /* Get stamp (timespec) */
+#define TARGET_SIOCGSTAMP_OLD   0x8906          /* Get stamp (timeval) */
+#define TARGET_SIOCGSTAMPNS_OLD 0x8907          /* Get stamp (timespec) */
+#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
+#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
+
 
 /* Networking ioctls */
 #define TARGET_SIOCADDRT       0x890B          /* add routing table entry */
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index b98a23b0f1..de4c5a5b6f 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -20,6 +20,10 @@ STRUCT(timeval,
 STRUCT(timespec,
        MK_ARRAY(TYPE_LONG, 2))
 
+STRUCT(timeval64, TYPE_LONGLONG, TYPE_LONG)
+
+STRUCT(timespec64, TYPE_LONGLONG, TYPE_LONG)
+
 STRUCT(rtentry,
        TYPE_ULONG, MK_STRUCT(STRUCT_sockaddr), MK_STRUCT(STRUCT_sockaddr), MK_STRUCT(STRUCT_sockaddr),
        TYPE_SHORT, TYPE_SHORT, TYPE_ULONG, TYPE_PTRVOID, TYPE_SHORT, TYPE_PTRVOID,
-- 
2.21.0

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190617131103.1413-1-berrange@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels
Type: series
Message-id: 20190617131103.1413-1-berrange@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20190615154352.26824-1-philmd@redhat.com -> patchew/20190615154352.26824-1-philmd@redhat.com
Switched to a new branch 'test'
aeb14b5 linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

=== OUTPUT BEGIN ===
ERROR: line over 90 characters
#78: FILE: linux-user/syscall_defs.h:755:
+#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */

ERROR: line over 90 characters
#79: FILE: linux-user/syscall_defs.h:756:
+#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */

total: 2 errors, 0 warnings, 51 lines checked

Commit aeb14b599d8b (linux-user: fix to handle variably sized SIOCGSTAMP with new kernels) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190617131103.1413-1-berrange@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

[Arnd Bergmann]

On Mon, Jun 17, 2019 at 3:11 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The SIOCGSTAMP symbol was previously defined in the
> asm-generic/sockios.h header file. QEMU sees that header
> indirectly via sys/socket.h
>
> In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> Instead it provides only SIOCGSTAMP_OLD, which only uses a
> 32-bit time_t on 32-bit architectures.

This is a bit misleading, as we still define SIOCGSTAMP in the
right place. asm-generic/sockios.h should not be used by normal
user space.

> The linux/sockios.h header then defines SIOCGSTAMP using
> either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> on 32-bit architectures
>
> To cope with this we must now define two separate syscalls,
> with corresponding old and new sizes, as well as including
> the new linux/sockios.h header.

The overall concept seems right. A few more comments on
details that may have gone wrong here. I'm not familiar with
the qemu-user implementation, so it's mostly guesswork
on my end.

>    IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
>    IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
>    IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
> +
> +#ifdef SIOCGSTAMP_OLD
> +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> +#else
>    IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> +#endif
> +#ifdef SIOCGSTAMPNS_OLD
> +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> +#else
>    IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> +#endif
> +#ifdef SIOCGSTAMP_NEW
> +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
> +#endif
> +#ifdef SIOCGSTAMPNS_NEW
> +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
> +#endif

Is timespec64 a qemu type? How is it defined?

> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 7f141f699c..7830b600e7 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -750,6 +750,11 @@ struct target_pollfd {
>
>  #define TARGET_SIOCGSTAMP      0x8906          /* Get stamp (timeval) */
>  #define TARGET_SIOCGSTAMPNS    0x8907          /* Get stamp (timespec) */
> +#define TARGET_SIOCGSTAMP_OLD   0x8906          /* Get stamp (timeval) */
> +#define TARGET_SIOCGSTAMPNS_OLD 0x8907          /* Get stamp (timespec) */

Note that these types are architecture specific. It seems that only
one architecture is left that has its own definitions though, so this
is only broken on arch/sh for current linux (and remains broken).

Future architectures, including 32-bit risc-v should only have the _NEW
version and not support SIOCGSTAMP_OLD at all.

When emulating risc-v user space on old kernels (pre-5.1), you may need to
translate the ioctl command and all system calls that take a 64-bit time_t into
the variants with a 32-bit time_t on the way into the kernel, and then back.

Similarly, running an old user binary on a riscv32 machine, you may
need to do the reverse translation.

> +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
> +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */

"sizeof(long long) + sizeof(long)" is not always the size of the argument to
TARGET_SIOCGSTAMP{NS}_NEW. On 32-bit architectures, the size is
two 64-bit values. sparc64 is potentially another special case, as 'struct
timeval is 'long + int' there (12 bytes).

On big-endian architectures, the nanoseconds are returned in the last
four bytes of the 16-byte structure.

>  /* Networking ioctls */
>  #define TARGET_SIOCADDRT       0x890B          /* add routing table entry */
> diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
> index b98a23b0f1..de4c5a5b6f 100644
> --- a/linux-user/syscall_types.h
> +++ b/linux-user/syscall_types.h
> @@ -20,6 +20,10 @@ STRUCT(timeval,
>  STRUCT(timespec,
>         MK_ARRAY(TYPE_LONG, 2))
>
> +STRUCT(timeval64, TYPE_LONGLONG, TYPE_LONG)
> +
> +STRUCT(timespec64, TYPE_LONGLONG, TYPE_LONG)

Same here.

        Arnd

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

[Laurent Vivier]

Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
> The SIOCGSTAMP symbol was previously defined in the
> asm-generic/sockios.h header file. QEMU sees that header
> indirectly via sys/socket.h
> 
> In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> Instead it provides only SIOCGSTAMP_OLD, which only uses a
> 32-bit time_t on 32-bit architectures.
> 
> The linux/sockios.h header then defines SIOCGSTAMP using
> either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> on 32-bit architectures
> 
> To cope with this we must now define two separate syscalls,
> with corresponding old and new sizes, as well as including
> the new linux/sockios.h header.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  linux-user/ioctls.h        | 15 +++++++++++++++
>  linux-user/syscall.c       |  1 +
>  linux-user/syscall_defs.h  |  5 +++++
>  linux-user/syscall_types.h |  4 ++++
>  4 files changed, 25 insertions(+)
> 
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 5e84dc7c3a..5a6d6def7e 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -222,8 +222,23 @@
>    IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
>    IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
>    IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
> +
> +#ifdef SIOCGSTAMP_OLD
> +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> +#else
>    IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> +#endif
> +#ifdef SIOCGSTAMPNS_OLD
> +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> +#else
>    IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> +#endif
> +#ifdef SIOCGSTAMP_NEW
> +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
> +#endif
> +#ifdef SIOCGSTAMPNS_NEW
> +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
> +#endif
>  
>    IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
>    IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b187c1281d..f13e260b02 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -37,6 +37,7 @@
>  #include <sched.h>
>  #include <sys/timex.h>
>  #include <sys/socket.h>
> +#include <linux/sockios.h>
>  #include <sys/un.h>
>  #include <sys/uio.h>
>  #include <poll.h>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 7f141f699c..7830b600e7 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -750,6 +750,11 @@ struct target_pollfd {
>  
>  #define TARGET_SIOCGSTAMP      0x8906          /* Get stamp (timeval) */
>  #define TARGET_SIOCGSTAMPNS    0x8907          /* Get stamp (timespec) */
> +#define TARGET_SIOCGSTAMP_OLD   0x8906          /* Get stamp (timeval) */
> +#define TARGET_SIOCGSTAMPNS_OLD 0x8907          /* Get stamp (timespec) */
> +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
> +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
kernel defines:

#define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
#define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])

So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])

Their codes are 0x80108906 and 80108907.

Thanks,
Laurent

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

[Christian Ehrhardt]

On Mon, Jun 17, 2019 at 5:35 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
> > The SIOCGSTAMP symbol was previously defined in the
> > asm-generic/sockios.h header file. QEMU sees that header
> > indirectly via sys/socket.h
> >
> > In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> > the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> > Instead it provides only SIOCGSTAMP_OLD, which only uses a
> > 32-bit time_t on 32-bit architectures.
> >
> > The linux/sockios.h header then defines SIOCGSTAMP using
> > either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> > SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> > on 32-bit architectures
> >
> > To cope with this we must now define two separate syscalls,
> > with corresponding old and new sizes, as well as including
> > the new linux/sockios.h header.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  linux-user/ioctls.h        | 15 +++++++++++++++
> >  linux-user/syscall.c       |  1 +
> >  linux-user/syscall_defs.h  |  5 +++++
> >  linux-user/syscall_types.h |  4 ++++
> >  4 files changed, 25 insertions(+)
> >
> > diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> > index 5e84dc7c3a..5a6d6def7e 100644
> > --- a/linux-user/ioctls.h
> > +++ b/linux-user/ioctls.h
> > @@ -222,8 +222,23 @@
> >    IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
> >    IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
> >    IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
> > +
> > +#ifdef SIOCGSTAMP_OLD
> > +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> > +#else
> >    IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> > +#endif
> > +#ifdef SIOCGSTAMPNS_OLD
> > +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> > +#else
> >    IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> > +#endif
> > +#ifdef SIOCGSTAMP_NEW
> > +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
> > +#endif
> > +#ifdef SIOCGSTAMPNS_NEW
> > +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
> > +#endif
> >
> >    IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
> >    IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index b187c1281d..f13e260b02 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -37,6 +37,7 @@
> >  #include <sched.h>
> >  #include <sys/timex.h>
> >  #include <sys/socket.h>
> > +#include <linux/sockios.h>
> >  #include <sys/un.h>
> >  #include <sys/uio.h>
> >  #include <poll.h>
> > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> > index 7f141f699c..7830b600e7 100644
> > --- a/linux-user/syscall_defs.h
> > +++ b/linux-user/syscall_defs.h
> > @@ -750,6 +750,11 @@ struct target_pollfd {
> >
> >  #define TARGET_SIOCGSTAMP      0x8906          /* Get stamp (timeval) */
> >  #define TARGET_SIOCGSTAMPNS    0x8907          /* Get stamp (timespec) */
> > +#define TARGET_SIOCGSTAMP_OLD   0x8906          /* Get stamp (timeval) */
> > +#define TARGET_SIOCGSTAMPNS_OLD 0x8907          /* Get stamp (timespec) */
> > +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
> > +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
> kernel defines:
>
> #define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
> #define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])
>
> So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])
>
> Their codes are 0x80108906 and 80108907.

Hi,
I found the discussion around this topic being almost a month old.
And related to this fedora bug [1] was closed by adding [2] which
matches [3] that was nacked in the discussion here.

Since I found nothing later (neither qemu commits nor further
discussions) I wonder if it has fallen through the cracks OR if there
was a kernel fix/change to resolve it (if that is the case a pointer
to the related kernel change would be nice)?

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1718926
[2]: https://src.fedoraproject.org/rpms/qemu/blob/master/f/0005-NOT-UPSTREAM-Build-fix-with-latest-kernel.patch
[3]: https://patchew.org/QEMU/20190604071915.288045-1-borntraeger@de.ibm.com/

> Thanks,
> Laurent
>


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

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

[Daniel P. Berrangé]

On Thu, Jul 11, 2019 at 10:02:01AM +0200, Christian Ehrhardt wrote:
> On Mon, Jun 17, 2019 at 5:35 PM Laurent Vivier <laurent@vivier.eu> wrote:
> >
> > Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
> > > The SIOCGSTAMP symbol was previously defined in the
> > > asm-generic/sockios.h header file. QEMU sees that header
> > > indirectly via sys/socket.h
> > >
> > > In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> > > the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> > > Instead it provides only SIOCGSTAMP_OLD, which only uses a
> > > 32-bit time_t on 32-bit architectures.
> > >
> > > The linux/sockios.h header then defines SIOCGSTAMP using
> > > either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> > > SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> > > on 32-bit architectures
> > >
> > > To cope with this we must now define two separate syscalls,
> > > with corresponding old and new sizes, as well as including
> > > the new linux/sockios.h header.
> > >
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  linux-user/ioctls.h        | 15 +++++++++++++++
> > >  linux-user/syscall.c       |  1 +
> > >  linux-user/syscall_defs.h  |  5 +++++
> > >  linux-user/syscall_types.h |  4 ++++
> > >  4 files changed, 25 insertions(+)
> > >
> > > diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> > > index 5e84dc7c3a..5a6d6def7e 100644
> > > --- a/linux-user/ioctls.h
> > > +++ b/linux-user/ioctls.h
> > > @@ -222,8 +222,23 @@
> > >    IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
> > >    IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
> > >    IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
> > > +
> > > +#ifdef SIOCGSTAMP_OLD
> > > +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> > > +#else
> > >    IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
> > > +#endif
> > > +#ifdef SIOCGSTAMPNS_OLD
> > > +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> > > +#else
> > >    IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
> > > +#endif
> > > +#ifdef SIOCGSTAMP_NEW
> > > +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
> > > +#endif
> > > +#ifdef SIOCGSTAMPNS_NEW
> > > +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
> > > +#endif
> > >
> > >    IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
> > >    IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
> > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > > index b187c1281d..f13e260b02 100644
> > > --- a/linux-user/syscall.c
> > > +++ b/linux-user/syscall.c
> > > @@ -37,6 +37,7 @@
> > >  #include <sched.h>
> > >  #include <sys/timex.h>
> > >  #include <sys/socket.h>
> > > +#include <linux/sockios.h>
> > >  #include <sys/un.h>
> > >  #include <sys/uio.h>
> > >  #include <poll.h>
> > > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> > > index 7f141f699c..7830b600e7 100644
> > > --- a/linux-user/syscall_defs.h
> > > +++ b/linux-user/syscall_defs.h
> > > @@ -750,6 +750,11 @@ struct target_pollfd {
> > >
> > >  #define TARGET_SIOCGSTAMP      0x8906          /* Get stamp (timeval) */
> > >  #define TARGET_SIOCGSTAMPNS    0x8907          /* Get stamp (timespec) */
> > > +#define TARGET_SIOCGSTAMP_OLD   0x8906          /* Get stamp (timeval) */
> > > +#define TARGET_SIOCGSTAMPNS_OLD 0x8907          /* Get stamp (timespec) */
> > > +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
> > > +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
> > kernel defines:
> >
> > #define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
> > #define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])
> >
> > So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])
> >
> > Their codes are 0x80108906 and 80108907.
> 
> Hi,
> I found the discussion around this topic being almost a month old.
> And related to this fedora bug [1] was closed by adding [2] which
> matches [3] that was nacked in the discussion here.
> 
> Since I found nothing later (neither qemu commits nor further
> discussions) I wonder if it has fallen through the cracks OR if there
> was a kernel fix/change to resolve it (if that is the case a pointer
> to the related kernel change would be nice)?

I didn't have time to address the feedback to this v2, and since the
immediate problem for Fedora has a workaround, its been lower priority
especially since my understanding of linux-iser is limited.

IOW, If anyone wants to take over my patch proposal here feel free. It
would obviously be nice to fix for this 4.1 release if practical.

> 
> [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1718926
> [2]: https://src.fedoraproject.org/rpms/qemu/blob/master/f/0005-NOT-UPSTREAM-Build-fix-with-latest-kernel.patch
> [3]: https://patchew.org/QEMU/20190604071915.288045-1-borntraeger@de.ibm.com/
> 
> > Thanks,
> > Laurent
> >
> 
> 
> -- 
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Christian Borntraeger]

On 11.07.19 11:24, Daniel P. Berrangé wrote:
> On Thu, Jul 11, 2019 at 10:02:01AM +0200, Christian Ehrhardt wrote:
>> On Mon, Jun 17, 2019 at 5:35 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>>
>>> Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
>>>> The SIOCGSTAMP symbol was previously defined in the
>>>> asm-generic/sockios.h header file. QEMU sees that header
>>>> indirectly via sys/socket.h
>>>>
>>>> In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
>>>> the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
>>>> Instead it provides only SIOCGSTAMP_OLD, which only uses a
>>>> 32-bit time_t on 32-bit architectures.
>>>>
>>>> The linux/sockios.h header then defines SIOCGSTAMP using
>>>> either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
>>>> SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
>>>> on 32-bit architectures
>>>>
>>>> To cope with this we must now define two separate syscalls,
>>>> with corresponding old and new sizes, as well as including
>>>> the new linux/sockios.h header.
>>>>
>>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> ---
>>>>  linux-user/ioctls.h        | 15 +++++++++++++++
>>>>  linux-user/syscall.c       |  1 +
>>>>  linux-user/syscall_defs.h  |  5 +++++
>>>>  linux-user/syscall_types.h |  4 ++++
>>>>  4 files changed, 25 insertions(+)
>>>>
>>>> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
>>>> index 5e84dc7c3a..5a6d6def7e 100644
>>>> --- a/linux-user/ioctls.h
>>>> +++ b/linux-user/ioctls.h
>>>> @@ -222,8 +222,23 @@
>>>>    IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
>>>>    IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
>>>>    IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
>>>> +
>>>> +#ifdef SIOCGSTAMP_OLD
>>>> +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
>>>> +#else
>>>>    IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
>>>> +#endif
>>>> +#ifdef SIOCGSTAMPNS_OLD
>>>> +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
>>>> +#else
>>>>    IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
>>>> +#endif
>>>> +#ifdef SIOCGSTAMP_NEW
>>>> +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
>>>> +#endif
>>>> +#ifdef SIOCGSTAMPNS_NEW
>>>> +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
>>>> +#endif
>>>>
>>>>    IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
>>>>    IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>>> index b187c1281d..f13e260b02 100644
>>>> --- a/linux-user/syscall.c
>>>> +++ b/linux-user/syscall.c
>>>> @@ -37,6 +37,7 @@
>>>>  #include <sched.h>
>>>>  #include <sys/timex.h>
>>>>  #include <sys/socket.h>
>>>> +#include <linux/sockios.h>
>>>>  #include <sys/un.h>
>>>>  #include <sys/uio.h>
>>>>  #include <poll.h>
>>>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>>>> index 7f141f699c..7830b600e7 100644
>>>> --- a/linux-user/syscall_defs.h
>>>> +++ b/linux-user/syscall_defs.h
>>>> @@ -750,6 +750,11 @@ struct target_pollfd {
>>>>
>>>>  #define TARGET_SIOCGSTAMP      0x8906          /* Get stamp (timeval) */
>>>>  #define TARGET_SIOCGSTAMPNS    0x8907          /* Get stamp (timespec) */
>>>> +#define TARGET_SIOCGSTAMP_OLD   0x8906          /* Get stamp (timeval) */
>>>> +#define TARGET_SIOCGSTAMPNS_OLD 0x8907          /* Get stamp (timespec) */
>>>> +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
>>>> +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
>>> kernel defines:
>>>
>>> #define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
>>> #define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])
>>>
>>> So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])
>>>
>>> Their codes are 0x80108906 and 80108907.
>>
>> Hi,
>> I found the discussion around this topic being almost a month old.
>> And related to this fedora bug [1] was closed by adding [2] which
>> matches [3] that was nacked in the discussion here.
>>
>> Since I found nothing later (neither qemu commits nor further
>> discussions) I wonder if it has fallen through the cracks OR if there
>> was a kernel fix/change to resolve it (if that is the case a pointer
>> to the related kernel change would be nice)?
> 
> I didn't have time to address the feedback to this v2, and since the
> immediate problem for Fedora has a workaround, its been lower priority
> especially since my understanding of linux-iser is limited.
> 
> IOW, If anyone wants to take over my patch proposal here feel free. It
> would obviously be nice to fix for this 4.1 release if practical.

Same for me. I have never dealt with linux-user and my workaround was the
simplest thing that I could come up with. Would be good it Laurent or other
linux-user experts could do the "right thing".


Adding Peter, for your awareness. qemu does not build with newer kernel headers.

> 
>>
>> [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1718926
>> [2]: https://src.fedoraproject.org/rpms/qemu/blob/master/f/0005-NOT-UPSTREAM-Build-fix-with-latest-kernel.patch
>> [3]: https://patchew.org/QEMU/20190604071915.288045-1-borntraeger@de.ibm.com/
>>
>>> Thanks,
>>> Laurent
>>>
>>
>>
>> -- 
>> Christian Ehrhardt
>> Software Engineer, Ubuntu Server
>> Canonical Ltd
>>
> 
> Regards,
> Daniel
> 

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

[Laurent Vivier]

Le 11/07/2019 à 11:24, Daniel P. Berrangé a écrit :
> On Thu, Jul 11, 2019 at 10:02:01AM +0200, Christian Ehrhardt wrote:
>> On Mon, Jun 17, 2019 at 5:35 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>>
>>> Le 17/06/2019 à 15:11, Daniel P. Berrangé a écrit :
>>>> The SIOCGSTAMP symbol was previously defined in the
>>>> asm-generic/sockios.h header file. QEMU sees that header
>>>> indirectly via sys/socket.h
>>>>
>>>> In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
>>>> the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
>>>> Instead it provides only SIOCGSTAMP_OLD, which only uses a
>>>> 32-bit time_t on 32-bit architectures.
>>>>
>>>> The linux/sockios.h header then defines SIOCGSTAMP using
>>>> either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
>>>> SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
>>>> on 32-bit architectures
>>>>
>>>> To cope with this we must now define two separate syscalls,
>>>> with corresponding old and new sizes, as well as including
>>>> the new linux/sockios.h header.
>>>>
>>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> ---
>>>>  linux-user/ioctls.h        | 15 +++++++++++++++
>>>>  linux-user/syscall.c       |  1 +
>>>>  linux-user/syscall_defs.h  |  5 +++++
>>>>  linux-user/syscall_types.h |  4 ++++
>>>>  4 files changed, 25 insertions(+)
>>>>
>>>> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
>>>> index 5e84dc7c3a..5a6d6def7e 100644
>>>> --- a/linux-user/ioctls.h
>>>> +++ b/linux-user/ioctls.h
>>>> @@ -222,8 +222,23 @@
>>>>    IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
>>>>    IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
>>>>    IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
>>>> +
>>>> +#ifdef SIOCGSTAMP_OLD
>>>> +  IOCTL(SIOCGSTAMP_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
>>>> +#else
>>>>    IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
>>>> +#endif
>>>> +#ifdef SIOCGSTAMPNS_OLD
>>>> +  IOCTL(SIOCGSTAMPNS_OLD, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
>>>> +#else
>>>>    IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
>>>> +#endif
>>>> +#ifdef SIOCGSTAMP_NEW
>>>> +  IOCTL(SIOCGSTAMP_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval64)))
>>>> +#endif
>>>> +#ifdef SIOCGSTAMPNS_NEW
>>>> +  IOCTL(SIOCGSTAMPNS_NEW, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec64)))
>>>> +#endif
>>>>
>>>>    IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
>>>>    IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>>> index b187c1281d..f13e260b02 100644
>>>> --- a/linux-user/syscall.c
>>>> +++ b/linux-user/syscall.c
>>>> @@ -37,6 +37,7 @@
>>>>  #include <sched.h>
>>>>  #include <sys/timex.h>
>>>>  #include <sys/socket.h>
>>>> +#include <linux/sockios.h>
>>>>  #include <sys/un.h>
>>>>  #include <sys/uio.h>
>>>>  #include <poll.h>
>>>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>>>> index 7f141f699c..7830b600e7 100644
>>>> --- a/linux-user/syscall_defs.h
>>>> +++ b/linux-user/syscall_defs.h
>>>> @@ -750,6 +750,11 @@ struct target_pollfd {
>>>>
>>>>  #define TARGET_SIOCGSTAMP      0x8906          /* Get stamp (timeval) */
>>>>  #define TARGET_SIOCGSTAMPNS    0x8907          /* Get stamp (timespec) */
>>>> +#define TARGET_SIOCGSTAMP_OLD   0x8906          /* Get stamp (timeval) */
>>>> +#define TARGET_SIOCGSTAMPNS_OLD 0x8907          /* Get stamp (timespec) */
>>>> +#define TARGET_SIOCGSTAMP_NEW   TARGET_IOC(TARGET_IOC_READ, 's', 6, sizeof(long long) + sizeof(long)) /* Get stamp (timeval64) */
>>>> +#define TARGET_SIOCGSTAMPNS_NEW TARGET_IOC(TARGET_IOC_READ, 's', 7, sizeof(long long) + sizeof(long)) /* Get stamp (timespec64) */
>>> kernel defines:
>>>
>>> #define SIOCGSTAMP_NEW   _IOR(SOCK_IOC_TYPE, 0x06, long long[2])
>>> #define SIOCGSTAMPNS_NEW _IOR(SOCK_IOC_TYPE, 0x07, long long[2])
>>>
>>> So it should be TARGET_IOR(0x89, 0x6, abi_llong[2])
>>>
>>> Their codes are 0x80108906 and 80108907.
>>
>> Hi,
>> I found the discussion around this topic being almost a month old.
>> And related to this fedora bug [1] was closed by adding [2] which
>> matches [3] that was nacked in the discussion here.
>>
>> Since I found nothing later (neither qemu commits nor further
>> discussions) I wonder if it has fallen through the cracks OR if there
>> was a kernel fix/change to resolve it (if that is the case a pointer
>> to the related kernel change would be nice)?
> 
> I didn't have time to address the feedback to this v2, and since the
> immediate problem for Fedora has a workaround, its been lower priority
> especially since my understanding of linux-iser is limited.
> 
> IOW, If anyone wants to take over my patch proposal here feel free. It
> would obviously be nice to fix for this 4.1 release if practical.

I'm going to try to update your patch.

Thanks,
Laurent