Messages in this thread | | | From | Andy Lutomirski <> | Date | Sat, 7 Nov 2015 08:49:46 -0800 | Subject | Re: [PATCH 3/4] x86/asm: Add asm macros for static keys/jump labels |
| |
On Nov 7, 2015 3:21 AM, "Thomas Gleixner" <tglx@linutronix.de> wrote: > > On Fri, 6 Nov 2015, Andy Lutomirski wrote: > > > Unfortunately, these only work if HAVE_JUMP_LABEL. In principle, we > > could do some serious surgery on the core jump label infrastructure > > to keep the patch infrastructure available on x86 on all builds, but > > that's probably not worth it. > > No, but we can be smarter about it. There is nothing which that asm > entry code needs from linux/jump_label.h and asm/jump_label.h execpt > STATIC_KEY_INIT_NOP. Something like the patch below should do the > trick.
I like some bits of this. See below.
> > Thanks, > > tglx > > Index: tip/arch/x86/entry/calling.h > =================================================================== > --- tip.orig/arch/x86/entry/calling.h > +++ tip/arch/x86/entry/calling.h > @@ -232,3 +232,22 @@ For 32-bit we have the following convent > > #endif /* CONFIG_X86_64 */ > > +/* ASM jump label support */ > +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > +#include <asm/jump_label.h> > + > + .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.)
> + 1: > + jmp.d32 2f
Old binutils will choke on this. But maybe no one has jump labels and old binutils.
> + 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.
> +#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.
--Andy
| |