linux-kernel - Re: [RFC] x86: bitops asm constraint fixes

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]

Message-Id: <47DE44F6.76E4.0078.0@novell.com>
Date:	Mon, 17 Mar 2008 09:16:22 +0000
From:	"Jan Beulich" <jbeulich@...ell.com>
To:	"Chuck Ebbert" <cebbert@...hat.com>
Cc:	<mingo@...e.hu>, <tglx@...utronix.de>,
	<linux-kernel@...r.kernel.org>, <hpa@...or.com>
Subject: Re: [RFC] x86: bitops asm constraint fixes

>> --- linux-2.6.25-rc5/include/asm-x86/bitops.h	2008-03-10 13:24:33.000000000 +0100
>> +++ 2.6.25-rc5-x86-clear-bit/include/asm-x86/bitops.h	2008-03-13 08:45:40.000000000 +0100
>> @@ -24,9 +24,12 @@
>>  /* Technically wrong, but this avoids compilation errors on some gcc
>>     versions. */
>>  #define ADDR "=m" (*(volatile long *) addr)
>> +#define BIT_ADDR "=m" (((volatile int *) addr)[nr >> 5])
>>  #else
>>  #define ADDR "+m" (*(volatile long *) addr)
>> +#define BIT_ADDR "+m" (((volatile int *) addr)[nr >> 5])
>>  #endif
>> +#define BASE_ADDR "m" (*(volatile int *) addr)
>
>Can't you just do everything with unsigned longs, like this?

That's not very desirable: For one part, because there are uses of
bitops on arrays of ints (and casting these up isn't fully correct on
x86-64 because of the same reason that using the bitops on char
arrays isn't correct (see the other response I sent to Jeremy's reply),
but also because operating on longs requires REX prefixes n x86-64,
hence making the code bigger for no good reason.

>>In include/asm-x86/types.h:
>>
>> ifdef CONFIG_X86_32
>> # define BITS_PER_LONG 32
>>+# define BITMAP_ORDER 5
>> #else
>> # define BITS_PER_LONG 64
>>+# define BITMAP_ORDER 6
>> #endif
>
>Then:
>
>>  #define ADDR "=m" (*(volatile long *) addr)
>> +#define BIT_ADDR "=m" (((volatile long *) addr)[nr >> BITMAP_ORDER])
>>  #else
>>  #define ADDR "+m" (*(volatile long *) addr)
>> +#define BIT_ADDR "+m" (((volatile long *) addr)[nr >> BITMAP_ORDER])
>>  #endif
>
>No need for BASE_ADDR that way (or ADDR could be renamed to that.)

Not really, since BASE_ADDR is an input, whereas ADDR is an output.
However, ultimately all uses of ADDR should go (since even if any of
the functions needs the memory clobber to stay, using an input for
specifying the array base address is sufficient - such operations simply
don't need an exact "m" output operand then).

Jan

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