On 17/06/2019 18:48, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>
>> * sysdeps/unix/sysv/linux/powerpc/dl-vdso.h: New file.
>> * sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c
>> (get_timebase_freq_fallback): New symbol.
>> (__get_timebase_freq): Call vDSO as a function pointer instead of
>> arch-specific macros.
>> * .../unix/sysv/linux/powerpc/gettimeofday.c (gettimeofday): Use
>> get_vdso_symbol.
>
> It's the first time a see a ChangeLog entry starting with '\t* ...'.
> Is this really valid?
>
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/sysdep.h
>> index 20167615c8..cf967a8c33 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/sysdep.h
>> +++ b/sysdeps/unix/sysv/linux/powerpc/sysdep.h
>> @@ -32,3 +33,12 @@
>> # define HAVE_SIGTRAMP_32 "__kernel_sigtramp32"
>> # define HAVE_SIGTRAMP_RT32 "__kernel_sigtramp_rt32"
>> #endif
>> +
>> +#ifndef __ASSEMBLER__
>> +
>> +/* PowerPC vDSO symbols always succeed, so there is no need to emulate a
>> + function call and check for CR0.SO value. */
>> +#define INTERNAL_VSYSCALL_CALL(funcptr, err, nr, args...) \
>> + ({ err = 0; funcptr (args); })
>
> After applying this patch, rt/cpu-clock1 started to fail with:
>
> clock_gettime on reaped PID 26033 clock fffffffffffcd272 => Success
> clock_getres on reaped PID 26033 clock fffffffffffcd272 => Success
>
> Both cases are expecting to receive errno = EINVAL instead of Success.
>
> Isn't CR0.SO important?
>
> Reference:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/vdso64/gettimeofday.S#n190
>
In fact we can't really remove the CR0.SO check because the vDSO symbol
might itself fallback to a syscall (the case of rt/cpu-clock1 where
is check for invalid clockid). The vDSO simplification turned out to
be less than I expected, but it is moving towards consolidate the
time and gettimeofday.
Updated patch below.
---
Subject: [PATCH 08/12] powerpc: Simplify vsyscall internal macros
This patch simplifies the powerpc internal macros for vDSO calls
by:
- Removing INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK, used solely on
get_timebase_freq.
- Adjust INTERNAL_VSYSCALL_CALL_TYPE powerpc32 to follow powerpc64
argument ordering.
- Use HAVE_*_VSYSCALL instead of explicit strings.
- Make powerpc libc-vdso.h include generic implementation.
No semantic change expected, checked on powerpc-linux-gnu-power4,
powerpc64-linux-gnu, and powerpc64le-linux-gnu.
* sysdeps/unix/sysv/linux/libc-vdso.h (VDSO_IFUNC_RET): Define if not
defined.
* sysdeps/unix/sysv/linux/powerpc/get_timebase_freq.c
(__get_timebase_freq): Remove use of
INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK.
(get_timebase_freq_fallback): New symbol.
* sysdeps/unix/sysv/linux/powerpc/gettimeofday.c (time): Use
HAVE_GETTIMEOFDAY_VSYSCALL.
* sysdeps/unix/sysv/linux/powerpc/time.c (gettimeofday): Use
HAVE_TIME_VSYSCALL.
* sysdeps/unix/sysv/linux/powerpc/libc-vdso.h: Include generic
implementation.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
(INTERNAL_VSYSCALL_CALL_TYPE): Make calling convention similar to
powerpc64.
(INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Remove macro.
* .../sysv/linux/powerpc/powerpc64/sysdep.h
(INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK): Likewise.
* sysdeps/unix/sysv/linux/powerpc/sysdep.h
(HAVE_GETTIMEOFDAY_VSYSCALL): Define.
---
sysdeps/unix/sysv/linux/libc-vdso.h | 7 ++++++
.../sysv/linux/powerpc/get_timebase_freq.c | 24 ++++++++++++-------
.../unix/sysv/linux/powerpc/gettimeofday.c | 2 +-
sysdeps/unix/sysv/linux/powerpc/libc-vdso.h | 24 ++++++++++---------
.../sysv/linux/powerpc/powerpc32/sysdep.h | 18 ++------------
.../sysv/linux/powerpc/powerpc64/sysdep.h | 19 +--------------
sysdeps/unix/sysv/linux/powerpc/sysdep.h | 1 +
sysdeps/unix/sysv/linux/powerpc/time.c | 2 +-
8 files changed, 41 insertions(+), 56 deletions(-)
@@ -21,6 +21,13 @@
#define VDSO_SYMBOL(__name) __vdso_##__name
+/* Adjust the return IFUNC value from a vDSO symbol accordingly required
+ architecture ABI. It is used for powerpc64 elfv1 to create a ODP
+ entry since kernvel vDSO does not provide it. */
+#ifndef VDSO_IFUNC_RET
+# define VDSO_IFUNC_RET(__value) (__value)
+#endif
+
#ifdef HAVE_CLOCK_GETTIME_VSYSCALL
extern int (*VDSO_SYMBOL(clock_gettime)) (clockid_t, struct timespec *);
#endif
@@ -23,17 +23,11 @@
#include <not-cancel.h>
#include <libc-vdso.h>
-uint64_t
-__get_timebase_freq (void)
+static uint64_t
+get_timebase_freq_fallback (void)
{
hp_timing_t result = 0L;
-#ifdef SHARED
- /* The vDSO does not return an error (it clear cr0.so on returning). */
- INTERNAL_SYSCALL_DECL (err);
- result =
- INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK (get_tbfreq, err, uint64_t, 0);
-#else
/* We read the information from the /proc filesystem. /proc/cpuinfo
contains at least one line like:
timebase : 33333333
@@ -99,8 +93,20 @@ __get_timebase_freq (void)
}
}
}
-#endif
return result;
}
+
+uint64_t
+__get_timebase_freq (void)
+{
+ /* The vDSO does not have a fallback mechanism (such calling a syscall). */
+ __typeof (VDSO_SYMBOL (get_tbfreq)) vdsop = VDSO_SYMBOL (get_tbfreq);
+ PTR_DEMANGLE (vdsop);
+ if (vdsop == NULL)
+ return get_timebase_freq_fallback ();
+
+ INTERNAL_SYSCALL_DECL (err);
+ return INTERNAL_VSYSCALL_CALL_TYPE (vdsop, err, uint64_t, 0);
+}
weak_alias (__get_timebase_freq, __ppc_get_timebase_freq)
@@ -58,7 +58,7 @@ __gettimeofday_syscall (struct timeval *tv, struct timezone *tz)
}
# define INIT_ARCH() \
- void *vdso_gettimeofday = get_vdso_symbol ("__kernel_gettimeofday")
+ void *vdso_gettimeofday = get_vdso_symbol (HAVE_GETTIMEOFDAY_VSYSCALL)
/* If the vDSO is not available we fall back syscall. */
@@ -22,17 +22,9 @@
#include <sysdep.h>
#include <sysdep-vdso.h>
-#include_next <libc-vdso.h>
-
-extern unsigned long long (*VDSO_SYMBOL(get_tbfreq)) (void);
-#if defined(__PPC64__) || defined(__powerpc64__)
-extern void *VDSO_SYMBOL(sigtramp_rt64);
-#else
-extern void *VDSO_SYMBOL(sigtramp32);
-extern void *VDSO_SYMBOL(sigtramp_rt32);
-#endif
#if (defined(__PPC64__) || defined(__powerpc64__)) && _CALL_ELF != 2
+# include <dl-machine.h>
/* The correct solution is for _dl_vdso_vsym to return the address of the OPD
for the kernel VDSO function. That address would then be stored in the
__vdso_* variables and returned as the result of the IFUNC resolver function.
@@ -51,7 +43,7 @@ extern void *VDSO_SYMBOL(sigtramp_rt32);
are processed immediately at startup the resolver functions and this code need
not be thread-safe, but if the caller writes to a PLT slot it must do so in a
thread-safe manner with all the required barriers. */
-#define VDSO_IFUNC_RET(value) \
+# define VDSO_IFUNC_RET(value) \
({ \
static Elf64_FuncDesc vdso_opd = { .fd_toc = ~0x0 }; \
vdso_opd.fd_func = (Elf64_Addr)value; \
@@ -59,7 +51,17 @@ extern void *VDSO_SYMBOL(sigtramp_rt32);
})
#else
-#define VDSO_IFUNC_RET(value) ((void *) (value))
+# define VDSO_IFUNC_RET(value) ((void *) (value))
+#endif
+
+#include_next <libc-vdso.h>
+
+extern unsigned long long (*VDSO_SYMBOL(get_tbfreq)) (void);
+#if defined(__PPC64__) || defined(__powerpc64__)
+extern void *VDSO_SYMBOL(sigtramp_rt64);
+#else
+extern void *VDSO_SYMBOL(sigtramp32);
+extern void *VDSO_SYMBOL(sigtramp_rt32);
#endif
#endif /* _LIBC_VDSO_H */
@@ -41,7 +41,7 @@
function call, with the exception of LR (which is needed for the
"sc; bnslr+" sequence) and CR (where only CR0.SO is clobbered to signal
an error return status). */
-# define INTERNAL_VSYSCALL_CALL_TYPE(funcptr, err, nr, type, args...) \
+# define INTERNAL_VSYSCALL_CALL_TYPE(funcptr, err, type, nr, args...) \
({ \
register void *r0 __asm__ ("r0"); \
register long int r3 __asm__ ("r3"); \
@@ -69,7 +69,7 @@
})
#define INTERNAL_VSYSCALL_CALL(funcptr, err, nr, args...) \
- INTERNAL_VSYSCALL_CALL_TYPE(funcptr, err, nr, long int, args)
+ INTERNAL_VSYSCALL_CALL_TYPE(funcptr, err, long int, nr, args)
# undef INLINE_SYSCALL
# define INLINE_SYSCALL(name, nr, args...) \
@@ -131,20 +131,6 @@
# undef INTERNAL_SYSCALL_ERRNO
# define INTERNAL_SYSCALL_ERRNO(val, err) (val)
-# define INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK(name, err, type, nr, args...) \
- ({ \
- type sc_ret = ENOSYS; \
- \
- __typeof (__vdso_##name) vdsop = __vdso_##name; \
- PTR_DEMANGLE (vdsop); \
- if (vdsop != NULL) \
- sc_ret = \
- INTERNAL_VSYSCALL_CALL_TYPE (vdsop, err, nr, type, ##args); \
- else \
- err = 1 << 28; \
- sc_ret; \
- })
-
# define LOADARGS_0(name, dummy) \
r0 = name
# define LOADARGS_1(name, __arg1) \
@@ -45,22 +45,6 @@
#endif /* __ASSEMBLER__ */
-/* This version is for internal uses when there is no desire
- to set errno */
-#define INTERNAL_VSYSCALL_NO_SYSCALL_FALLBACK(name, err, type, nr, args...) \
- ({ \
- type sc_ret = ENOSYS; \
- \
- __typeof (__vdso_##name) vdsop = __vdso_##name; \
- PTR_DEMANGLE (vdsop); \
- if (vdsop != NULL) \
- sc_ret = \
- INTERNAL_VSYSCALL_CALL_TYPE (vdsop, err, type, nr, ##args); \
- else \
- err = 1 << 28; \
- sc_ret; \
- })
-
/* Define a macro which expands inline into the wrapper code for a system
call. This use is for internal calls that do not need to handle errors
normally. It will never touch errno. This returns just what the kernel
@@ -94,10 +78,9 @@
#define INTERNAL_VSYSCALL_CALL(funcptr, err, nr, args...) \
INTERNAL_VSYSCALL_CALL_TYPE(funcptr, err, long int, nr, args)
-#undef INLINE_SYSCALL
-
/* This version is for kernels that implement system calls that
behave like function calls as far as register saving. */
+#undef INLINE_SYSCALL
#define INLINE_SYSCALL(name, nr, args...) \
({ \
INTERNAL_SYSCALL_DECL (sc_err); \
@@ -24,6 +24,7 @@
#define HAVE_CLOCK_GETTIME_VSYSCALL "__kernel_clock_gettime"
#define HAVE_GETCPU_VSYSCALL "__kernel_getcpu"
#define HAVE_TIME_VSYSCALL "__kernel_time"
+#define HAVE_GETTIMEOFDAY_VSYSCALL "__kernel_gettimeofday"
#define HAVE_GET_TBFREQ "__kernel_get_tbfreq"
#if defined(__PPC64__) || defined(__powerpc64__)
@@ -67,7 +67,7 @@ time_syscall (time_t *t)
}
# define INIT_ARCH() \
- void *vdso_time = get_vdso_symbol ("__kernel_time");
+ void *vdso_time = get_vdso_symbol (HAVE_TIME_VSYSCALL);
/* If the vDSO is not available we fall back to the syscall. */
libc_ifunc_hidden (__redirect_time, time,