[PATCH 1/3] powerpc/uaccess: Don't use "m<>" constraint with GCC 4.9

[PATCH 1/3] powerpc/uaccess: Don't use "m<>" constraint with GCC 4.9

[Christophe Leroy]

GCC 4.9 sometimes fails to build with "m<>" constraint in
inline assembly.

  CC      lib/iov_iter.o
In file included from ./arch/powerpc/include/asm/cmpxchg.h:6:0,
                 from ./arch/powerpc/include/asm/atomic.h:11,
                 from ./include/linux/atomic.h:7,
                 from ./include/linux/crypto.h:15,
                 from ./include/crypto/hash.h:11,
                 from lib/iov_iter.c:2:
lib/iov_iter.c: In function 'iovec_from_user.part.30':
./arch/powerpc/include/asm/uaccess.h:287:2: error: 'asm' operand has impossible constraints
  __asm__ __volatile__(    \
  ^
./include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
 # define unlikely(x) __builtin_expect(!!(x), 0)
                                          ^
./arch/powerpc/include/asm/uaccess.h:583:34: note: in expansion of macro 'unsafe_op_wrap'
 #define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
                                  ^
./arch/powerpc/include/asm/uaccess.h:329:10: note: in expansion of macro '__get_user_asm'
  case 4: __get_user_asm(x, (u32 __user *)ptr, retval, "lwz"); break; \
          ^
./arch/powerpc/include/asm/uaccess.h:363:3: note: in expansion of macro '__get_user_size_allowed'
   __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
   ^
./arch/powerpc/include/asm/uaccess.h:100:2: note: in expansion of macro '__get_user_nocheck'
  __get_user_nocheck((x), (ptr), sizeof(*(ptr)), false)
  ^
./arch/powerpc/include/asm/uaccess.h:583:49: note: in expansion of macro '__get_user_allowed'
 #define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
                                                 ^
lib/iov_iter.c:1663:3: note: in expansion of macro 'unsafe_get_user'
   unsafe_get_user(len, &uiov[i].iov_len, uaccess_end);
   ^
make[1]: *** [scripts/Makefile.build:283: lib/iov_iter.o] Error 1

Define a UPD_CONSTR macro that is "<>" by default and
only "" with GCC prior to GCC 5.

Fixes: fcf1f26895a4 ("powerpc/uaccess: Add pre-update addressing to __put_user_asm_goto()")
Fixes: 2f279eeb68b8 ("powerpc/uaccess: Add pre-update addressing to __get_user_asm() and __put_user_asm()")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/ppc_asm.h | 14 ++++++++++++++
 arch/powerpc/include/asm/uaccess.h |  4 ++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 511786f0e40d..471c7c57fc98 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -803,6 +803,20 @@ END_FTR_SECTION_NESTED(CPU_FTR_CELL_TB_BUG, CPU_FTR_CELL_TB_BUG, 96)
 
 #endif /* !CONFIG_PPC_BOOK3E */
 
+#else /* __ASSEMBLY */
+
+/*
+ * Inline assembly memory constraint
+ *
+ * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
+ *
+ */
+#if defined(GCC_VERSION) && GCC_VERSION < 50000
+#define UPD_CONSTR ""
+#else
+#define UPD_CONSTR "<>"
+#endif
+
 #endif /*  __ASSEMBLY__ */
 
 /*
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 604d705f1bb8..8f27ea48fadb 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -223,7 +223,7 @@ do {								\
 		"1:	" op "%U1%X1 %0,%1	# put_user\n"	\
 		EX_TABLE(1b, %l2)				\
 		:						\
-		: "r" (x), "m<>" (*addr)				\
+		: "r" (x), "m"UPD_CONSTR (*addr)		\
 		:						\
 		: label)
 
@@ -294,7 +294,7 @@ extern long __get_user_bad(void);
 		".previous\n"				\
 		EX_TABLE(1b, 3b)			\
 		: "=r" (err), "=r" (x)			\
-		: "m<>" (*addr), "i" (-EFAULT), "0" (err))
+		: "m"UPD_CONSTR (*addr), "i" (-EFAULT), "0" (err))
 
 #ifdef __powerpc64__
 #define __get_user_asm2(x, addr, err)			\
-- 
2.25.0

[PATCH 2/3] powerpc: Fix incorrect stw{, ux, u, x} instructions in __set_pte_at

[Christophe Leroy]

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

The placeholder for instruction selection should use the second
argument's operand, which is %1, not %0. This could generate incorrect
assembly code if the instruction selection for argument %0 ever differs
from argument %1.

Fixes: 9bf2b5cdc5fe ("powerpc: Fixes for CONFIG_PTE_64BIT for SMP support")
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: <stable@vger.kernel.org> # v2.6.28+
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +-
 arch/powerpc/include/asm/nohash/pgtable.h    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 36443cda8dcf..34f5ca391f0c 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -524,7 +524,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
 	__asm__ __volatile__("\
 		stw%U0%X0 %2,%0\n\
 		eieio\n\
-		stw%U0%X0 %L2,%1"
+		stw%U1%X1 %L2,%1"
 	: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
 	: "r" (pte) : "memory");
 
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
index 4b7c3472eab1..a00e4c1746d6 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -199,7 +199,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
 		__asm__ __volatile__("\
 			stw%U0%X0 %2,%0\n\
 			eieio\n\
-			stw%U0%X0 %L2,%1"
+			stw%U1%X1 %L2,%1"
 		: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
 		: "r" (pte) : "memory");
 		return;
-- 
2.25.0

[PATCH 3/3] powerpc: Fix pre-update addressing in inline assembly

[Christophe Leroy]

In several places, inline assembly uses the "%Un" modifier
to enable the use of instruction with pre-update addressing,
but the associated "<>" constraint is missing.

As mentioned in previous patch, this fails with gcc 4.9, so
"<>" can't be used directly.

Use UPD_CONSTR macro everywhere %Un modifier is used.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/atomic.h            | 9 +++++----
 arch/powerpc/include/asm/book3s/32/pgtable.h | 2 +-
 arch/powerpc/include/asm/io.h                | 4 ++--
 arch/powerpc/include/asm/nohash/pgtable.h    | 2 +-
 arch/powerpc/kvm/powerpc.c                   | 4 ++--
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index 8a55eb8cc97b..b82f9154e45a 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -10,6 +10,7 @@
 #include <linux/types.h>
 #include <asm/cmpxchg.h>
 #include <asm/barrier.h>
+#include <asm/ppc_asm.h>
 
 /*
  * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
@@ -26,14 +27,14 @@ static __inline__ int atomic_read(const atomic_t *v)
 {
 	int t;
 
-	__asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
+	__asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"UPD_CONSTR(v->counter));
 
 	return t;
 }
 
 static __inline__ void atomic_set(atomic_t *v, int i)
 {
-	__asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i));
+	__asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) : "r"(i));
 }
 
 #define ATOMIC_OP(op, asm_op)						\
@@ -316,14 +317,14 @@ static __inline__ s64 atomic64_read(const atomic64_t *v)
 {
 	s64 t;
 
-	__asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
+	__asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) : "m"UPD_CONSTR(v->counter));
 
 	return t;
 }
 
 static __inline__ void atomic64_set(atomic64_t *v, s64 i)
 {
-	__asm__ __volatile__("std%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i));
+	__asm__ __volatile__("std%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) : "r"(i));
 }
 
 #define ATOMIC64_OP(op, asm_op)						\
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 34f5ca391f0c..0e1b6e020cef 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -525,7 +525,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
 		stw%U0%X0 %2,%0\n\
 		eieio\n\
 		stw%U1%X1 %L2,%1"
-	: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
+	: "=m"UPD_CONSTR (*ptep), "=m"UPD_CONSTR (*((unsigned char *)ptep+4))
 	: "r" (pte) : "memory");
 
 #else
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 58635960403c..87964dfb838e 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -122,7 +122,7 @@ static inline u##size name(const volatile u##size __iomem *addr)	\
 {									\
 	u##size ret;							\
 	__asm__ __volatile__("sync;"#insn"%U1%X1 %0,%1;twi 0,%0,0;isync"\
-		: "=r" (ret) : "m" (*addr) : "memory");			\
+		: "=r" (ret) : "m"UPD_CONSTR (*addr) : "memory");	\
 	return ret;							\
 }
 
@@ -130,7 +130,7 @@ static inline u##size name(const volatile u##size __iomem *addr)	\
 static inline void name(volatile u##size __iomem *addr, u##size val)	\
 {									\
 	__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0"			\
-		: "=m" (*addr) : "r" (val) : "memory");			\
+		: "=m"UPD_CONSTR (*addr) : "r" (val) : "memory");	\
 	mmiowb_set_pending();						\
 }
 
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
index a00e4c1746d6..55ef2112ed00 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -200,7 +200,7 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
 			stw%U0%X0 %2,%0\n\
 			eieio\n\
 			stw%U1%X1 %L2,%1"
-		: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
+		: "=m"UPD_CONSTR (*ptep), "=m"UPD_CONSTR (*((unsigned char *)ptep+4))
 		: "r" (pte) : "memory");
 		return;
 	}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 13999123b735..cf52d26f49cd 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1087,7 +1087,7 @@ static inline u64 sp_to_dp(u32 fprs)
 
 	preempt_disable();
 	enable_kernel_fp();
-	asm ("lfs%U1%X1 0,%1; stfd%U0%X0 0,%0" : "=m" (fprd) : "m" (fprs)
+	asm ("lfs%U1%X1 0,%1; stfd%U0%X0 0,%0" : "=m"UPD_CONSTR (fprd) : "m"UPD_CONSTR (fprs)
 	     : "fr0");
 	preempt_enable();
 	return fprd;
@@ -1099,7 +1099,7 @@ static inline u32 dp_to_sp(u64 fprd)
 
 	preempt_disable();
 	enable_kernel_fp();
-	asm ("lfd%U1%X1 0,%1; stfs%U0%X0 0,%0" : "=m" (fprs) : "m" (fprd)
+	asm ("lfd%U1%X1 0,%1; stfs%U0%X0 0,%0" : "=m"UPD_CONSTR (fprs) : "m"UPD_CONSTR (fprd)
 	     : "fr0");
 	preempt_enable();
 	return fprs;
-- 
2.25.0

Re: [PATCH 3/3] powerpc: Fix pre-update addressing in inline assembly

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5813 bytes --]

Hi Christophe,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on linus/master next-20201016]
[cannot apply to kvm-ppc/kvm-ppc-next mpe/next v5.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-uaccess-Don-t-use-m-constraint-with-GCC-4-9/20201019-201504
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-r012-20201019 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 094e9f4779eb9b5c6a49014f2f80b8cbb833572f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/d57fd8d270993414b8c0414d7be4b03cc3de1856
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christophe-Leroy/powerpc-uaccess-Don-t-use-m-constraint-with-GCC-4-9/20201019-201504
        git checkout d57fd8d270993414b8c0414d7be4b03cc3de1856
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/kernel/asm-offsets.c:14:
   In file included from include/linux/compat.h:14:
   In file included from include/linux/sem.h:5:
   In file included from include/uapi/linux/sem.h:5:
   In file included from include/linux/ipc.h:5:
   In file included from include/linux/spinlock.h:51:
   In file included from include/linux/preempt.h:78:
   In file included from ./arch/powerpc/include/generated/asm/preempt.h:1:
   In file included from include/asm-generic/preempt.h:5:
   In file included from include/linux/thread_info.h:21:
   In file included from arch/powerpc/include/asm/current.h:13:
   In file included from arch/powerpc/include/asm/paca.h:31:
   In file included from arch/powerpc/include/asm/atomic.h:13:
   In file included from arch/powerpc/include/asm/ppc_asm.h:9:
   In file included from arch/powerpc/include/asm/processor.h:40:
>> arch/powerpc/include/asm/ptrace.h:288:20: error: use of undeclared identifier 'THREAD_SIZE'
           return ((addr & ~(THREAD_SIZE - 1))  ==
                             ^
   arch/powerpc/include/asm/ptrace.h:289:35: error: use of undeclared identifier 'THREAD_SIZE'
                   (kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1)));
                                                   ^
   In file included from arch/powerpc/kernel/asm-offsets.c:21:
   include/linux/mman.h:137:9: warning: division by zero is undefined [-Wdivision-by-zero]
                  _calc_vm_trans(flags, MAP_LOCKED,     VM_LOCKED    ) |
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/mman.h:115:21: note: expanded from macro '_calc_vm_trans'
      : ((x) & (bit1)) / ((bit1) / (bit2))))
                       ^ ~~~~~~~~~~~~~~~~~
   include/linux/mman.h:138:9: warning: division by zero is undefined [-Wdivision-by-zero]
                  _calc_vm_trans(flags, MAP_SYNC,       VM_SYNC      );
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/mman.h:115:21: note: expanded from macro '_calc_vm_trans'
      : ((x) & (bit1)) / ((bit1) / (bit2))))
                       ^ ~~~~~~~~~~~~~~~~~
   2 warnings and 2 errors generated.
   make[2]: *** [scripts/Makefile.build:117: arch/powerpc/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1202: prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

vim +/THREAD_SIZE +288 arch/powerpc/include/asm/ptrace.h

359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  275  
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  276  /**
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  277   * regs_within_kernel_stack() - check the address in the stack
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  278   * @regs:      pt_regs which contains kernel stack pointer.
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  279   * @addr:      address which is checked.
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  280   *
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  281   * regs_within_kernel_stack() checks @addr is within the kernel stack page(s).
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  282   * If @addr is within the kernel stack, it returns true. If not, returns false.
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  283   */
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  284  
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  285  static inline bool regs_within_kernel_stack(struct pt_regs *regs,
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  286  						unsigned long addr)
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  287  {
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07 @288  	return ((addr & ~(THREAD_SIZE - 1))  ==
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  289  		(kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1)));
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  290  }
359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  291  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26981 bytes --]

