[Qemu-devel] [PATCH v8 0/2] linux-user: Fix assorted Qemu user mode issues

[Qemu-devel] [PATCH v8 0/2] linux-user: Fix assorted Qemu user mode issues

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

v7->v8: 

    - patches for clock_adjtime() and syncfs() fixed so that certain older
      system can detect absence of support for these system calls and exclude
      appropriate code segments from building
    - implemented better error condition handling for clock_adjtime()
    - added strace support for syncfs()
    - improved commit messages 
    
NOTE: v8 contains only these two patches, since it was noticed that older
      systems may have build problems with such support; other patches are
      fine in that sense. Also, these two patches in v8 assume that patch 1
      from v7 (adjtimex() support) is already applied.

v6->v7:

    - rebased to the latest code (there was a large linux-user change since
          v6, consisting of 26 patches)
    - slightly changed order of patches
    - changed PATH_MAX to 128 in sysfs() patch (last remaining item
          that was supposed to be in the previous version) 

v5->v6:

    - rebased to the latest code
    - reworked all patches according to review feedback
    - added two new patches on syncfs() and mq_open()
    - some improvements in commit messages

v4->v5:

    - removed three cleanup patches

v3->v4:

    - rebased to the latest code
    - added patch on clock_adjtime() support
    - minor commit messages improvements

v2->v3:

    - rebased to the latest code
    - merged patches on adjtimex(), sysfs(), and ustat() from another series
    - added patch on socketcall() support
    - cleanup patches reorganized

v1->v2:

    - improved usage of "#ifdefs" in patch on syslog()
    - removed EIDRM-related code from patch on msgrcv(), since this error
      code is already handled well
    - added three cleanup patches

(v1 for some reason did not appear on qemu-devel, but mails are sent)

This series fixes certain Qemu user mode issues. The fixes mainly originate
from observation of LTP tests failures for execution in Qemu user mode on
various platforms.

Aleksandar Markovic (2):
  linux-user: Add support for clock_adjtime() syscall
  linux-user: Add support for syncfs() syscall

 configure              | 36 ++++++++++++++++++++++++
 linux-user/strace.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list |  5 +++-
 linux-user/syscall.c   | 30 ++++++++++++++++++++
 4 files changed, 146 insertions(+), 1 deletion(-)

-- 
2.9.3

[Qemu-devel] [PATCH v8 1/2] linux-user: Add support for clock_adjtime() syscall

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

This patch implements Qemu user mode clock_adjtime() syscall support.

The implementation is based on invocation of host's clock_adjtime().
There are fairly complex rules regarding conditions for each error
code that could be returned by clock_adjtime(). They are all taken
into account while conversions of pointers to timex structure between
target and host are done. By passing NULL as the second argument
in certain cases, it is assured that the emulated clock_adjtime()
will return the correct error code for all cases of its input.

Signed-off-by: Aleksandar Rikalo <aleksandar.rikalo@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
 configure              | 18 ++++++++++++
 linux-user/strace.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list |  3 ++
 linux-user/syscall.c   | 25 +++++++++++++++++
 4 files changed, 122 insertions(+)

diff --git a/configure b/configure
index 5751d8e..044e0cd 100755
--- a/configure
+++ b/configure
@@ -3898,6 +3898,21 @@ if compile_prog "" "" ; then
   setns=yes
 fi
 
+# clock_adjtime probe
+clock_adjtime=no
+cat > $TMPC <<EOF
+#include <time.h>
+
+int main(void)
+{
+    return clock_adjtime(0, 0);
+}
+EOF
+clock_adjtime=no
+if compile_prog "" "" ; then
+  clock_adjtime=yes
+fi
+
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
   if has makeinfo && has pod2man; then
@@ -5183,6 +5198,9 @@ fi
 if test "$setns" = "yes" ; then
   echo "CONFIG_SETNS=y" >> $config_host_mak
 fi
+if test "$clock_adjtime" = "yes" ; then
+  echo "CONFIG_CLOCK_ADJTIME=y" >> $config_host_mak
+fi
 if test "$inotify" = "yes" ; then
   echo "CONFIG_INOTIFY=y" >> $config_host_mak
 fi
