[PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]

[PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Tulio Magno Quites Machado Filho]

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

Re: [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Aurelien Jarno]

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

Re: [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Adhemerval Zanella]

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>
> 

Re: [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Florian Weimer]

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

Re: [PATCH] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Adhemerval Zanella]

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?

[PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Tulio Magno Quites Machado Filho]

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

Re: [PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Florian Weimer]

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

Re: [PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Tulio Magno Quites Machado Filho]

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

Re: [PATCHv2] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Florian Weimer]

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

[PATCHv3] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Tulio Magno Quites Machado Filho]

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

Re: [PATCHv3] powerpc: Fix syscalls during early process initialization [BZ #22685]

[Aurelien Jarno]

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