Re: [PATCH 3/3] powerpc: Fix pre-update addressing in inline assembly

[Christophe Leroy]

Le 19/10/2020 à 17:35, kernel test robot a écrit :
> Hi Christophe,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on powerpc/next]
> [also build test ERROR on linus/master next-20201016]
> [cannot apply to kvm-ppc/kvm-ppc-next mpe/next v5.9]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-uaccess-Don-t-use-m-constraint-with-GCC-4-9/20201019-201504
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: powerpc64-randconfig-r012-20201019 (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 094e9f4779eb9b5c6a49014f2f80b8cbb833572f)
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # install powerpc64 cross compiling tool for clang build
>          # apt-get install binutils-powerpc64-linux-gnu
>          # https://github.com/0day-ci/linux/commit/d57fd8d270993414b8c0414d7be4b03cc3de1856
>          git remote add linux-review https://github.com/0day-ci/linux
>          git fetch --no-tags linux-review Christophe-Leroy/powerpc-uaccess-Don-t-use-m-constraint-with-GCC-4-9/20201019-201504
>          git checkout d57fd8d270993414b8c0414d7be4b03cc3de1856
>          # save the attached .config to linux build tree
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>     In file included from arch/powerpc/kernel/asm-offsets.c:14:
>     In file included from include/linux/compat.h:14:
>     In file included from include/linux/sem.h:5:
>     In file included from include/uapi/linux/sem.h:5:
>     In file included from include/linux/ipc.h:5:
>     In file included from include/linux/spinlock.h:51:
>     In file included from include/linux/preempt.h:78:
>     In file included from ./arch/powerpc/include/generated/asm/preempt.h:1:
>     In file included from include/asm-generic/preempt.h:5:
>     In file included from include/linux/thread_info.h:21:
>     In file included from arch/powerpc/include/asm/current.h:13:
>     In file included from arch/powerpc/include/asm/paca.h:31:
>     In file included from arch/powerpc/include/asm/atomic.h:13:
>     In file included from arch/powerpc/include/asm/ppc_asm.h:9:
>     In file included from arch/powerpc/include/asm/processor.h:40:
>>> arch/powerpc/include/asm/ptrace.h:288:20: error: use of undeclared identifier 'THREAD_SIZE'
>             return ((addr & ~(THREAD_SIZE - 1))  ==
>                               ^
>     arch/powerpc/include/asm/ptrace.h:289:35: error: use of undeclared identifier 'THREAD_SIZE'
>                     (kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1)));

