linux-kernel - Re: [patch V4 09/31] bitops: Add x86-specific parity functions

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 11 May 2016 11:31:39 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	zengzhaoxiu@....com
Cc:	linux-kernel@...r.kernel.org, bp@...e.de,
	gnomes@...rguk.ukuu.org.uk, andi@...stfloor.org,
	dvyukov@...gle.com, Zhaoxiu Zeng <zhaoxiu.zeng@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	Denys Vlasenko <dvlasenk@...hat.com>
Subject: Re: [patch V4 09/31] bitops: Add x86-specific parity functions

On Wed, May 11, 2016 at 05:16:38PM +0800, zengzhaoxiu@....com wrote:

> +static inline unsigned int __arch_parity4(unsigned int w)
> +{
> +	unsigned int res = 0;
> +
> +	asm("test $0xf, %1; setpo %b0"
> +		: "+q" (res)
> +		: "r" (w)
> +		: "cc");
> +
> +	return res;
> +}
> +
> +static inline unsigned int __arch_parity8(unsigned int w)
> +{
> +	unsigned int res = 0;
> +
> +	asm("test %1, %1; setpo %b0"
> +		: "+q" (res)
> +		: "r" (w)
> +		: "cc");
> +
> +	return res;
> +}
> +
> +static inline unsigned int __arch_parity16(unsigned int w)
> +{
> +	unsigned int res = 0;
> +
> +	asm("xor %h1, %b1; setpo %b0"
> +		: "+q" (res), "+q" (w)
> +		: : "cc");
> +
> +	return res;
> +}

Please use the GEN_*_RMWcc() stuff to avoid the setpo where possible.

> +
> +#ifdef CONFIG_64BIT
> +/* popcnt %eax, %eax -- redundant REX prefix for alignment */
> +#define POPCNT32 ".byte 0xf3,0x40,0x0f,0xb8,0xc0"
> +/* popcnt %rax, %rax */
> +#define POPCNT64 ".byte 0xf3,0x48,0x0f,0xb8,0xc0"
> +#else
> +/* popcnt %eax, %eax */
> +#define POPCNT32 ".byte 0xf3,0x0f,0xb8,0xc0"
> +#endif

Yuck, please don't duplicate stuff like this.

> +
> +static __always_inline unsigned int __arch_parity32(unsigned int w)
> +{
> +	unsigned int res;
> +	unsigned int tmp;
> +
> +	asm(ALTERNATIVE(
> +		"	mov	%%eax, %1	\n"
> +		"	shr	$16, %%eax	\n"
> +		"	xor	%1, %%eax	\n"
> +		"	xor	%%ah, %%al	\n"
> +		"	mov	$0, %%eax	\n"
> +		"	setpo	%%al	\n",
> +		POPCNT32 "			\n"
> +		"	and	$1, %%eax	\n",
> +		X86_FEATURE_POPCNT)
> +		: "=a" (res), "=&r" (tmp)
> +		: "a" (w)
> +		: "cc");
> +
> +	return res;
> +}

How many bytes does that end up being? Should we make it a call?