From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753974AbbKGRF3 (ORCPT ); Sat, 7 Nov 2015 12:05:29 -0500 Received: from mail-oi0-f45.google.com ([209.85.218.45]:35248 "EHLO mail-oi0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753820AbbKGRF0 (ORCPT ); Sat, 7 Nov 2015 12:05:26 -0500 MIME-Version: 1.0 In-Reply-To: References: <42b51cafe425b8d7d177c7c363235df0ace22692.1446849780.git.luto@kernel.org> From: Andy Lutomirski Date: Sat, 7 Nov 2015 09:05:06 -0800 Message-ID: Subject: Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels To: Thomas Gleixner Cc: Borislav Petkov , Frederic Weisbecker , Brian Gerst , "linux-kernel@vger.kernel.org" , X86 ML , Peter Zijlstra , Linus Torvalds Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Nov 7, 2015 at 8:58 AM, Thomas Gleixner wrote: > On Sat, 7 Nov 2015, Andy Lutomirski wrote: >> On Nov 7, 2015 3:21 AM, "Thomas Gleixner" wrote: >> > +/* ASM jump label support */ >> > +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) >> > +#include >> > + >> > + .macro STATIC_CALL_IF_ENABLED fun, key >> >> I think we still need the "def" parameter. But maybe the "static >> call" is a good idea. (By giving 'def' a default value or omitting it >> entirely, it's too easy not to think about it, and that's a huge >> historical source of breakage and it's a good part of the reason for >> the new improved C API. At least these days I *think* we'll diagnose >> the problem on bootup instead of waiting for random breakage later.) > > Ok. That's easy to add :) > >> > + 1: >> > + jmp.d32 2f >> >> Old binutils will choke on this. But maybe no one has jump labels and >> old binutils. > > Good question. > >> > + call fun >> > + .pushsection __jump_table, "aw" >> > + _ASM_ALIGN >> > + _ASM_PTR 1b, 2f, \key >> > + .popsection >> > + 2: >> > + .endm >> > +#else >> > + .macro STATIC_CALL_IF_ENABLED fun, key >> > + call fun >> > + .endm >> >> This could result in errors down the road, since this macro is really >> "static call if enabled or if the kernel is built without jump >> labels". enter_from_user_mode is special because it's already a no-op >> if context tracking is off, but I'm not sure that this oddity should >> leak into more generic code. > > Hmm. Maybe we need a better name for this macro. > >> > +#endif >> > Index: tip/arch/x86/entry/entry_64.S >> > =================================================================== >> > --- tip.orig/arch/x86/entry/entry_64.S >> > +++ tip/arch/x86/entry/entry_64.S >> > @@ -510,7 +510,7 @@ END(irq_entries_start) >> > */ >> > SWAPGS >> > #ifdef CONFIG_CONTEXT_TRACKING >> > - call enter_from_user_mode >> > + STATIC_CALL_IF_ENABLED enter_from_user_mode, context_tracking_enabled >> > #endif >> >> This doesn't get rid of the ifdeffery, which I thought was a nice >> feature of my patch. > > Right. I was just too lazy to add another macro :) > > My main point was to avoid the extra stuff for !HAVE_JUMP_LABEL and > hide this in a header file. > True. But I hid it in a header file, too, but it was just a different header file -- I had it all hidden away as CALL_ENTER_FROM_USER_MODE. --Andy