Most likely a circular inclusion problem.

I'll have to put it in a header that doesn't include pile of other stuff. The least bad candidate 
seems to be asm-const.h

Christophe

>                                                     ^
>     In file included from arch/powerpc/kernel/asm-offsets.c:21:
>     include/linux/mman.h:137:9: warning: division by zero is undefined [-Wdivision-by-zero]
>                    _calc_vm_trans(flags, MAP_LOCKED,     VM_LOCKED    ) |
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     include/linux/mman.h:115:21: note: expanded from macro '_calc_vm_trans'
>        : ((x) & (bit1)) / ((bit1) / (bit2))))
>                         ^ ~~~~~~~~~~~~~~~~~
>     include/linux/mman.h:138:9: warning: division by zero is undefined [-Wdivision-by-zero]
>                    _calc_vm_trans(flags, MAP_SYNC,       VM_SYNC      );
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     include/linux/mman.h:115:21: note: expanded from macro '_calc_vm_trans'
>        : ((x) & (bit1)) / ((bit1) / (bit2))))
>                         ^ ~~~~~~~~~~~~~~~~~
>     2 warnings and 2 errors generated.
>     make[2]: *** [scripts/Makefile.build:117: arch/powerpc/kernel/asm-offsets.s] Error 1
>     make[2]: Target '__build' not remade because of errors.
>     make[1]: *** [Makefile:1202: prepare0] Error 2
>     make[1]: Target 'prepare' not remade because of errors.
>     make: *** [Makefile:185: __sub-make] Error 2
>     make: Target 'prepare' not remade because of errors.
> 
> vim +/THREAD_SIZE +288 arch/powerpc/include/asm/ptrace.h
> 
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  275
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  276  /**
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  277   * regs_within_kernel_stack() - check the address in the stack
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  278   * @regs:      pt_regs which contains kernel stack pointer.
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  279   * @addr:      address which is checked.
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  280   *
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  281   * regs_within_kernel_stack() checks @addr is within the kernel stack page(s).
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  282   * If @addr is within the kernel stack, it returns true. If not, returns false.
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  283   */
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  284
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  285  static inline bool regs_within_kernel_stack(struct pt_regs *regs,
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  286  						unsigned long addr)
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  287  {
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07 @288  	return ((addr & ~(THREAD_SIZE - 1))  ==
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  289  		(kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1)));
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  290  }
> 359e4284a3f37ab Mahesh Salgaonkar 2010-04-07  291
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 

