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 for Android: free password hash cracker in your pocket

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

Message-Id: <48046F4F.76E4.0078.0@novell.com>
Date:	Tue, 15 Apr 2008 08:03:11 +0100
From:	"Jan Beulich" <jbeulich@...ell.com>
To:	"Jeremy Fitzhardinge" <jeremy@...p.org>
Cc:	"Ingo Molnar" <mingo@...e.hu>, <tglx@...utronix.de>,
	<linux-kernel@...r.kernel.org>, <hpa@...or.com>
Subject: Re: [RFC] x86: bitops asm constraint fixes

>>> Jeremy Fitzhardinge <jeremy@...p.org> 14.04.08 18:21 >>>
>Jan Beulich wrote:
>>  
>> +struct __bits { int _[1UL << (32 - 3 - sizeof(int))]; };
>>   
>
>I don't understand what you're doing here.  The array can be 1<<(32 - 1) 
>bytes (assuming we never allow a 64-bit bit offset).  The int array 
>makes that 1<<(32 - 1 - log2(sizeof(int))) ints.  But I don't see what 
>the sizeof(int) is doing in there.

The array really can be only 1<<(32-1) bits (the bit offset is signed,
but I intentionally neglect this here for not further complicating things,
since its meaningless in this context), hence 1<<(32-4) bytes and
1<<(32-6) ints. So the -3 in there represents the bits-to-bytes
conversion, and the sizeof(int) is for the bytes-to-ints one.

>> +
>>  #if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 1)
>>  /* 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])
>> +#define ADDR "=m" (*(volatile long *) addr)
>> +#define BIT_ADDR "=m" (((volatile int *) addr)[nr >> 5])
>> +#define FULL_ADDR "=m" (*(volatile struct __bits *) addr)
>>  #else
>>  #define ADDR "+m" (*(volatile long *) addr)
>> -#define BIT_ADDR "+m" (((volatile int *)addr)[nr >> 5])
>> +#define BIT_ADDR "+m" (((volatile int *) addr)[nr >> 5])
>> +#define FULL_ADDR "+m" (*(volatile struct __bits *) addr)
>>  #endif
>> -#define BASE_ADDR "m" (*(volatile int *)addr)
>> +#define BASE_ADDR "m" (*(volatile int *) addr)
>>   
>
>Shouldn't BASE_ADDR also use __bits?  Otherwise it won't get write-read 
>dependencies right (a read could move before a write).

No, BASE_ADDR is used for the address calculation that the raw bt?
instructions are to use, BIT_ADDR specifies the actual field that
changes. They are always used together (and it's BIT_ADDR that's
responsible for representing the dependency), and always under
__builtin_constant_p(nr) (so we know the compiler doesn't need to
generate extra code for computing the [not needed in the instruction]
second address).

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/