Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755574AbbGQRRm (ORCPT ); Fri, 17 Jul 2015 13:17:42 -0400 Received: from mail.skyhub.de ([78.46.96.112]:52344 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755538AbbGQRRg (ORCPT ); Fri, 17 Jul 2015 13:17:36 -0400 Date: Fri, 17 Jul 2015 19:17:26 +0200 From: Borislav Petkov To: Josh Poimboeuf Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Michal Marek , Peter Zijlstra , Andy Lutomirski , Linus Torvalds , Andi Kleen , Pedro Alves , x86@kernel.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*() Message-ID: <20150717171726.GA21568@nazgul.tnic> References: <0d8517b7ab757e00a13b3abe2b677d9eb23362be.1437150175.git.jpoimboe@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <0d8517b7ab757e00a13b3abe2b677d9eb23362be.1437150175.git.jpoimboe@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2510 Lines: 68 On Fri, Jul 17, 2015 at 11:47:20AM -0500, Josh Poimboeuf wrote: > If __arch_hweight32() or __arch_hweight64() is inlined at the beginning > of a function, gcc can insert the call instruction before setting up a > stack frame, which breaks frame pointer convention if > CONFIG_FRAME_POINTER is enabled and can result in a bad stack trace. > > Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by > listing the stack pointer as an output operand for the inline asm > statement. > > Signed-off-by: Josh Poimboeuf > --- > arch/x86/include/asm/arch_hweight.h | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h > index 9686c3d..e438a0d 100644 > --- a/arch/x86/include/asm/arch_hweight.h > +++ b/arch/x86/include/asm/arch_hweight.h > @@ -23,10 +23,11 @@ > */ > static inline unsigned int __arch_hweight32(unsigned int w) > { > + register void *__sp asm("esp"); > unsigned int res = 0; > > asm (ALTERNATIVE("call __sw_hweight32", POPCNT32, X86_FEATURE_POPCNT) > - : "="REG_OUT (res) > + : "="REG_OUT (res), "+r" (__sp) > : REG_IN (w)); > > return res; > @@ -44,6 +45,7 @@ static inline unsigned int __arch_hweight8(unsigned int w) > > static inline unsigned long __arch_hweight64(__u64 w) > { > + register void __maybe_unused *__sp asm("rsp"); > unsigned long res = 0; > > #ifdef CONFIG_X86_32 > @@ -51,7 +53,7 @@ static inline unsigned long __arch_hweight64(__u64 w) > __arch_hweight32((u32)(w >> 32)); > #else > asm (ALTERNATIVE("call __sw_hweight64", POPCNT64, X86_FEATURE_POPCNT) > - : "="REG_OUT (res) > + : "="REG_OUT (res), "+r" (__sp) > : REG_IN (w)); > #endif /* CONFIG_X86_32 */ Eeew, useless code so that some compile-time validation is done. Let's not add this clutter please. In this particular case, the majority of CPUs out there will get POPCNT patched in and that CALL is gone. And for the remaining cases where we do end up using the __sw_* variants, I'd prefer to rather not do the validation instead of polluting the code with that fake rsp dependency. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/