Re: [PATCH 1/3] powerpc/uaccess: Don't use "m<>" constraint with GCC 4.9

[Segher Boessenkool]

On Mon, Oct 19, 2020 at 12:12:46PM +0000, Christophe Leroy wrote:
> GCC 4.9 sometimes fails to build with "m<>" constraint in
> inline assembly.

> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -223,7 +223,7 @@ do {								\
>  		"1:	" op "%U1%X1 %0,%1	# put_user\n"	\
>  		EX_TABLE(1b, %l2)				\
>  		:						\
> -		: "r" (x), "m<>" (*addr)				\
> +		: "r" (x), "m"UPD_CONSTR (*addr)		\
>  		:						\
>  		: label)
>  
> @@ -294,7 +294,7 @@ extern long __get_user_bad(void);
>  		".previous\n"				\
>  		EX_TABLE(1b, 3b)			\
>  		: "=r" (err), "=r" (x)			\
> -		: "m<>" (*addr), "i" (-EFAULT), "0" (err))
> +		: "m"UPD_CONSTR (*addr), "i" (-EFAULT), "0" (err))

Wow, ugly!  But these are the only two places that use this, so

Acked-by: Segher Boessenkool  <segher@kernel.crashing.org>

I just hope that we get rid of 4.9 before we would use this a lot more ;-)