diff --git a/linux-user/strace.c b/linux-user/strace.c
index f37b386..a61717d 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -435,6 +435,69 @@ print_fdset(int n, abi_ulong target_fds_addr)
 }
 #endif
 
+#ifdef TARGET_NR_clock_adjtime
+/* IDs of the various system clocks */
+#define TARGET_CLOCK_REALTIME              0
+#define TARGET_CLOCK_MONOTONIC             1
+#define TARGET_CLOCK_PROCESS_CPUTIME_ID    2
+#define TARGET_CLOCK_THREAD_CPUTIME_ID     3
+#define TARGET_CLOCK_MONOTONIC_RAW         4
+#define TARGET_CLOCK_REALTIME_COARSE       5
+#define TARGET_CLOCK_MONOTONIC_COARSE      6
+#define TARGET_CLOCK_BOOTTIME              7
+#define TARGET_CLOCK_REALTIME_ALARM        8
+#define TARGET_CLOCK_BOOTTIME_ALARM        9
+#define TARGET_CLOCK_SGI_CYCLE             10
+#define TARGET_CLOCK_TAI                   11
+
+static void
+print_clockid(int clockid, int last)
+{
+    switch (clockid) {
+    case TARGET_CLOCK_REALTIME:
+        gemu_log("CLOCK_REALTIME");
+        break;
+    case TARGET_CLOCK_MONOTONIC:
+        gemu_log("CLOCK_MONOTONIC");
+        break;
+    case TARGET_CLOCK_PROCESS_CPUTIME_ID:
+        gemu_log("CLOCK_PROCESS_CPUTIME_ID");
+        break;
+    case TARGET_CLOCK_THREAD_CPUTIME_ID:
+        gemu_log("CLOCK_THREAD_CPUTIME_ID");
+        break;
+    case TARGET_CLOCK_MONOTONIC_RAW:
+        gemu_log("CLOCK_MONOTONIC_RAW");
+        break;
+    case TARGET_CLOCK_REALTIME_COARSE:
+        gemu_log("CLOCK_REALTIME_COARSE");
+        break;
+    case TARGET_CLOCK_MONOTONIC_COARSE:
+        gemu_log("CLOCK_MONOTONIC_COARSE");
+        break;
+    case TARGET_CLOCK_BOOTTIME:
+        gemu_log("CLOCK_BOOTTIME");
+        break;
+    case TARGET_CLOCK_REALTIME_ALARM:
+        gemu_log("CLOCK_REALTIME_ALARM");
+        break;
+    case TARGET_CLOCK_BOOTTIME_ALARM:
+        gemu_log("CLOCK_BOOTTIME_ALARM");
+        break;
+    case TARGET_CLOCK_SGI_CYCLE:
+        gemu_log("CLOCK_SGI_CYCLE");
+        break;
+    case TARGET_CLOCK_TAI:
+        gemu_log("CLOCK_TAI");
+        break;
+    default:
+        gemu_log("%d", clockid);
+        break;
+    }
+    gemu_log("%s", get_comma(last));
+}
+#endif
+
 /*
  * Sysycall specific output functions
  */
