| Message ID | 153130635623.17872.6735154721391367314.stgit@lep8c.aus.stglabs.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: ppc64: use the correct values for F_*LK64s | expand |
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 <sbhat@linux.vnet.ibm.com> > --- > 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
Le 11/07/2018 à 15:04, Laurent Vivier a écrit : > 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 <sbhat@linux.vnet.ibm.com> >> --- >> 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__) Moreover, I think the "defined(__linux__)" is not needed as linux-user/syscall.c is compiled only on linux. Thanks, Laurent
Le 11/07/2018 à 15:04, Laurent Vivier a écrit : > 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 <sbhat@linux.vnet.ibm.com> >> --- >> 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; But as you said, __USE_FILE_OFFSET64 is defined in qemu, and F_GETLK is equal to F_GETLK64, so we should use something like: ... host_cmd -= F_GETLK64 - 5; ... Thanks, Laurent
On 07/12/2018 02:21 AM, Laurent Vivier wrote: > Le 11/07/2018 à 15:04, Laurent Vivier a écrit : >> 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 <sbhat@linux.vnet.ibm.com> >>> --- >>> 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; > But as you said, __USE_FILE_OFFSET64 is defined in qemu, and F_GETLK is > equal to F_GETLK64, so we should use something like: > > ... > host_cmd -= F_GETLK64 - 5; > ... Hi Laurent, Thanks for the comments. I agree to the comments, sending the v2 accordingly. Thanks, Shivaprasad > > Thanks, > Laurent >
Le 12/07/2018 à 09:00, Shivaprasad G Bhat a écrit : > > > On 07/12/2018 02:21 AM, Laurent Vivier wrote: >> Le 11/07/2018 à 15:04, Laurent Vivier a écrit : >>> 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 <sbhat@linux.vnet.ibm.com> >>>> --- >>>> 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; >> But as you said, __USE_FILE_OFFSET64 is defined in qemu, and F_GETLK is >> equal to F_GETLK64, so we should use something like: >> >> ... >> host_cmd -= F_GETLK64 - 5; >> ... Hi Shivaprasad, > Hi Laurent, Thanks for the comments. I agree to the comments, > sending the v2 accordingly. Thank you. I did some tests, (qemu-hppa on ppc64 with dpkg), and we need the conversion with TARGET_NR_fcntl64 too because it also calls safe_fcntl() for TARGET_F_SETLK64 and TARGET_F_SETLKW64. Thanks, Laurent
On 07/12/2018 12:36 PM, Laurent Vivier wrote: > Le 12/07/2018 à 09:00, Shivaprasad G Bhat a écrit : >> >> On 07/12/2018 02:21 AM, Laurent Vivier wrote: >>> Le 11/07/2018 à 15:04, Laurent Vivier a écrit : >>>> 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 <sbhat@linux.vnet.ibm.com> >>>>> --- >>>>> 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; >>> But as you said, __USE_FILE_OFFSET64 is defined in qemu, and F_GETLK is >>> equal to F_GETLK64, so we should use something like: >>> >>> ... >>> host_cmd -= F_GETLK64 - 5; >>> ... > Hi Shivaprasad, > >> Hi Laurent, Thanks for the comments. I agree to the comments, >> sending the v2 accordingly. > Thank you. > > I did some tests, (qemu-hppa on ppc64 with dpkg), and we need the > conversion with TARGET_NR_fcntl64 too because it also calls safe_fcntl() > for TARGET_F_SETLK64 and TARGET_F_SETLKW64. I moved the adjustment code inside target_to_host_fcntl_cmd() to address all cases for future too. Just sent the v2. Couldn't get the hppa config to work on my setup, and that part is not tested. Regards, Shivaprasad > > Thanks, > Laurent >
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
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 <sbhat@linux.vnet.ibm.com> --- linux-user/syscall.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)