Segher

Re: [PATCH 2/3] powerpc: Fix incorrect stw{, ux, u, x} instructions in __set_pte_at

[Segher Boessenkool]

On Mon, Oct 19, 2020 at 12:12:47PM +0000, Christophe Leroy wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> 
> The placeholder for instruction selection should use the second
> argument's operand, which is %1, not %0. This could generate incorrect
> assembly code if the instruction selection for argument %0 ever differs
> from argument %1.

"Instruction selection" isn't correct here...  "if the memory addressing
of operand 0 is a different form from that of operand 1", perhaps?

The patch looks fine of course :-)

Acked-by: Segher Boessenkool <segher@kernel.crashing.org>


Segher

Re: [PATCH 3/3] powerpc: Fix pre-update addressing in inline assembly

[Segher Boessenkool]

On Mon, Oct 19, 2020 at 12:12:48PM +0000, Christophe Leroy wrote:
> In several places, inline assembly uses the "%Un" modifier
> to enable the use of instruction with pre-update addressing,

Calling this "pre-update" is misleading: the register is not updated
before the address is generated (or the memory access done!), and the
addressing is exactly the same as the "non-u" insn would use.  It is
called an "update form" instruction, because (at the same time as doing
the memory access, logically anyway) it writes back the address used to
the base register.

