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
| ||
|
Date: Sat, 6 Feb 2010 10:36:59 +0100 From: Borislav Petkov <bp@...64.org> To: "H. Peter Anvin" <hpa@...or.com> Cc: Peter Zijlstra <peterz@...radead.org>, Andrew Morton <akpm@...ux-foundation.org>, Wu Fengguang <fengguang.wu@...el.com>, LKML <linux-kernel@...r.kernel.org>, Jamie Lokier <jamie@...reable.org>, Roland Dreier <rdreier@...co.com>, Al Viro <viro@...IV.linux.org.uk>, "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>, Brian Gerst <brgerst@...il.com> Subject: Re: [PATCH 2/5] bitops: compile time optimization for hweight_long(CONSTANT) On Fri, Feb 05, 2010 at 01:54:42PM -0800, H. Peter Anvin wrote: > On 02/05/2010 04:11 AM, Borislav Petkov wrote: > > + > > +unsigned int __arch_hweight16(unsigned int w) > > +{ > > + unsigned int res = 0; > > + > > + asm volatile("xor %%dh, %%dh\n\t" > > + __arch_hweight_alt(32) > > + : "=di" (res) > > + : "di" (w) > > + : "ecx", "memory"); > > + > > This is wrong in more ways than I can shake a stick at. Thanks for reviewing it though - how else would I learn :). > a) "di" doesn't mean the DI register - it means the DX register (d) or > an immediate (i). Since you don't have any reference to either %0 or %1 > in your code, you have no way of knowing which one it is. The > constraint for the di register is "D". right. > b) On 32 bits, the first argument register is in %eax (with %edx used > for the upper half of a 32-bit argument), but on 64 bits, the first > argument is in %rdi, with the return still in %rax. Sure, it is right there in arch/x86/include/asm/calling.h. Shame on me. > c) You call a C function, but you don't clobber the set of registers > that a C function would clobber. You either need to put the function in > an assembly wrapper (which is better in the long run), or clobber the > full set of registers that is clobbered by a C function (which is better > in the short term) -- which is eax, edx, ecx on 32 bits, but rax, rdi, > esi, rdx, rcx, r8, r9, r10, r11 on 64 bits. I think you mean rsi instead of esi here. Well, the example Brian pointed me to - __mutex_fastpath_lock - lists the full set of clobbered registers. Please elaborate on the assembly wrapper for the function, wouldn't I need to list all the clobbered registers there too or am I missing something? > d) On the other hand, you do *not* need a "memory" clobber. Right, in this case we have all non-barrier like inlines so no memory clobber, according to the comment above alternative() macro. Thanks. -- Regards/Gruss, Boris. - Advanced Micro Devices, Inc. Operating Systems Research Center -- 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