linux-kernel - [PATCH] x86/locking/rwsem: Cleanup ____down

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 27 Apr 2016 14:02:17 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Michal Hocko <mhocko@...nel.org>,
	Ingo Molnar <mingo@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"David S. Miller" <davem@...emloft.net>,
	Tony Luck <tony.luck@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Chris Zankel <chris@...kel.net>,
	Max Filippov <jcmvbkbc@...il.com>, x86@...nel.org,
	linux-alpha@...r.kernel.org, linux-ia64@...r.kernel.org,
	linux-s390@...r.kernel.org, linux-sh@...r.kernel.org,
	sparclinux@...r.kernel.org, linux-xtensa@...ux-xtensa.org,
	linux-arch@...r.kernel.org, Josh Poimboeuf <jpoimboe@...hat.com>
Subject: [PATCH] x86/locking/rwsem: Cleanup ____down_write()

On Wed, Apr 20, 2016 at 03:29:30PM -0700, H. Peter Anvin wrote:
> Since it is a fixed register we could just mark edx clobbered, but
> with more flexible register constraints it can permit gcc to allocate
> a temp resister for us.

How about the following?

It boots fine in kvm and the asm changes are only trivial gcc comments
differences:

---
From: Borislav Petkov <bp@...e.de>
Date: Wed, 27 Apr 2016 13:47:32 +0200
Subject: [PATCH] x86/locking/rwsem: Cleanup ____down_write()

Move the RWSEM_ACTIVE_WRITE_BIAS out of the inline asm to reduce the
number of arguments. Also, make it an input argument only (why it was an
output operand, I still don't know...).

For better readability, use symbolic names for the arguments and move
the linebreak backspace to 80 cols.

Resulting asm differs only in the temporary gcc variable names and
locations:

  --- before      2016-04-27 13:39:05.320778458 +0200
  +++ after       2016-04-27 13:52:37.336778994 +0200
  @@ -11,8 +11,8 @@ down_write_killable:
   .LBB84:
   .LBB85:
   .LBB86:
  -        .loc 2 128 0
  -        movabsq $-4294967295, %rdx      #, tmp
  +        .loc 2 130 0
  +        movabsq $-4294967295, %rdx      #, tmp94
           movq    %rdi, %rax      # sem, sem
   .LBE86:
   .LBE85:
  @@ -23,17 +23,17 @@ down_write_killable:
   .LBB89:
   .LBB88:
   .LBB87:
  -        .loc 2 128 0
  +        .loc 2 130 0
   #APP
  -# 128 "./arch/x86/include/asm/rwsem.h" 1
  +# 130 "./arch/x86/include/asm/rwsem.h" 1
           # beginning down_write
           .pushsection .smp_locks,"a"
   .balign 4
   .long 671f - .
   .popsection
   671:
  -        lock;   xadd      %rdx,(%rax)   # tmp, sem
  -          test  %edx , %edx     # tmp
  +        lock;   xadd      %rdx,(%rax)   # tmp94, sem
  +          test  %edx , %edx     # tmp94
             jz        1f
     call call_rwsem_down_write_failed_killable
   1:

Signed-off-by: Borislav Petkov <bp@...e.de>
---
 arch/x86/include/asm/rwsem.h | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/rwsem.h b/arch/x86/include/asm/rwsem.h
index 453744c1d347..d2f8d10a6d97 100644
--- a/arch/x86/include/asm/rwsem.h
+++ b/arch/x86/include/asm/rwsem.h
@@ -99,23 +99,25 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 /*
  * lock for writing
  */
-#define ____down_write(sem, slow_path)			\
-({							\
-	long tmp;					\
-	struct rw_semaphore* ret;			\
-	asm volatile("# beginning down_write\n\t"	\
-		     LOCK_PREFIX "  xadd      %1,(%3)\n\t"	\
-		     /* adds 0xffff0001, returns the old value */ \
-		     "  test " __ASM_SEL(%w1,%k1) "," __ASM_SEL(%w1,%k1) "\n\t" \
-		     /* was the active mask 0 before? */\
-		     "  jz        1f\n"			\
-		     "  call " slow_path "\n"		\
-		     "1:\n"				\
-		     "# ending down_write"		\
-		     : "+m" (sem->count), "=d" (tmp), "=a" (ret)	\
-		     : "a" (sem), "1" (RWSEM_ACTIVE_WRITE_BIAS) \
-		     : "memory", "cc");			\
-	ret;						\
+#define ____down_write(sem, slow_path)						\
+({										\
+	long tmp = RWSEM_ACTIVE_WRITE_BIAS;					\
+	struct rw_semaphore* ret;						\
+										\
+	asm volatile("# beginning down_write\n\t"				\
+		     LOCK_PREFIX "  xadd      %[tmp],(%[sem])\n\t"		\
+		     /* adds 0xffff0001, returns the old value */		\
+		     "  test " __ASM_SEL(%w[tmp],%k[tmp]) ","			\
+			       __ASM_SEL(%w[tmp],%k[tmp]) "\n\t"		\
+		     /* was the active mask 0 before? */			\
+		     "  jz        1f\n"						\
+		     "  call " slow_path "\n"					\
+		     "1:\n"							\
+		     "# ending down_write"					\
+		     : "+m" (sem->count), "=a" (ret)				\
+		     : [sem] "a" (sem), [tmp] "r" (tmp)				\
+		     : "memory", "cc");						\
+	ret;									\
 })
 
 static inline void __down_write(struct rw_semaphore *sem)
-- 
2.7.3


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.