| 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
| ||
|
Message-ID: <51E2FAB9.9050900@goop.org>
Date: Sun, 14 Jul 2013 12:23:37 -0700
From: Jeremy Fitzhardinge <jeremy@...p.org>
To: Ramkumar Ramachandra <artagnon@...il.com>
CC: LKML <linux-kernel@...r.kernel.org>,
Andi Kleen <ak@...ux.intel.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Ingo Molnar <mingo@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Eli Friedman <eli.friedman@...il.com>,
Jim Grosbach <grosbach@...le.com>,
Stephen Checkoway <s@...tak.org>, LLVMdev <llvmdev@...uiuc.edu>
Subject: Re: [PATCH] x86/asm: avoid mnemonics without type suffix
(resent without HTML)
On 07/14/2013 05:56 AM, Ramkumar Ramachandra wrote:
> 1c54d77 (x86: partial unification of asm-x86/bitops.h, 2008-01-30)
> changed a bunch of btrl/btsl instructions to btr/bts, with the following
> justification:
>
> The inline assembly for the bit operations has been changed to remove
> explicit sizing hints on the instructions, so the assembler will pick
> the appropriate instruction forms depending on the architecture and
> the context.
>
> Unfortunately, GNU as does no such thing, and the AT&T syntax manual
> [1] contains no references to any such inference. As evidenced by the
> following experiment, gas always disambiguates btr/bts to btrl/btsl.
> Feed the following input to gas:
>
> btrl $1, 0
> btr $1, 0
> btsl $1, 0
> bts $1, 0
When I originally did those patches, I was careful make sure that we
didn't give implied sizes to operations with only immediate and/or
memory operands because - in general - gas can't infer the operation
size from such operands. However, in the case of the bit test/set
operations, the memory access size is not really derived from the
operation size (the SDM is a bit vague), and even if it were it would be
an operation rather than semantic difference. So there's no real
problem with gas choosing 'l' as a default size in the absence of any
explicit override or constraint.
> Check that btr matches btrl, and bts matches btsl in both cases:
>
> $ as --32 -a in.s
> $ as --64 -a in.s
>
> To avoid giving readers the illusion of such an inference, and for
> clarity, change btr/bts back to btrl/btsl. Also, llvm-mc refuses to
> disambiguate btr/bts automatically.
That sounds reasonable for all other operations because it makes a real
semantic difference, but overly strict for bit operations.
J
> [1]: http://docs.oracle.com/cd/E19253-01/817-5477/817-5477.pdf
>
> Cc: Jeremy Fitzhardinge <jeremy@...p.org>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Eli Friedman <eli.friedman@...il.com>
> Cc: Jim Grosbach <grosbach@...le.com>
> Cc: Stephen Checkoway <s@...tak.org>
> Cc: LLVMdev <llvmdev@...uiuc.edu>
> Signed-off-by: Ramkumar Ramachandra <artagnon@...il.com>
> ---
> We discussed this pretty extensively on LLVMDev, but I'm still not
> sure that I haven't missed something.
>
> arch/x86/include/asm/bitops.h | 16 ++++++++--------
> arch/x86/include/asm/percpu.h | 2 +-
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
> index 6dfd019..6ed3d1e 100644
> --- a/arch/x86/include/asm/bitops.h
> +++ b/arch/x86/include/asm/bitops.h
> @@ -67,7 +67,7 @@ set_bit(unsigned int nr, volatile unsigned long *addr)
> : "iq" ((u8)CONST_MASK(nr))
> : "memory");
> } else {
> - asm volatile(LOCK_PREFIX "bts %1,%0"
> + asm volatile(LOCK_PREFIX "btsl %1,%0"
> : BITOP_ADDR(addr) : "Ir" (nr) : "memory");
> }
> }
> @@ -83,7 +83,7 @@ set_bit(unsigned int nr, volatile unsigned long *addr)
> */
> static inline void __set_bit(int nr, volatile unsigned long *addr)
> {
> - asm volatile("bts %1,%0" : ADDR : "Ir" (nr) : "memory");
> + asm volatile("btsl %1,%0" : ADDR : "Ir" (nr) : "memory");
> }
>
> /**
> @@ -104,7 +104,7 @@ clear_bit(int nr, volatile unsigned long *addr)
> : CONST_MASK_ADDR(nr, addr)
> : "iq" ((u8)~CONST_MASK(nr)));
> } else {
> - asm volatile(LOCK_PREFIX "btr %1,%0"
> + asm volatile(LOCK_PREFIX "btrl %1,%0"
> : BITOP_ADDR(addr)
> : "Ir" (nr));
> }
> @@ -126,7 +126,7 @@ static inline void clear_bit_unlock(unsigned nr, volatile unsigned long *addr)
>
> static inline void __clear_bit(int nr, volatile unsigned long *addr)
> {
> - asm volatile("btr %1,%0" : ADDR : "Ir" (nr));
> + asm volatile("btrl %1,%0" : ADDR : "Ir" (nr));
> }
>
> /*
> @@ -198,7 +198,7 @@ static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
> {
> int oldbit;
>
> - asm volatile(LOCK_PREFIX "bts %2,%1\n\t"
> + asm volatile(LOCK_PREFIX "btsl %2,%1\n\t"
> "sbb %0,%0" : "=r" (oldbit), ADDR : "Ir" (nr) : "memory");
>
> return oldbit;
> @@ -230,7 +230,7 @@ static inline int __test_and_set_bit(int nr, volatile unsigned long *addr)
> {
> int oldbit;
>
> - asm("bts %2,%1\n\t"
> + asm("btsl %2,%1\n\t"
> "sbb %0,%0"
> : "=r" (oldbit), ADDR
> : "Ir" (nr));
> @@ -249,7 +249,7 @@ static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
> {
> int oldbit;
>
> - asm volatile(LOCK_PREFIX "btr %2,%1\n\t"
> + asm volatile(LOCK_PREFIX "btrl %2,%1\n\t"
> "sbb %0,%0"
> : "=r" (oldbit), ADDR : "Ir" (nr) : "memory");
>
> @@ -276,7 +276,7 @@ static inline int __test_and_clear_bit(int nr, volatile unsigned long *addr)
> {
> int oldbit;
>
> - asm volatile("btr %2,%1\n\t"
> + asm volatile("btrl %2,%1\n\t"
> "sbb %0,%0"
> : "=r" (oldbit), ADDR
> : "Ir" (nr));
> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> index 0da5200..fda54c9 100644
> --- a/arch/x86/include/asm/percpu.h
> +++ b/arch/x86/include/asm/percpu.h
> @@ -490,7 +490,7 @@ do { \
> #define x86_test_and_clear_bit_percpu(bit, var) \
> ({ \
> int old__; \
> - asm volatile("btr %2,"__percpu_arg(1)"\n\tsbbl %0,%0" \
> + asm volatile("btrl %2,"__percpu_arg(1)"\n\tsbbl %0,%0" \
> : "=r" (old__), "+m" (var) \
> : "dIr" (bit)); \
> old__; \
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists