Re: [PATCH] linux-user: Add translation for argument of msync()

[Helge Deller]

On 12/15/22 08:58, Philippe Mathieu-Daudé wrote:
> On 15/12/22 08:27, Helge Deller wrote:
> > msync() uses the flags MS_ASYNC, MS_INVALIDATE and MS_SYNC, which differ
> > between platforms, specifcally on alpha and hppa.
> >
> > Add a target to host translation for those and wire up a nicer strace
> > output.
> >
> > This fixes the testsuite of the macaulay2 debian package with a hppa-linux
> > guest on a x86-64 host.
> >
> > Signed-off-by: Helge Deller <deller@gmx.de>
> >
> > diff --git a/linux-user/alpha/target_mman.h b/linux-user/alpha/target_mman.h
> > index cd6e3d70a6..051544f5ab 100644
> > --- a/linux-user/alpha/target_mman.h
> > +++ b/linux-user/alpha/target_mman.h
> > @@ -3,6 +3,10 @@
> >
> >   #define TARGET_MADV_DONTNEED 6
> >
> > +#define TARGET_MS_ASYNC 1
> > +#define TARGET_MS_SYNC 2
> > +#define TARGET_MS_INVALIDATE 4
> > +
> >   #include "../generic/target_mman.h"
> >
> >   #endif
> > diff --git a/linux-user/generic/target_mman.h b/linux-user/generic/target_mman.h
> > index 1436a3c543..32bf1a52d0 100644
> > --- a/linux-user/generic/target_mman.h
> > +++ b/linux-user/generic/target_mman.h
> > @@ -89,4 +89,17 @@
> >   #define TARGET_MADV_DONTNEED_LOCKED 24
> >   #endif
> >
> > +
> > +#ifndef TARGET_MS_ASYNC
> > +#define TARGET_MS_ASYNC 1
>
> Hmm don't we want to keep the host flag instead?
>
>     #define TARGET_MS_ASYNC MS_ASYNC

Yes, that would be possible, but the value is the same.
In the <arch>/*h files you usually want to have numerical values
which makes it easier to search for conversion bugs.

I'd prefer to keep it as is, it's done for the other
files/values like that.

Helge


> > +#endif
> > +
> > +#ifndef TARGET_MS_INVALIDATE
> > +#define TARGET_MS_INVALIDATE 2
>
> Ditto,
>
> > +#endif
> > +
> > +#ifndef TARGET_MS_SYNC
> > +#define TARGET_MS_SYNC 4
>
> ditto.
>
> LGTM otherwise.
>
> > +#endif
> > +
> >   #endif
> > diff --git a/linux-user/hppa/target_mman.h b/linux-user/hppa/target_mman.h
> > index 66dd9f7941..f9b6b97032 100644
> > --- a/linux-user/hppa/target_mman.h
> > +++ b/linux-user/hppa/target_mman.h
> > @@ -10,6 +10,10 @@
> >   #define TARGET_MADV_WIPEONFORK 71
> >   #define TARGET_MADV_KEEPONFORK 72
> >
> > +#define TARGET_MS_SYNC 1
> > +#define TARGET_MS_ASYNC 2
> > +#define TARGET_MS_INVALIDATE 4
> > +
> >   #include "../generic/target_mman.h"
> >
> >   #endif
> > diff --git a/linux-user/strace.list b/linux-user/strace.list
> > index a75101fca1..ac8f872371 100644
> > --- a/linux-user/strace.list
> > +++ b/linux-user/strace.list
> > @@ -650,7 +650,7 @@
> >   { TARGET_NR_msgsnd, "msgsnd" , NULL, NULL, NULL },
> >   #endif
> >   #ifdef TARGET_NR_msync
> > -{ TARGET_NR_msync, "msync" , NULL, NULL, NULL },
> > +{ TARGET_NR_msync, "msync" , "%s(%p,%u,%d)", NULL, NULL },
> >   #endif
> >   #ifdef TARGET_NR_multiplexer
> >   { TARGET_NR_multiplexer, "multiplexer" , NULL, NULL, NULL },
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index d58e9b8d10..e541fbe09a 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -22,6 +22,7 @@
> >   #include "qemu/path.h"
> >   #include "qemu/memfd.h"
> >   #include "qemu/queue.h"
> > +#include "target_mman.h"
> >   #include <elf.h>
> >   #include <endian.h>
> >   #include <grp.h>
> > @@ -7667,6 +7668,14 @@ static inline int target_to_host_mlockall_arg(int arg)
> >   }
> >   #endif
> >
> > +static inline int target_to_host_msync_arg(abi_long arg)
> > +{
> > +    return ((arg & TARGET_MS_ASYNC) ? MS_ASYNC : 0) |
> > +           ((arg & TARGET_MS_INVALIDATE) ? MS_INVALIDATE : 0) |
> > +           ((arg & TARGET_MS_SYNC) ? MS_SYNC : 0) |
> > +           (arg & ~(TARGET_MS_ASYNC | TARGET_MS_INVALIDATE | TARGET_MS_SYNC));
> > +}
> > +
> >   #if (defined(TARGET_NR_stat64) || defined(TARGET_NR_lstat64) ||     \
> >        defined(TARGET_NR_fstat64) || defined(TARGET_NR_fstatat64) ||  \
> >        defined(TARGET_NR_newfstatat))
> > @@ -10163,7 +10172,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
> > num, abi_long arg1,
> >           /* ??? msync/mlock/munlock are broken for softmmu.  */
> >   #ifdef TARGET_NR_msync
> >       case TARGET_NR_msync:
> > -        return get_errno(msync(g2h(cpu, arg1), arg2, arg3));
> > +        return get_errno(msync(g2h(cpu, arg1), arg2,
> > +                               target_to_host_msync_arg(arg3)));
> >   #endif
> >   #ifdef TARGET_NR_mlock
> >       case TARGET_NR_mlock: