[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] linux-user: ppc64: use the correct values for F
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH] linux-user: ppc64: use the correct values for F_*LK64s |
Date: |
Wed, 11 Jul 2018 15:04:53 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
Le 11/07/2018 à 12:55, Shivaprasad G Bhat a écrit :
> Qemu includes the glibc headers for the host defines and target headers are
> part of the qemu source themselves. The glibc has the F_GETLK64, F_SETLK64
> and F_SETLKW64 defined to 12, 13 and 14 for all archs(generic) in
> sysdeps/unix/sysv/linux/bits/fcntl-linux.h. The linux kernel generic
> definition for F_*LK is 5, 6 & 7 and F_*LK64* is 12,13, and 14 as seen in
> include/uapi/asm-generic/fcntl.h. On 64bit machine, by default the kernel
> assumes all F_*LK to 64bit calls and doesnt support use of F_*LK64* as
> can be seen in include/linux/fcntl.h in linux source.
>
> On x86_64 host, the values for F_*LK64* are set to 5, 6 and 7
> explicitly in /usr/include/x86_64-linux-gnu/bits/fcntl.h by the glibc.
> Whereas, a PPC64 host doesn't have such a definition in
> /usr/include/powerpc64le-linux-gnu/bits/fcntl.h by the glibc. So,
> the sources on PPC64 host sees the default value of F_*LK64*
> as 12, 13 & 14(fcntl-linux.h).
>
> Since the 64bit kernel doesnt support 12, 13 & 14; the glibc fcntl syscall
> implementation(__libc_fcntl*(), __fcntl64_nocancel) does the F_*LK64* value
> convertion back to F_*LK* values on PPC64 as seen in
> sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h with FCNTL_ADJUST_CMD()
> macro. Whereas on x86_64 host the values for F_*LK64* are set to 5, 6 and 7
> and no adjustments are needed.
>
> Since qemu doesnt use the glibc fcntl, but makes the safe_syscall* on its
> own, the PPC64 qemu is calling the syscall with 12, 13, and 14(without
> adjustment) and they all fail. The fcntl calls to F_GETLK/F_SETLK|W all
> fail by all pplications run on PPC64 host user emulation.
>
> The fix here could be to see why on PPC64 the glibc is still keeping
> F_*LK64* different from F_*LK and why adjusting them to 5, 6 and 7 before
> the syscall for PPC only. See if we can make the
> /usr/include/powerpc64le-linux-gnu/bits/fcntl.h to have the values
> 5, 6 & 7 just like x86_64 and remove the adjustment code in glibc. That way,
> qemu sources see the kernel supported values in glibc headers.
>
> OR
>
> On PPC64 host, qemu sources see both F_LK* & F_LK64* as same and set to
> 12, 13 and 14 because __USE_FILE_OFFSET64 is defined in qemu
> sources(also refer sysdeps/unix/sysv/linux/bits/fcntl-linux.h).
> Since F_*LK and F_*LK64 are same, the value adjument like done by glibc in
> qemu sources is difficult. So, Overwrite the glibc defaults with the actual
> supported values in Qemu. The current patch is doing this.
>
> Signed-off-by: Shivaprasad G Bhat <address@hidden>
> ---
> linux-user/syscall.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 7b9ac3b408..1693e69ce0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -250,6 +250,20 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4
> arg4,type5 arg5, \
> #define TARGET_NR__llseek TARGET_NR_llseek
> #endif
>
> +/* glibc headers has these defined to 12, 13 and 14 and is not supported
> + * by kernel. The glibc fcntl call actually adjusts them back to 5, 6 and 7
> + * before making the syscall(). Since we make the syscall directly,
> + * overwite/adjust to what is supported by the kernel.
> + */
> +#if defined(__linux__) && defined(__powerpc64__)
> +#undef F_GETLK64
> +#define F_GETLK64 5 /* Get record locking info. */
> +#undef F_SETLK64
> +#define F_SETLK64 6 /* Set record locking info (non-blocking). */
> +#undef F_SETLKW64
> +#define F_SETLKW64 7 /* Set record locking info (blocking). */
> +#endif
> +
> #ifdef __NR_gettid
> _syscall0(int, gettid)
> #else
>
These macros are used in target_to_host_fcntl_cmd(), and this function
is used with safe_fcntl() and fcntl().
So I think it would be cleaner to do the change after
target_to_host_fcntl_cmd() in do_fcntl() as it is done in glibc instead
of redefining system values. Something like:
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6782,6 +6782,12 @@ static abi_long do_fcntl(int fd, int cmd,
abi_ulong arg)
if (host_cmd == -TARGET_EINVAL)
return host_cmd;
+#if defined(__linux__) && defined(__powerpc64__)
+ if (host_cmd >= F_GETLK64 && host_cmd <= F_SETLKW64) {
+ host_cmd -= F_GETLK64 - F_GETLK;
+ }
+#endif
+
switch(cmd) {
case TARGET_F_GETLK:
ret = copy_from_user_flock(&fl64, arg);
Thanks,
Laurent