* [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]
@ 2018-01-11 19:41 Tulio Magno Quites Machado Filho
2018-01-11 21:37 ` Aurelien Jarno
0 siblings, 1 reply; 11+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-01-11 19:41 UTC (permalink / raw)
To: libc-alpha; +Cc: aurelien
The tunables framework needs to make syscall early during process
initialization, before the TCB is available for consumption. This
behavior conflicts with powerpc{|64|64le}'s lock elision code, that
tries to abort transactions before a syscall when lock elision is
available and enabled.
This patch adds the macro EARLY_INTERNAL_SYSCALL in order to let early
syscalls happen without depending on the TCB initialization for
powerpc{|64|64le}. Other architectures are redirected to INTERNAL_SYSCALL.
Tested on powerpc{|64|64le}, s390x and x86_64.
2018-01-11 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
[BZ #22685]
* sysdeps/unix/sysdep.h (__EARLY_INTERNAL_SYSCALL0,
__EARLY_INTERNAL_SYSCALL1, __EARLY_INTERNAL_SYSCALL2,
__EARLY_INTERNAL_SYSCALL3, __EARLY_INTERNAL_SYSCALL4,
__EARLY_INTERNAL_SYSCALL5, __EARLY_INTERNAL_SYSCALL6,
__EARLY_INTERNAL_SYSCALL7, EARLY_INTERNAL_SYSCALL_CALL): New macros
(EARLY_INTERNAL_SYSCALL): New macro. Redirect to
INTERNAL_SYSCALL by default.
* sysdeps/unix/sysv/linux/not-errno.h (__access_noerrno):
Replace INTERNAL_SYSCALL_CALL with EARLY_INTERNAL_SYSCALL_CALL.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
(EARLY_INTERNAL_SYSCALL_NCS, EARLY_INTERNAL_SYSCALL): New macros.
* sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
(EARLY_INTERNAL_SYSCALL_NCS, EARLY_INTERNAL_SYSCALL): Likewise.
Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
---
sysdeps/unix/sysdep.h | 25 +++++++++++++++
sysdeps/unix/sysv/linux/not-errno.h | 4 +--
sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h | 36 +++++++++++++++++++++-
sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h | 34 +++++++++++++++++++-
4 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/sysdeps/unix/sysdep.h b/sysdeps/unix/sysdep.h
index aac9303..689272e 100644
--- a/sysdeps/unix/sysdep.h
+++ b/sysdeps/unix/sysdep.h
@@ -57,6 +57,31 @@
#define INTERNAL_SYSCALL_CALL(...) \
__INTERNAL_SYSCALL_DISP (__INTERNAL_SYSCALL, __VA_ARGS__)
+#define __EARLY_INTERNAL_SYSCALL0(name, err) \
+ EARLY_INTERNAL_SYSCALL (name, err, 0)
+#define __EARLY_INTERNAL_SYSCALL1(name, err, a1) \
+ EARLY_INTERNAL_SYSCALL (name, err, 1, a1)
+#define __EARLY_INTERNAL_SYSCALL2(name, err, a1, a2) \
+ EARLY_INTERNAL_SYSCALL (name, err, 2, a1, a2)
+#define __EARLY_INTERNAL_SYSCALL3(name, err, a1, a2, a3) \
+ EARLY_INTERNAL_SYSCALL (name, err, 3, a1, a2, a3)
+#define __EARLY_INTERNAL_SYSCALL4(name, err, a1, a2, a3, a4) \
+ EARLY_INTERNAL_SYSCALL (name, err, 4, a1, a2, a3, a4)
+#define __EARLY_INTERNAL_SYSCALL5(name, err, a1, a2, a3, a4, a5) \
+ EARLY_INTERNAL_SYSCALL (name, err, 5, a1, a2, a3, a4, a5)
+#define __EARLY_INTERNAL_SYSCALL6(name, err, a1, a2, a3, a4, a5, a6) \
+ EARLY_INTERNAL_SYSCALL (name, err, 6, a1, a2, a3, a4, a5, a6)
+#define __EARLY_INTERNAL_SYSCALL7(name, err, a1, a2, a3, a4, a5, a6, a7) \
+ EARLY_INTERNAL_SYSCALL (name, err, 7, a1, a2, a3, a4, a5, a6, a7)
+
+/* It is similar to INTERNAL_SYSCALL_CALL, but it is reserved to system calls
+ during process initialization, when internal structures may not be
+ available, e.g. TCB on powerpc. */
+#define EARLY_INTERNAL_SYSCALL_CALL(...) \
+ __INTERNAL_SYSCALL_DISP (__EARLY_INTERNAL_SYSCALL, __VA_ARGS__)
+#define EARLY_INTERNAL_SYSCALL(name, err, nr, args...) \
+ INTERNAL_SYSCALL (name, err, nr, args)
+
#define __INLINE_SYSCALL0(name) \
INLINE_SYSCALL (name, 0)
#define __INLINE_SYSCALL1(name, a1) \
diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h
index 106ba5c..a033b7b 100644
--- a/sysdeps/unix/sysv/linux/not-errno.h
+++ b/sysdeps/unix/sysv/linux/not-errno.h
@@ -25,9 +25,9 @@ __access_noerrno (const char *pathname, int mode)
int res;
INTERNAL_SYSCALL_DECL (err);
#ifdef __NR_access
- res = INTERNAL_SYSCALL_CALL (access, err, pathname, mode);
+ res = EARLY_INTERNAL_SYSCALL_CALL (access, err, pathname, mode);
#else
- res = INTERNAL_SYSCALL_CALL (faccessat, err, AT_FDCWD, pathname, mode);
+ res = EARLY_INTERNAL_SYSCALL_CALL (faccessat, err, AT_FDCWD, pathname, mode);
#endif
if (INTERNAL_SYSCALL_ERROR_P (res, err))
return INTERNAL_SYSCALL_ERRNO (res, err);
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
index f7277d5..efc6cbc 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
@@ -89,7 +89,10 @@
On powerpc a system call basically clobbers the same registers like a
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). */
+ an error return status).
+
+ Notice it requires the TCB to be allocated and completely set in order to
+ abort transactions before the syscall. */
# undef INTERNAL_SYSCALL_DECL
# define INTERNAL_SYSCALL_DECL(err) long int err __attribute__ ((unused))
@@ -124,6 +127,37 @@
# define INTERNAL_SYSCALL(name, err, nr, args...) \
INTERNAL_SYSCALL_NCS (__NR_##name, err, nr, ##args)
+/* Similar to INTERNAL_SYSCALL, but reserved to early process initialization
+ without requiring the TCB to allocated and completely set. */
+# undef EARLY_INTERNAL_SYSCALL
+# define EARLY_INTERNAL_SYSCALL_NCS(name, err, nr, args...) \
+ ({ \
+ register long int r0 __asm__ ("r0"); \
+ register long int r3 __asm__ ("r3"); \
+ register long int r4 __asm__ ("r4"); \
+ register long int r5 __asm__ ("r5"); \
+ register long int r6 __asm__ ("r6"); \
+ register long int r7 __asm__ ("r7"); \
+ register long int r8 __asm__ ("r8"); \
+ register long int r9 __asm__ ("r9"); \
+ register long int r10 __asm__ ("r10"); \
+ register long int r11 __asm__ ("r11"); \
+ register long int r12 __asm__ ("r12"); \
+ LOADARGS_##nr(name, args); \
+ __asm__ __volatile__ \
+ ("sc \n\t" \
+ "mfcr %0" \
+ : "=&r" (r0), \
+ "=&r" (r3), "=&r" (r4), "=&r" (r5), "=&r" (r6), "=&r" (r7), \
+ "=&r" (r8), "=&r" (r9), "=&r" (r10), "=&r" (r11), "=&r" (r12) \
+ : ASM_INPUT_##nr \
+ : "cr0", "ctr", "memory"); \
+ err = r0; \
+ (int) r3; \
+ })
+# define EARLY_INTERNAL_SYSCALL(name, err, nr, args...) \
+ EARLY_INTERNAL_SYSCALL_NCS (__NR_##name, err, nr, ##args)
+
# undef INTERNAL_SYSCALL_ERROR_P
# define INTERNAL_SYSCALL_ERROR_P(val, err) \
((void) (val), __builtin_expect ((err) & (1 << 28), 0))
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
index 0fc179a..d431fdb 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
@@ -118,7 +118,10 @@
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
gave back in the non-error (CR0.SO cleared) case, otherwise (CR0.SO set)
- the negation of the return value in the kernel gets reverted. */
+ the negation of the return value in the kernel gets reverted.
+
+ Notice it requires the TCB to be allocated and completely set in order to
+ abort transactions before the syscall. */
#undef INTERNAL_SYSCALL
#define INTERNAL_SYSCALL_NCS(name, err, nr, args...) \
@@ -148,6 +151,35 @@
#define INTERNAL_SYSCALL(name, err, nr, args...) \
INTERNAL_SYSCALL_NCS (__NR_##name, err, nr, args)
+/* Similar to INTERNAL_SYSCALL, but reserved to early process initialization
+ without requiring the TCB to allocated and completely set. */
+#undef EARLY_INTERNAL_SYSCALL
+#define EARLY_INTERNAL_SYSCALL_NCS(name, err, nr, args...) \
+ ({ \
+ register long int r0 __asm__ ("r0"); \
+ register long int r3 __asm__ ("r3"); \
+ register long int r4 __asm__ ("r4"); \
+ register long int r5 __asm__ ("r5"); \
+ register long int r6 __asm__ ("r6"); \
+ register long int r7 __asm__ ("r7"); \
+ register long int r8 __asm__ ("r8"); \
+ LOADARGS_##nr (name, ##args); \
+ __asm__ __volatile__ \
+ ("sc\n\t" \
+ "mfcr %0\n\t" \
+ "0:" \
+ : "=&r" (r0), \
+ "=&r" (r3), "=&r" (r4), "=&r" (r5), \
+ "=&r" (r6), "=&r" (r7), "=&r" (r8) \
+ : ASM_INPUT_##nr \
+ : "r9", "r10", "r11", "r12", \
+ "cr0", "ctr", "memory"); \
+ err = r0; \
+ r3; \
+ })
+#define EARLY_INTERNAL_SYSCALL(name, err, nr, args...) \
+ EARLY_INTERNAL_SYSCALL_NCS (__NR_##name, err, nr, args)
+
#undef INTERNAL_SYSCALL_DECL
#define INTERNAL_SYSCALL_DECL(err) long int err __attribute__ ((unused))
--
2.9.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-11 19:41 [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685] Tulio Magno Quites Machado Filho
@ 2018-01-11 21:37 ` Aurelien Jarno
2018-01-11 22:40 ` Adhemerval Zanella
0 siblings, 1 reply; 11+ messages in thread
From: Aurelien Jarno @ 2018-01-11 21:37 UTC (permalink / raw)
To: Tulio Magno Quites Machado Filho; +Cc: libc-alpha
On 2018-01-11 17:27, Tulio Magno Quites Machado Filho wrote:
> The tunables framework needs to make syscall early during process
> initialization, before the TCB is available for consumption. This
> behavior conflicts with powerpc{|64|64le}'s lock elision code, that
> tries to abort transactions before a syscall when lock elision is
> available and enabled.
>
> This patch adds the macro EARLY_INTERNAL_SYSCALL in order to let early
> syscalls happen without depending on the TCB initialization for
> powerpc{|64|64le}. Other architectures are redirected to INTERNAL_SYSCALL.
>
> Tested on powerpc{|64|64le}, s390x and x86_64.
>
> 2018-01-11 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
>
> [BZ #22685]
> * sysdeps/unix/sysdep.h (__EARLY_INTERNAL_SYSCALL0,
> __EARLY_INTERNAL_SYSCALL1, __EARLY_INTERNAL_SYSCALL2,
> __EARLY_INTERNAL_SYSCALL3, __EARLY_INTERNAL_SYSCALL4,
> __EARLY_INTERNAL_SYSCALL5, __EARLY_INTERNAL_SYSCALL6,
> __EARLY_INTERNAL_SYSCALL7, EARLY_INTERNAL_SYSCALL_CALL): New macros
> (EARLY_INTERNAL_SYSCALL): New macro. Redirect to
> INTERNAL_SYSCALL by default.
> * sysdeps/unix/sysv/linux/not-errno.h (__access_noerrno):
> Replace INTERNAL_SYSCALL_CALL with EARLY_INTERNAL_SYSCALL_CALL.
> * sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> (EARLY_INTERNAL_SYSCALL_NCS, EARLY_INTERNAL_SYSCALL): New macros.
> * sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> (EARLY_INTERNAL_SYSCALL_NCS, EARLY_INTERNAL_SYSCALL): Likewise.
>
> Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
> ---
> sysdeps/unix/sysdep.h | 25 +++++++++++++++
> sysdeps/unix/sysv/linux/not-errno.h | 4 +--
> sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h | 36 +++++++++++++++++++++-
> sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h | 34 +++++++++++++++++++-
> 4 files changed, 95 insertions(+), 4 deletions(-)
Thanks for the patch, I have just tested it and I confirm it fixes the
issue.
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-11 21:37 ` Aurelien Jarno
@ 2018-01-11 22:40 ` Adhemerval Zanella
2018-01-11 23:42 ` Florian Weimer
0 siblings, 1 reply; 11+ messages in thread
From: Adhemerval Zanella @ 2018-01-11 22:40 UTC (permalink / raw)
To: libc-alpha
On 11/01/2018 19:36, Aurelien Jarno wrote:
> On 2018-01-11 17:27, Tulio Magno Quites Machado Filho wrote:
>> The tunables framework needs to make syscall early during process
>> initialization, before the TCB is available for consumption. This
>> behavior conflicts with powerpc{|64|64le}'s lock elision code, that
>> tries to abort transactions before a syscall when lock elision is
>> available and enabled.
>>
>> This patch adds the macro EARLY_INTERNAL_SYSCALL in order to let early
>> syscalls happen without depending on the TCB initialization for
>> powerpc{|64|64le}. Other architectures are redirected to INTERNAL_SYSCALL.
>>
>> Tested on powerpc{|64|64le}, s390x and x86_64.
I am not really understanding why exactly this is failing because the only
object that currently uses __access_noerrno, dl-tunables.os, is built with
-DMODULE_NAME=rtld and thus ABORT_TRANSACTION should be an empty statement.
>>
>> 2018-01-11 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
>>
>> [BZ #22685]
>> * sysdeps/unix/sysdep.h (__EARLY_INTERNAL_SYSCALL0,
>> __EARLY_INTERNAL_SYSCALL1, __EARLY_INTERNAL_SYSCALL2,
>> __EARLY_INTERNAL_SYSCALL3, __EARLY_INTERNAL_SYSCALL4,
>> __EARLY_INTERNAL_SYSCALL5, __EARLY_INTERNAL_SYSCALL6,
>> __EARLY_INTERNAL_SYSCALL7, EARLY_INTERNAL_SYSCALL_CALL): New macros
>> (EARLY_INTERNAL_SYSCALL): New macro. Redirect to
>> INTERNAL_SYSCALL by default.
>> * sysdeps/unix/sysv/linux/not-errno.h (__access_noerrno):
>> Replace INTERNAL_SYSCALL_CALL with EARLY_INTERNAL_SYSCALL_CALL.
>> * sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
>> (EARLY_INTERNAL_SYSCALL_NCS, EARLY_INTERNAL_SYSCALL): New macros.
>> * sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
>> (EARLY_INTERNAL_SYSCALL_NCS, EARLY_INTERNAL_SYSCALL): Likewise.
>>
>> Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
>> ---
>> sysdeps/unix/sysdep.h | 25 +++++++++++++++
>> sysdeps/unix/sysv/linux/not-errno.h | 4 +--
>> sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h | 36 +++++++++++++++++++++-
>> sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h | 34 +++++++++++++++++++-
>> 4 files changed, 95 insertions(+), 4 deletions(-)
>
> Thanks for the patch, I have just tested it and I confirm it fixes the
> issue.
>
> Tested-by: Aurelien Jarno <aurelien@aurel32.net>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-11 22:40 ` Adhemerval Zanella
@ 2018-01-11 23:42 ` Florian Weimer
2018-01-12 0:56 ` Adhemerval Zanella
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2018-01-11 23:42 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 01/11/2018 11:40 PM, Adhemerval Zanella wrote:
> I am not really understanding why exactly this is failing because the only
> object that currently uses __access_noerrno, dl-tunables.os, is built with
> -DMODULE_NAME=rtld and thus ABORT_TRANSACTION should be an empty statement.
elf/dl-tunables.o uses it as well, and is built with -DMODULE_NAME=libc.
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-11 23:42 ` Florian Weimer
@ 2018-01-12 0:56 ` Adhemerval Zanella
2018-01-12 16:07 ` [PATCHv2] " Tulio Magno Quites Machado Filho
0 siblings, 1 reply; 11+ messages in thread
From: Adhemerval Zanella @ 2018-01-12 0:56 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 11/01/2018 21:42, Florian Weimer wrote:
> On 01/11/2018 11:40 PM, Adhemerval Zanella wrote:
>> I am not really understanding why exactly this is failing because the only
>> object that currently uses __access_noerrno, dl-tunables.os, is built with
>> -DMODULE_NAME=rtld and thus ABORT_TRANSACTION should be an empty statement.
>
> elf/dl-tunables.o uses it as well, and is built with -DMODULE_NAME=libc.
Right, and if I recall correctly thread register is an undefined position at
the time. The only regard I have is adding some specific syscall tinkering
due a very specific arch/os requirement. Wouldn't be better to just
reimplement __access_noerrno/not-errno.h for powerpc?
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-12 0:56 ` Adhemerval Zanella
@ 2018-01-12 16:07 ` Tulio Magno Quites Machado Filho
2018-01-12 16:31 ` Florian Weimer
0 siblings, 1 reply; 11+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-01-12 16:07 UTC (permalink / raw)
To: libc-alpha; +Cc: Adhemerval Zanella, Florian Weimer, aurelien
Changes since v1:
- Re-implemented the patch to re-define a powerpc-specific
__access_noerrno.
--- 8< ---
The tunables framework needs to execute syscall early in process
initialization, before the TCB is available for consumption. This
behavior conflicts with powerpc{|64|64le}'s lock elision code, that
checks the TCB before trying to abort transactions immediately before
executing a syscall.
This patch adds a powerpc-specific implementation of __access_noerrno
that does not abort transactions before the executing syscall.
Tested on powerpc{|64|64le}.
2018-01-12 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
[BZ #22685]
* sysdeps/unix/sysv/linux/powerpc/not-errno.h: New file. Reuse
Linux code, but remove the code that aborts transactions.
Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
---
sysdeps/unix/sysv/linux/powerpc/not-errno.h | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 sysdeps/unix/sysv/linux/powerpc/not-errno.h
diff --git a/sysdeps/unix/sysv/linux/powerpc/not-errno.h b/sysdeps/unix/sysv/linux/powerpc/not-errno.h
new file mode 100644
index 0000000..642cb8a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/not-errno.h
@@ -0,0 +1,25 @@
+/* Syscall wrapper that do not set errno. Linux powerpc version.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* __access_noerrno is used during process initialization in elf/dl-tunables.c
+ before the TCB is initialized, prohibiting the usage of
+ ABORT_TRANSACTION. */
+#undef ABORT_TRANSACTION
+#define ABORT_TRANSACTION
+
+#include "sysdeps/unix/sysv/linux/not-errno.h"
--
2.9.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-12 16:07 ` [PATCHv2] " Tulio Magno Quites Machado Filho
@ 2018-01-12 16:31 ` Florian Weimer
2018-01-12 16:40 ` Tulio Magno Quites Machado Filho
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2018-01-12 16:31 UTC (permalink / raw)
To: Tulio Magno Quites Machado Filho, libc-alpha; +Cc: Adhemerval Zanella, aurelien
On 01/12/2018 05:07 PM, Tulio Magno Quites Machado Filho wrote:
> +/* __access_noerrno is used during process initialization in elf/dl-tunables.c
> + before the TCB is initialized, prohibiting the usage of
> + ABORT_TRANSACTION. */
> +#undef ABORT_TRANSACTION
> +#define ABORT_TRANSACTION
> +
> +#include "sysdeps/unix/sysv/linux/not-errno.h"
This #define isn't properly scoped, so I really don't like this approach
for a header file.
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-12 16:31 ` Florian Weimer
@ 2018-01-12 16:40 ` Tulio Magno Quites Machado Filho
2018-01-12 16:44 ` Florian Weimer
0 siblings, 1 reply; 11+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-01-12 16:40 UTC (permalink / raw)
To: Florian Weimer, libc-alpha; +Cc: Adhemerval Zanella, aurelien
Florian Weimer <fweimer@redhat.com> writes:
> On 01/12/2018 05:07 PM, Tulio Magno Quites Machado Filho wrote:
>> +/* __access_noerrno is used during process initialization in elf/dl-tunables.c
>> + before the TCB is initialized, prohibiting the usage of
>> + ABORT_TRANSACTION. */
>> +#undef ABORT_TRANSACTION
>> +#define ABORT_TRANSACTION
>> +
>> +#include "sysdeps/unix/sysv/linux/not-errno.h"
>
> This #define isn't properly scoped, so I really don't like this approach
> for a header file.
Do you think that checking for TUNABLES_INTERNAL would be enough?
--
Tulio Magno
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-12 16:40 ` Tulio Magno Quites Machado Filho
@ 2018-01-12 16:44 ` Florian Weimer
2018-01-12 20:53 ` [PATCHv3] " Tulio Magno Quites Machado Filho
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2018-01-12 16:44 UTC (permalink / raw)
To: Tulio Magno Quites Machado Filho, libc-alpha; +Cc: Adhemerval Zanella, aurelien
On 01/12/2018 05:40 PM, Tulio Magno Quites Machado Filho wrote:
> Florian Weimer <fweimer@redhat.com> writes:
>
>> On 01/12/2018 05:07 PM, Tulio Magno Quites Machado Filho wrote:
>>> +/* __access_noerrno is used during process initialization in elf/dl-tunables.c
>>> + before the TCB is initialized, prohibiting the usage of
>>> + ABORT_TRANSACTION. */
>>> +#undef ABORT_TRANSACTION
>>> +#define ABORT_TRANSACTION
>>> +
>>> +#include "sysdeps/unix/sysv/linux/not-errno.h"
>>
>> This #define isn't properly scoped, so I really don't like this approach
>> for a header file.
>
> Do you think that checking for TUNABLES_INTERNAL would be enough?
I'm not sure what you mean.
If you move the current definition of ABORT_TRANSACTION to
ABORT_TRANSACTION_IMPL, you could perhaps do
#undef ABORT_TRANSACTION
#define ABORT_TRANSACTION
#include "sysdeps/unix/sysv/linux/not-errno.h"
#undef ABORT_TRANSACTION
#define ABORT_TRANSACTION ABORT_TRANSACTION_IMPL
(with comments, obviously).
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-12 16:44 ` Florian Weimer
@ 2018-01-12 20:53 ` Tulio Magno Quites Machado Filho
2018-01-16 15:51 ` Aurelien Jarno
0 siblings, 1 reply; 11+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-01-12 20:53 UTC (permalink / raw)
To: libc-alpha; +Cc: Florian Weimer, Adhemerval Zanella, aurelien
Changes since v2:
- Limited the scope of the changes to ABORT_TRANSACTION.
Changes since v1:
- Re-implemented the patch to re-define a powerpc-specific
__access_noerrno.
--- 8< ---
The tunables framework needs to execute syscall early in process
initialization, before the TCB is available for consumption. This
behavior conflicts with powerpc{|64|64le}'s lock elision code, that
checks the TCB before trying to abort transactions immediately before
executing a syscall.
This patch adds a powerpc-specific implementation of __access_noerrno
that does not abort transactions before the executing syscall.
Tested on powerpc{|64|64le}.
2018-01-12 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
[BZ #22685]
* sysdeps/powerpc/powerpc32/sysdep.h (ABORT_TRANSACTION_IMPL): Renamed
from ABORT_TRANSACTION.
(ABORT_TRANSACTION): Redirect to ABORT_TRANSACTION_IMPL.
* sysdeps/powerpc/powerpc64/sysdep.h (ABORT_TRANSACTION,
ABORT_TRANSACTION_IMPL): Likewise.
* sysdeps/unix/sysv/linux/powerpc/not-errno.h: New file. Reuse
Linux code, but remove the code that aborts transactions.
Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
---
sysdeps/powerpc/powerpc32/sysdep.h | 5 +++--
sysdeps/powerpc/powerpc64/sysdep.h | 5 +++--
sysdeps/unix/sysv/linux/powerpc/not-errno.h | 30 +++++++++++++++++++++++++++++
3 files changed, 36 insertions(+), 4 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/powerpc/not-errno.h
diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
index 62c018f..8e32a2a 100644
--- a/sysdeps/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/powerpc/powerpc32/sysdep.h
@@ -91,7 +91,7 @@ GOT_LABEL: ; \
ASM_SIZE_DIRECTIVE(name)
#if ! IS_IN(rtld)
-# define ABORT_TRANSACTION \
+# define ABORT_TRANSACTION_IMPL \
cmpwi 2,0; \
beq 1f; \
lwz 0,TM_CAPABLE(2); \
@@ -102,8 +102,9 @@ GOT_LABEL: ; \
.align 4; \
1:
#else
-# define ABORT_TRANSACTION
+# define ABORT_TRANSACTION_IMPL
#endif
+#define ABORT_TRANSACTION ABORT_TRANSACTION_IMPL
#define DO_CALL(syscall) \
ABORT_TRANSACTION \
diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index 26b0885..2df1d9b 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -264,7 +264,7 @@ LT_LABELSUFFIX(name,_name_end): ; \
END_2(name)
#if !IS_IN(rtld)
-# define ABORT_TRANSACTION \
+# define ABORT_TRANSACTION_IMPL \
cmpdi 13,0; \
beq 1f; \
lwz 0,TM_CAPABLE(13); \
@@ -275,8 +275,9 @@ LT_LABELSUFFIX(name,_name_end): ; \
.p2align 4; \
1:
#else
-# define ABORT_TRANSACTION
+# define ABORT_TRANSACTION_IMPL
#endif
+#define ABORT_TRANSACTION ABORT_TRANSACTION_IMPL
#define DO_CALL(syscall) \
ABORT_TRANSACTION \
diff --git a/sysdeps/unix/sysv/linux/powerpc/not-errno.h b/sysdeps/unix/sysv/linux/powerpc/not-errno.h
new file mode 100644
index 0000000..27da21b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/not-errno.h
@@ -0,0 +1,30 @@
+/* Syscall wrapper that do not set errno. Linux powerpc version.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* __access_noerrno is used during process initialization in elf/dl-tunables.c
+ before the TCB is initialized, prohibiting the usage of
+ ABORT_TRANSACTION. */
+#undef ABORT_TRANSACTION
+#define ABORT_TRANSACTION
+
+#include "sysdeps/unix/sysv/linux/not-errno.h"
+
+/* Recover ABORT_TRANSACTION's previous value, in order to not affect
+ other syscalls. */
+#undef ABORT_TRANSACTION
+#define ABORT_TRANSACTION ABORT_TRANSACTION_IMPL
--
2.9.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3] powerpc: Fix syscalls during early process initialization [BZ #22685]
2018-01-12 20:53 ` [PATCHv3] " Tulio Magno Quites Machado Filho
@ 2018-01-16 15:51 ` Aurelien Jarno
0 siblings, 0 replies; 11+ messages in thread
From: Aurelien Jarno @ 2018-01-16 15:51 UTC (permalink / raw)
To: Tulio Magno Quites Machado Filho
Cc: libc-alpha, Florian Weimer, Adhemerval Zanella
On 2018-01-12 18:52, Tulio Magno Quites Machado Filho wrote:
> Changes since v2:
> - Limited the scope of the changes to ABORT_TRANSACTION.
>
> Changes since v1:
> - Re-implemented the patch to re-define a powerpc-specific
> __access_noerrno.
>
> --- 8< ---
>
> The tunables framework needs to execute syscall early in process
> initialization, before the TCB is available for consumption. This
> behavior conflicts with powerpc{|64|64le}'s lock elision code, that
> checks the TCB before trying to abort transactions immediately before
> executing a syscall.
>
> This patch adds a powerpc-specific implementation of __access_noerrno
> that does not abort transactions before the executing syscall.
>
> Tested on powerpc{|64|64le}.
>
> 2018-01-12 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
>
> [BZ #22685]
> * sysdeps/powerpc/powerpc32/sysdep.h (ABORT_TRANSACTION_IMPL): Renamed
> from ABORT_TRANSACTION.
> (ABORT_TRANSACTION): Redirect to ABORT_TRANSACTION_IMPL.
> * sysdeps/powerpc/powerpc64/sysdep.h (ABORT_TRANSACTION,
> ABORT_TRANSACTION_IMPL): Likewise.
> * sysdeps/unix/sysv/linux/powerpc/not-errno.h: New file. Reuse
> Linux code, but remove the code that aborts transactions.
Thanks for this new version. It is indeed much simpler than the initial
version. I confirm it fixes the issue and does not introduce regressions
in the testsuite.
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-01-16 15:51 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-11 19:41 [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685] Tulio Magno Quites Machado Filho
2018-01-11 21:37 ` Aurelien Jarno
2018-01-11 22:40 ` Adhemerval Zanella
2018-01-11 23:42 ` Florian Weimer
2018-01-12 0:56 ` Adhemerval Zanella
2018-01-12 16:07 ` [PATCHv2] " Tulio Magno Quites Machado Filho
2018-01-12 16:31 ` Florian Weimer
2018-01-12 16:40 ` Tulio Magno Quites Machado Filho
2018-01-12 16:44 ` Florian Weimer
2018-01-12 20:53 ` [PATCHv3] " Tulio Magno Quites Machado Filho
2018-01-16 15:51 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).