> but the associated "<>" constraint is missing.

But that is just fine.  Pointless, sure, but not a bug.

> Use UPD_CONSTR macro everywhere %Un modifier is used.

Eww.  My poor stomach.

Have you verified that update form is *correct* in all these, and that
we even *want* this there?


Segher

Re: [PATCH 3/3] powerpc: Fix pre-update addressing in inline assembly

[Christophe Leroy]

Le 19/10/2020 à 22:24, Segher Boessenkool a écrit :
> On Mon, Oct 19, 2020 at 12:12:48PM +0000, Christophe Leroy wrote:
>> In several places, inline assembly uses the "%Un" modifier
>> to enable the use of instruction with pre-update addressing,
> 
> Calling this "pre-update" is misleading: the register is not updated
> before the address is generated (or the memory access done!), and the
> addressing is exactly the same as the "non-u" insn would use.  It is
> called an "update form" instruction, because (at the same time as doing
> the memory access, logically anyway) it writes back the address used to
> the base register.
> 
>> but the associated "<>" constraint is missing.
> 
> But that is just fine.  Pointless, sure, but not a bug.

Most of those are from prehistoric code. So at some point in time it was effective. Then one day GCC 
changed it's way and they became pointless. So, not a software bug, but still a regression at some 
point.

> 
>> Use UPD_CONSTR macro everywhere %Un modifier is used.
> 
> Eww.  My poor stomach.

There are not that many :)

> 
> Have you verified that update form is *correct* in all these, and that
> we even *want* this there?

I can't see anything that would militate against it, do you ?

I guess if the elders have put %Us there, it was wanted.

Christophe

Re: [PATCH 3/3] powerpc: Fix pre-update addressing in inline assembly

[Segher Boessenkool]

On Tue, Oct 20, 2020 at 09:44:33AM +0200, Christophe Leroy wrote:
> Le 19/10/2020 à 22:24, Segher Boessenkool a écrit :
> >>but the associated "<>" constraint is missing.
> >
> >But that is just fine.  Pointless, sure, but not a bug.
> 
> Most of those are from prehistoric code. So at some point in time it was 
> effective. Then one day GCC changed it's way and they became pointless. So, 
> not a software bug, but still a regression at some point.
> 
> >>Use UPD_CONSTR macro everywhere %Un modifier is used.
> >
> >Eww.  My poor stomach.
> 
> There are not that many :)

Heh, your pain threshold is much higher than mine I guess :-)

> >Have you verified that update form is *correct* in all these, and that
> >we even *want* this there?
> 
> I can't see anything that would militate against it, do you ?
> 
> I guess if the elders have put %Us there, it was wanted.

On old CPUs, update form load/stores actually executed faster than a
"normal" memory access and an addi (or plain add).  But on more recent
stuff it mostly saves code size.  Which is nice of course, and can speed
up your code a bit, in theory at least.

It is quite hard to trigger the compiler to generate update form insns
in asm, sigh.  So testing will probably not show anything either way.
Oh well :-)


Segher