@@ -1096,6 +1159,19 @@ print_chmod(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_clock_adjtime
+static void
+print_clock_adjtime(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_clockid(arg0, 0);
+    print_pointer(arg1, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #ifdef TARGET_NR_clone
 static void do_print_clone(unsigned int flags, abi_ulong newsp,
                            abi_ulong parent_tidptr, target_ulong newtls,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index f6dd044..9355075 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -79,6 +79,9 @@
 #ifdef TARGET_NR_chroot
 { TARGET_NR_chroot, "chroot" , NULL, NULL, NULL },
 #endif
+#ifdef TARGET_NR_clock_adjtime
+{ TARGET_NR_clock_adjtime, "clock_adjtime" , NULL, print_clock_adjtime, NULL },
+#endif
 #ifdef TARGET_NR_clock_getres
 { TARGET_NR_clock_getres, "clock_getres" , NULL, NULL, NULL },
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 35363cc..25be16b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -48,6 +48,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
 #include <sys/shm.h>
 #include <sys/sem.h>
 #include <sys/statfs.h>
+#include <time.h>
 #include <utime.h>
 #include <sys/sysinfo.h>
 #include <sys/signalfd.h>
@@ -9624,6 +9625,30 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             }
         }
         break;
+#if defined(TARGET_NR_clock_adjtime) && defined(CONFIG_CLOCK_ADJTIME)
+    case TARGET_NR_clock_adjtime:
+        {
+            struct timex htx, *phtx = &htx;
+
+            if (target_to_host_timex(phtx, arg2) != 0) {
+                /*
+                 * rather than returning -EFAULT, we here pass NULL as the
+                 * second argument of host's clock_adjtime, in order to let
+                 * the logic inside the kernel decide whether the returned
+                 * error code should be EFAULT, or perhaps, under some
+                 * circumstances related to arg1, EINVAL or EOPNOTSUPP
+                 */
+                phtx = NULL;
+            }
+            ret = get_errno(clock_adjtime(arg1, phtx));
+            if (!is_error(ret) && phtx) {
+                if (host_to_target_timex(arg2, phtx) != 0) {
+                    goto efault;
+                }
+            }
+        }
+        break;
+#endif
 #ifdef TARGET_NR_create_module
     case TARGET_NR_create_module:
 #endif
-- 
2.9.3

Re: [Qemu-devel] [PATCH v8 1/2] linux-user: Add support for clock_adjtime() syscall

[Peter Maydell]

On 10 October 2016 at 11:18, Aleksandar Markovic
<aleksandar.markovic@rt-rk.com> wrote:
> From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
>
> This patch implements Qemu user mode clock_adjtime() syscall support.
>
> The implementation is based on invocation of host's clock_adjtime().
> There are fairly complex rules regarding conditions for each error
> code that could be returned by clock_adjtime(). They are all taken
> into account while conversions of pointers to timex structure between
> target and host are done. By passing NULL as the second argument
> in certain cases, it is assured that the emulated clock_adjtime()
> will return the correct error code for all cases of its input.

Is this ordering of which error conditions are checked first
actually mandated by the clock_adjtime() documentation, or is
this just placating an LTP test case that does two wrong things
at once when it's really only trying to test one of them?
If the latter, I think it's better to fix the broken LTP test
case. (For instance
https://github.com/linux-test-project/ltp/commit/3d4b0e9aa524fc6418d34614299ac2df1e8045ae
is a fix for an issue like that in handling the read syscall.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 1/2] linux-user: Add support for clock_adjtime() syscall

[Aleksandar Markovic]

Hi,

Thanks for bringing up this issue. LTP was not the reference for this implementation. Man page does not exist. The reference was kernel source. Apart from source code analysis. test examples were utilized to confirm the same behavior on real and emulated systems.

Yours,
Aleksandar


________________________________________
From: Peter Maydell [peter.maydell@linaro.org]
Sent: Monday, October 10, 2016 3:25 AM
To: Aleksandar Markovic
Cc: QEMU Developers; Riku Voipio; Laurent Vivier; Petar Jovanovic; Miodrag Dinic; Aleksandar Rikalo; Aleksandar Markovic
Subject: Re: [PATCH v8 1/2] linux-user: Add support for clock_adjtime() syscall

On 10 October 2016 at 11:18, Aleksandar Markovic
<aleksandar.markovic@rt-rk.com> wrote:
> From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
>
> This patch implements Qemu user mode clock_adjtime() syscall support.
>
> The implementation is based on invocation of host's clock_adjtime().
> There are fairly complex rules regarding conditions for each error
> code that could be returned by clock_adjtime(). They are all taken
> into account while conversions of pointers to timex structure between
> target and host are done. By passing NULL as the second argument
> in certain cases, it is assured that the emulated clock_adjtime()
> will return the correct error code for all cases of its input.

Is this ordering of which error conditions are checked first
actually mandated by the clock_adjtime() documentation, or is
this just placating an LTP test case that does two wrong things
at once when it's really only trying to test one of them?
If the latter, I think it's better to fix the broken LTP test
case. (For instance
https://github.com/linux-test-project/ltp/commit/3d4b0e9aa524fc6418d34614299ac2df1e8045ae
is a fix for an issue like that in handling the read syscall.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 1/2] linux-user: Add support for clock_adjtime() syscall

[Peter Maydell]

On 10 October 2016 at 11:34, Aleksandar Markovic
<Aleksandar.Markovic@imgtec.com> wrote:
> Thanks for bringing up this issue. LTP was not the reference for this
> implementation. Man page does not exist. The reference was kernel source.
> Apart from source code analysis. test examples were utilized to confirm
> the same behavior on real and emulated systems.

If no real world code or even LTP test code is relying on the
order in which error conditions are checked for, and the API
is not documented to make that kind of distinction, then I think
we should implement this the same way we do all other syscalls,
ie just check for EFAULT first.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v8 1/2] linux-user: Add support for clock_adjtime() syscall

[Aleksandar Markovic]

Sure, I am going to send a new and corrected version of the series in short time.

Thanks,
Aleksandar
________________________________________
From: Peter Maydell [peter.maydell@linaro.org]
Sent: Monday, October 10, 2016 3:35 AM
To: Aleksandar Markovic
Cc: Aleksandar Markovic; QEMU Developers; Riku Voipio; Laurent Vivier; Petar Jovanovic; Miodrag Dinic; Aleksandar Rikalo
Subject: Re: [PATCH v8 1/2] linux-user: Add support for clock_adjtime() syscall

On 10 October 2016 at 11:34, Aleksandar Markovic
<Aleksandar.Markovic@imgtec.com> wrote:
> Thanks for bringing up this issue. LTP was not the reference for this
> implementation. Man page does not exist. The reference was kernel source.
> Apart from source code analysis. test examples were utilized to confirm
> the same behavior on real and emulated systems.

If no real world code or even LTP test code is relying on the
order in which error conditions are checked for, and the API
is not documented to make that kind of distinction, then I think
we should implement this the same way we do all other syscalls,
ie just check for EFAULT first.

thanks
-- PMM

[Qemu-devel] [PATCH v8 2/2] linux-user: Add support for syncfs() syscall

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

This patch implements Qemu user mode syncfs() syscall support. Syscall
syncfs() syncs the filesystem containing file determined by the open
file descriptor passed as the argument to syncfs().

The implementation consists of a straightforward invocation of host's
syncfs(). Configure and strace support is included as well.

Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
 configure              | 18 ++++++++++++++++++
 linux-user/strace.list |  2 +-
 linux-user/syscall.c   |  5 +++++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 044e0cd..7b9984b 100755
--- a/configure
+++ b/configure
@@ -3913,6 +3913,21 @@ if compile_prog "" "" ; then
   clock_adjtime=yes
 fi
 
+# syncfs probe
+syncfs=no
+cat > $TMPC <<EOF
+#include <unistd.h>
+
+int main(void)
+{
+    return syncfs(0);
+}
+EOF
+syncfs=no
+if compile_prog "" "" ; then
+  syncfs=yes
+fi
+
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
   if has makeinfo && has pod2man; then
@@ -5201,6 +5216,9 @@ fi
 if test "$clock_adjtime" = "yes" ; then
   echo "CONFIG_CLOCK_ADJTIME=y" >> $config_host_mak
 fi
+if test "$syncfs" = "yes" ; then
+  echo "CONFIG_SYNCFS=y" >> $config_host_mak
+fi
 if test "$inotify" = "yes" ; then
   echo "CONFIG_INOTIFY=y" >> $config_host_mak
 fi
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 9355075..c549f99 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1462,7 +1462,7 @@
 { TARGET_NR_sync_file_range, "sync_file_range" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_syncfs
-{ TARGET_NR_syncfs, "syncfs" , NULL, NULL, NULL },
+{ TARGET_NR_syncfs, "syncfs" , "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_syscall
 { TARGET_NR_syscall, "syscall" , NULL, NULL, NULL },
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 25be16b..850f4cc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8071,6 +8071,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         sync();
         ret = 0;
         break;
+#if defined(TARGET_NR_syncfs) && defined(CONFIG_SYNCFS)
+    case TARGET_NR_syncfs:
+        ret = get_errno(syncfs(arg1));
+        break;
+#endif
     case TARGET_NR_kill:
         ret = get_errno(safe_kill(arg1, target_to_host_signal(arg2)));
         break;
-- 
2.9.3