[PATCH 0/5] add support for relative references in jump tables

[PATCH 0/5] add support for relative references in jump tables

[Ard Biesheuvel]

This series implements support for emitting the data structures associated
with jump tables as 32-bit relative references instead of absolute
references, which take up more space on builds that target 64-bit
architectures, or implement self relocation [or both].

This series enables it for arm64 and x86, although other architectures
might benefit as well.

Patch #1 does some preparatory refactoring before patch #2 introduces the
generic pieces required for using relative references.

Patch #3 wires everything up for arm64.

For x86, patch #4 applies some preparatory changes for the arch specific
jump label C code, which is a lot more involved than on arm64, which is
why it is split off in this case. Patch #5 wires it up for x86 as well.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com> 
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>

Ard Biesheuvel (5):
  kernel/jump_label: abstract jump_entry member accessors
  kernel/jump_label: implement generic support for relative references
  arm64/kernel: jump_label: switch to relative references
  x86: jump_label: switch to jump_entry accessors
  x86/kernel: jump_table: use relative references

 arch/Kconfig                        |  3 +
 arch/arm64/Kconfig                  |  1 +
 arch/arm64/include/asm/jump_label.h | 16 ++---
 arch/arm64/kernel/jump_label.c      |  6 +-
 arch/x86/Kconfig                    |  1 +
 arch/x86/include/asm/jump_label.h   | 28 +++------
 arch/x86/kernel/jump_label.c        | 43 +++++++------
 include/linux/jump_label.h          | 64 ++++++++++++++++++++
 kernel/jump_label.c                 | 56 ++++++++++-------
 tools/objtool/special.c             |  4 +-
 10 files changed, 145 insertions(+), 77 deletions(-)

-- 
2.11.0

[PATCH 1/5] kernel/jump_label: abstract jump_entry member accessors

[Ard Biesheuvel]

In preparation of allowing architectures to use relative references
in jump_label entries [which can dramatically reduce the memory
footprint], introduce abstractions for references to the 'code' and
'key' members of struct jump_entry.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 include/linux/jump_label.h | 36 ++++++++++++++++++++
 kernel/jump_label.c        | 36 ++++++++------------
 2 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index b46b541c67c4..86ec0652d3b1 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -119,6 +119,42 @@ struct static_key {
 
 #ifdef HAVE_JUMP_LABEL
 #include <asm/jump_label.h>
+
+#ifndef __ASSEMBLY__
+
+struct jump_entry; /* defined by the architecture */
+
+static inline unsigned long jump_entry_code(const struct jump_entry *entry)
+{
+	return entry->code;
+}
+
+static inline unsigned long jump_entry_target(const struct jump_entry *entry)
+{
+	return entry->target;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+	return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+	return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+	return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+	entry->code = 0;
+}
+
+#endif
 #endif
 
 #ifndef __ASSEMBLY__
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 01ebdf1f9f40..c3524c9b3004 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -38,10 +38,12 @@ static int jump_label_cmp(const void *a, const void *b)
 	const struct jump_entry *jea = a;
 	const struct jump_entry *jeb = b;
 
-	if (jea->key < jeb->key)
+	if ((unsigned long)jump_entry_key(jea) <
+	    (unsigned long)jump_entry_key(jeb))
 		return -1;
 
-	if (jea->key > jeb->key)
+	if ((unsigned long)jump_entry_key(jea) >
+	    (unsigned long)jump_entry_key(jeb))
 		return 1;
 
 	return 0;
@@ -261,8 +263,8 @@ EXPORT_SYMBOL_GPL(jump_label_rate_limit);
 
 static int addr_conflict(struct jump_entry *entry, void *start, void *end)
 {
-	if (entry->code <= (unsigned long)end &&
-		entry->code + JUMP_LABEL_NOP_SIZE > (unsigned long)start)
+	if (jump_entry_code(entry) <= (unsigned long)end &&
+	    jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE > (unsigned long)start)
 		return 1;
 
 	return 0;
@@ -321,16 +323,6 @@ static inline void static_key_set_linked(struct static_key *key)
 	key->type |= JUMP_TYPE_LINKED;
 }
 
-static inline struct static_key *jump_entry_key(struct jump_entry *entry)
-{
-	return (struct static_key *)((unsigned long)entry->key & ~1UL);
-}
-
-static bool jump_entry_branch(struct jump_entry *entry)
-{
-	return (unsigned long)entry->key & 1UL;
-}
-
 /***
  * A 'struct static_key' uses a union such that it either points directly
  * to a table of 'struct jump_entry' or to a linked list of modules which in
@@ -355,7 +347,7 @@ static enum jump_label_type jump_label_type(struct jump_entry *entry)
 {
 	struct static_key *key = jump_entry_key(entry);
 	bool enabled = static_key_enabled(key);
-	bool branch = jump_entry_branch(entry);
+	bool branch = jump_entry_is_branch(entry);
 
 	/* See the comment in linux/jump_label.h */
 	return enabled ^ branch;
@@ -370,8 +362,8 @@ static void __jump_label_update(struct static_key *key,
 		 * An entry->code of 0 indicates an entry which has been
 		 * disabled because it was in an init text area.
 		 */
-		if (entry->code) {
-			if (kernel_text_address(entry->code))
+		if (!jump_entry_is_module_init(entry)) {
+			if (kernel_text_address(jump_entry_code(entry)))
 				arch_jump_label_transform(entry, jump_label_type(entry));
 			else
 				WARN_ONCE(1, "can't patch jump_label at %pS",
@@ -441,7 +433,7 @@ static enum jump_label_type jump_label_init_type(struct jump_entry *entry)
 {
 	struct static_key *key = jump_entry_key(entry);
 	bool type = static_key_type(key);
-	bool branch = jump_entry_branch(entry);
+	bool branch = jump_entry_is_branch(entry);
 
 	/* See the comment in linux/jump_label.h */
 	return type ^ branch;
@@ -565,7 +557,7 @@ static int jump_label_add_module(struct module *mod)
 			continue;
 
 		key = iterk;
-		if (within_module(iter->key, mod)) {
+		if (within_module((unsigned long)key, mod)) {
 			static_key_set_entries(key, iter);
 			continue;
 		}
@@ -615,7 +607,7 @@ static void jump_label_del_module(struct module *mod)
 
 		key = jump_entry_key(iter);
 
-		if (within_module(iter->key, mod))
+		if (within_module((unsigned long)key, mod))
 			continue;
 
 		/* No memory during module load */
@@ -659,8 +651,8 @@ static void jump_label_invalidate_module_init(struct module *mod)
 	struct jump_entry *iter;
 
 	for (iter = iter_start; iter < iter_stop; iter++) {
-		if (within_module_init(iter->code, mod))
-			iter->code = 0;
+		if (within_module_init(jump_entry_code(iter), mod))
+			jump_entry_set_module_init(iter);
 	}
 }
 
-- 
2.11.0

[PATCH 1/5] kernel/jump_label: abstract jump_entry member accessors

[Peter Zijlstra]

On Wed, Jun 27, 2018 at 06:06:00PM +0200, Ard Biesheuvel wrote:

> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index 01ebdf1f9f40..c3524c9b3004 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -38,10 +38,12 @@ static int jump_label_cmp(const void *a, const void *b)
>  	const struct jump_entry *jea = a;
>  	const struct jump_entry *jeb = b;
>  
> -	if (jea->key < jeb->key)
> +	if ((unsigned long)jump_entry_key(jea) <
> +	    (unsigned long)jump_entry_key(jeb))
>  		return -1;
>  
> -	if (jea->key > jeb->key)
> +	if ((unsigned long)jump_entry_key(jea) >
> +	    (unsigned long)jump_entry_key(jeb))
>  		return 1;

I think you can ditch the unsigned long cast and directly compare
pointers. That leads to much prettier code:

	if (jump_entry_key(jea) < jump_entry_key(jeb))
		return -1;

etc..

[PATCH 2/5] kernel/jump_label: implement generic support for relative references

[Ard Biesheuvel]

To reduce the size taken up by absolute references in jump label
entries themselves and the associated relocation records in the
.init segment, add support for emitting them as 32-bit relative
references instead.

Note that this requires some extra care in the sorting routine, given
that the offsets change when entries are moved around in the jump_entry
table.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/Kconfig               |  3 +++
 include/linux/jump_label.h | 28 ++++++++++++++++++++
 kernel/jump_label.c        | 20 +++++++++++++-
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 2b8b70820002..22fa3792626e 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -348,6 +348,9 @@ config HAVE_PERF_USER_STACK_DUMP
 config HAVE_ARCH_JUMP_LABEL
 	bool
 
+config HAVE_ARCH_JUMP_LABEL_RELATIVE
+	bool
+
 config HAVE_RCU_TABLE_FREE
 	bool
 
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 86ec0652d3b1..aa203dffe72c 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -121,6 +121,32 @@ struct static_key {
 #include <asm/jump_label.h>
 
 #ifndef __ASSEMBLY__
+#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE
+
+struct jump_entry {
+	int code;
+	int target;
+	int key;
+};
+
+static inline unsigned long jump_entry_code(const struct jump_entry *entry)
+{
+	return (unsigned long)&entry->code + entry->code;
+}
+
+static inline unsigned long jump_entry_target(const struct jump_entry *entry)
+{
+	return (unsigned long)&entry->target + entry->target;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+	unsigned long key = (unsigned long)&entry->key + entry->key;
+
+	return (struct static_key *)(key & ~1UL);
+}
+
+#else
 
 struct jump_entry; /* defined by the architecture */
 
@@ -139,6 +165,8 @@ static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
 	return (struct static_key *)((unsigned long)entry->key & ~1UL);
 }
 
+#endif
+
 static inline bool jump_entry_is_branch(const struct jump_entry *entry)
 {
 	return (unsigned long)entry->key & 1UL;
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index c3524c9b3004..285eff13ecd1 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -49,6 +49,22 @@ static int jump_label_cmp(const void *a, const void *b)
 	return 0;
 }
 
+static void jump_label_swap(void *a, void *b, int size)
+{
+	long delta = (unsigned long)a - (unsigned long)b;
+	struct jump_entry *jea = a;
+	struct jump_entry *jeb = b;
+	struct jump_entry tmp = *jea;
+
+	jea->code	= jeb->code - delta;
+	jea->target	= jeb->target - delta;
+	jea->key	= jeb->key - delta;
+
+	jeb->code	= tmp.code + delta;
+	jeb->target	= tmp.target + delta;
+	jeb->key	= tmp.key + delta;
+}
+
 static void
 jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
 {
@@ -56,7 +72,9 @@ jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
 
 	size = (((unsigned long)stop - (unsigned long)start)
 					/ sizeof(struct jump_entry));
-	sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL);
+	sort(start, size, sizeof(struct jump_entry), jump_label_cmp,
+	     IS_ENABLED(CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE) ? jump_label_swap
+							      : NULL);
 }
 
 static void jump_label_update(struct static_key *key);
-- 
2.11.0

[PATCH 2/5] kernel/jump_label: implement generic support for relative references

[Peter Zijlstra]

On Wed, Jun 27, 2018 at 06:06:01PM +0200, Ard Biesheuvel wrote:
> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> index 86ec0652d3b1..aa203dffe72c 100644
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -121,6 +121,32 @@ struct static_key {
>  #include <asm/jump_label.h>
>  
>  #ifndef __ASSEMBLY__
> +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE
> +
> +struct jump_entry {
> +	int code;
> +	int target;
> +	int key;
> +};

I much prefer you use 'u32' there.


> +static void jump_label_swap(void *a, void *b, int size)
> +{
> +	long delta = (unsigned long)a - (unsigned long)b;
> +	struct jump_entry *jea = a;
> +	struct jump_entry *jeb = b;
> +	struct jump_entry tmp = *jea;
> +
> +	jea->code	= jeb->code - delta;
> +	jea->target	= jeb->target - delta;
> +	jea->key	= jeb->key - delta;
> +
> +	jeb->code	= tmp.code + delta;
> +	jeb->target	= tmp.target + delta;
> +	jeb->key	= tmp.key + delta;
> +}
> +
>  static void
>  jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
>  {
> @@ -56,7 +72,9 @@ jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
>  
>  	size = (((unsigned long)stop - (unsigned long)start)
>  					/ sizeof(struct jump_entry));
> -	sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL);
> +	sort(start, size, sizeof(struct jump_entry), jump_label_cmp,
> +	     IS_ENABLED(CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE) ? jump_label_swap
> +							      : NULL);
>  }

That will result in jump_label_swap being an unused symbol for some
compile options.

Would it not be much nicer to write that like:

static void jump_label_swap(void *a, void *b, int size)
{
	struct jump_entry *jea = a, *jeb = b;

#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE
	long delta = a - b;

	jea->code += delta;
	jea->target += delta;
	jea->key += delta;

	jeb->code -= delta;
	jeb->target -= delta;
	jeb->key -= delta;
#else

	swap(*jea, *jeb);
}

And then unconditionally use jump_label_swap().

[PATCH 2/5] kernel/jump_label: implement generic support for relative references

[Ard Biesheuvel]

On 28 June 2018 at 10:50, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Jun 27, 2018 at 06:06:01PM +0200, Ard Biesheuvel wrote:
>> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
>> index 86ec0652d3b1..aa203dffe72c 100644
>> --- a/include/linux/jump_label.h
>> +++ b/include/linux/jump_label.h
>> @@ -121,6 +121,32 @@ struct static_key {
>>  #include <asm/jump_label.h>
>>
>>  #ifndef __ASSEMBLY__
>> +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE
>> +
>> +struct jump_entry {
>> +     int code;
>> +     int target;
>> +     int key;
>> +};
>
> I much prefer you use 'u32' there.
>

Actually, they are signed so that would be s32. But yeah, I can change that.

>
>> +static void jump_label_swap(void *a, void *b, int size)
>> +{
>> +     long delta = (unsigned long)a - (unsigned long)b;
>> +     struct jump_entry *jea = a;
>> +     struct jump_entry *jeb = b;
>> +     struct jump_entry tmp = *jea;
>> +
>> +     jea->code       = jeb->code - delta;
>> +     jea->target     = jeb->target - delta;
>> +     jea->key        = jeb->key - delta;
>> +
>> +     jeb->code       = tmp.code + delta;
>> +     jeb->target     = tmp.target + delta;
>> +     jeb->key        = tmp.key + delta;
>> +}
>> +
>>  static void
>>  jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
>>  {
>> @@ -56,7 +72,9 @@ jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
>>
>>       size = (((unsigned long)stop - (unsigned long)start)
>>                                       / sizeof(struct jump_entry));
>> -     sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL);
>> +     sort(start, size, sizeof(struct jump_entry), jump_label_cmp,
>> +          IS_ENABLED(CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE) ? jump_label_swap
>> +                                                           : NULL);
>>  }
>
> That will result in jump_label_swap being an unused symbol for some
> compile options.
>

No, and isn't that the point of IS_ENABLED()? The compiler sees a
reference to jump_label_swap(), so it won't complain about it being
unused.

> Would it not be much nicer to write that like:
>
> static void jump_label_swap(void *a, void *b, int size)
> {
>         struct jump_entry *jea = a, *jeb = b;
>
> #ifdef CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE
>         long delta = a - b;
>
>         jea->code += delta;
>         jea->target += delta;
>         jea->key += delta;
>
>         jeb->code -= delta;
>         jeb->target -= delta;
>         jeb->key -= delta;
> #else
>
>         swap(*jea, *jeb);
> }
>
> And then unconditionally use jump_label_swap().

Meh. I thought IS_ENABLED() was preferred over #ifdef, no? That way,
the compiler always sees the code, and simply discards it without
complaining if it ends up left unused.

[PATCH 2/5] kernel/jump_label: implement generic support for relative references

[Ard Biesheuvel]

On 28 June 2018 at 11:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 28 June 2018 at 10:50, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Wed, Jun 27, 2018 at 06:06:01PM +0200, Ard Biesheuvel wrote:
>>> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
>>> index 86ec0652d3b1..aa203dffe72c 100644
>>> --- a/include/linux/jump_label.h
>>> +++ b/include/linux/jump_label.h
>>> @@ -121,6 +121,32 @@ struct static_key {
>>>  #include <asm/jump_label.h>
>>>
>>>  #ifndef __ASSEMBLY__
>>> +#ifdef CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE
>>> +
>>> +struct jump_entry {
>>> +     int code;
>>> +     int target;
>>> +     int key;
>>> +};
>>
>> I much prefer you use 'u32' there.
>>
>
> Actually, they are signed so that would be s32. But yeah, I can change that.
>
>>
>>> +static void jump_label_swap(void *a, void *b, int size)
>>> +{
>>> +     long delta = (unsigned long)a - (unsigned long)b;
>>> +     struct jump_entry *jea = a;
>>> +     struct jump_entry *jeb = b;
>>> +     struct jump_entry tmp = *jea;
>>> +
>>> +     jea->code       = jeb->code - delta;
>>> +     jea->target     = jeb->target - delta;
>>> +     jea->key        = jeb->key - delta;
>>> +
>>> +     jeb->code       = tmp.code + delta;
>>> +     jeb->target     = tmp.target + delta;
>>> +     jeb->key        = tmp.key + delta;
>>> +}
>>> +
>>>  static void
>>>  jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
>>>  {
>>> @@ -56,7 +72,9 @@ jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
>>>
>>>       size = (((unsigned long)stop - (unsigned long)start)
>>>                                       / sizeof(struct jump_entry));
>>> -     sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL);
>>> +     sort(start, size, sizeof(struct jump_entry), jump_label_cmp,
>>> +          IS_ENABLED(CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE) ? jump_label_swap
>>> +                                                           : NULL);
>>>  }
>>
>> That will result in jump_label_swap being an unused symbol for some
>> compile options.
>>
>
> No, and isn't that the point of IS_ENABLED()? The compiler sees a
> reference to jump_label_swap(), so it won't complain about it being
> unused.
>
>> Would it not be much nicer to write that like:
>>
>> static void jump_label_swap(void *a, void *b, int size)
>> {
>>         struct jump_entry *jea = a, *jeb = b;
>>
>> #ifdef CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE
>>         long delta = a - b;
>>
>>         jea->code += delta;
>>         jea->target += delta;
>>         jea->key += delta;
>>
>>         jeb->code -= delta;
>>         jeb->target -= delta;
>>         jeb->key -= delta;
>> #else
>>
>>         swap(*jea, *jeb);
>> }
>>
>> And then unconditionally use jump_label_swap().
>
> Meh. I thought IS_ENABLED() was preferred over #ifdef, no? That way,
> the compiler always sees the code, and simply discards it without
> complaining if it ends up left unused.

... and it means the sort() routine will unconditionally perform an
indirect function call even if the arch does not require it.

[PATCH 2/5] kernel/jump_label: implement generic support for relative references

[Peter Zijlstra]

On Thu, Jun 28, 2018 at 11:04:45AM +0200, Ard Biesheuvel wrote:
> On 28 June 2018 at 11:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >>> @@ -56,7 +72,9 @@ jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
> >>>
> >>>       size = (((unsigned long)stop - (unsigned long)start)
> >>>                                       / sizeof(struct jump_entry));
> >>> -     sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL);
> >>> +     sort(start, size, sizeof(struct jump_entry), jump_label_cmp,
> >>> +          IS_ENABLED(CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE) ? jump_label_swap
> >>> +                                                           : NULL);
> >>>  }
> >>
> >> That will result in jump_label_swap being an unused symbol for some
> >> compile options.
> >
> > No, and isn't that the point of IS_ENABLED()? The compiler sees a
> > reference to jump_label_swap(), so it won't complain about it being
> > unused.

Ah, ok. I hadn't figured it was quite that smart about it.

> > Meh. I thought IS_ENABLED() was preferred over #ifdef, no?

Dunno, I just reacted to the proposed code's uglyness :-)

> ... and it means the sort() routine will unconditionally perform an
> indirect function call even if the arch does not require it.

Yeah, not sure I care about that here, this is a one time affair, very
far away from any fast paths.

[PATCH 2/5] kernel/jump_label: implement generic support for relative references

[Ard Biesheuvel]

On 28 June 2018 at 11:25, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jun 28, 2018 at 11:04:45AM +0200, Ard Biesheuvel wrote:
>> On 28 June 2018 at 11:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >>> @@ -56,7 +72,9 @@ jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
>> >>>
>> >>>       size = (((unsigned long)stop - (unsigned long)start)
>> >>>                                       / sizeof(struct jump_entry));
>> >>> -     sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL);
>> >>> +     sort(start, size, sizeof(struct jump_entry), jump_label_cmp,
>> >>> +          IS_ENABLED(CONFIG_HAVE_ARCH_JUMP_LABEL_RELATIVE) ? jump_label_swap
>> >>> +                                                           : NULL);
>> >>>  }
>> >>
>> >> That will result in jump_label_swap being an unused symbol for some
>> >> compile options.
>> >
>> > No, and isn't that the point of IS_ENABLED()? The compiler sees a
>> > reference to jump_label_swap(), so it won't complain about it being
>> > unused.
>
> Ah, ok. I hadn't figured it was quite that smart about it.
>

Yeah. I could use a temp variable to make the indentation less
obnoxious, but since this is an opt-in feature, I'd like to preserve
the NULL (*swap)() argument for the existing users.

>> > Meh. I thought IS_ENABLED() was preferred over #ifdef, no?
>
> Dunno, I just reacted to the proposed code's uglyness :-)
>

I will try to come up with something that rhymes, ok? :-)

>> ... and it means the sort() routine will unconditionally perform an
>> indirect function call even if the arch does not require it.
>
> Yeah, not sure I care about that here, this is a one time affair, very
> far away from any fast paths.
>

Fair enough.

[PATCH 2/5] kernel/jump_label: implement generic support for relative references

[Peter Zijlstra]

On Thu, Jun 28, 2018 at 11:29:40AM +0200, Ard Biesheuvel wrote:
> On 28 June 2018 at 11:25, Peter Zijlstra <peterz@infradead.org> wrote:
> >> > Meh. I thought IS_ENABLED() was preferred over #ifdef, no?
> >
> > Dunno, I just reacted to the proposed code's uglyness :-)
> >
> 
> I will try to come up with something that rhymes, ok? :-)

:-)

> >> ... and it means the sort() routine will unconditionally perform an
> >> indirect function call even if the arch does not require it.
> >
> > Yeah, not sure I care about that here, this is a one time affair, very
> > far away from any fast paths.
> >
> 
> Fair enough.

I recently had thoughts about doing the sort at link time, but then I
figured I didn't care enough to go write the tool to do that. The reason
was in that other patch-set that might conflict, that wants to use jump
labels _very_ early (for x86), so we had to lift the ideal nops stuff
and the cpu feature bits it relies upon to early code.

All in all the patch wasn't terrible and that made me completely loose
interest about doing the link-time thing again.

However, if someone is 'concerned' about the boot time performance, and
figures avoiding the sort is somehow worth it, they can look into it.

[PATCH 3/5] arm64/kernel: jump_label: switch to relative references

[Ard Biesheuvel]

On a randomly chosen distro kernel build for arm64, vmlinux.o shows the
following sections, containing jump label entries, and the associated
RELA relocation records, respectively:

  ...
  [38088] __jump_table      PROGBITS         0000000000000000  00e19f30
       000000000002ea10  0000000000000000  WA       0     0     8
  [38089] .rela__jump_table RELA             0000000000000000  01fd8bb0
       000000000008be30  0000000000000018   I      38178   38088     8
  ...

In other words, we have 190 KB worth of 'struct jump_entry' instances,
and 573 KB worth of RELA entries to relocate each entry's code, target
and key members. This means the RELA section occupies 10% of the .init
segment, and the two sections combined represent 5% of vmlinux's entire
memory footprint.

So let's switch from 64-bit absolute references to 32-bit relative
references: this reduces the size of the __jump_table by 50%, and gets
rid of the RELA section entirely.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/Kconfig                  |  1 +
 arch/arm64/include/asm/jump_label.h | 16 ++++------------
 arch/arm64/kernel/jump_label.c      |  6 +++---
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1940c6405d04..d17aa9614e69 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -91,6 +91,7 @@ config ARM64
 	select HAVE_ARCH_BITREVERSE
 	select HAVE_ARCH_HUGE_VMAP
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_ARCH_JUMP_LABEL_RELATIVE
 	select HAVE_ARCH_KASAN if !(ARM64_16K_PAGES && ARM64_VA_BITS_48)
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_MMAP_RND_BITS
diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
index 1b5e0e843c3a..5e3e31df3805 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -30,8 +30,8 @@ static __always_inline bool arch_static_branch(struct static_key *key, bool bran
 {
 	asm goto("1: nop\n\t"
 		 ".pushsection __jump_table,  \"aw\"\n\t"
-		 ".align 3\n\t"
-		 ".quad 1b, %l[l_yes], %c0\n\t"
+		 ".align 2\n\t"
+		 ".long 1b - ., %l[l_yes] - ., %c0 - .\n\t"
 		 ".popsection\n\t"
 		 :  :  "i"(&((char *)key)[branch]) :  : l_yes);
 
@@ -44,8 +44,8 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
 {
 	asm goto("1: b %l[l_yes]\n\t"
 		 ".pushsection __jump_table,  \"aw\"\n\t"
-		 ".align 3\n\t"
-		 ".quad 1b, %l[l_yes], %c0\n\t"
+		 ".align 2\n\t"
+		 ".long 1b - ., %l[l_yes] - ., %c0 - .\n\t"
 		 ".popsection\n\t"
 		 :  :  "i"(&((char *)key)[branch]) :  : l_yes);
 
@@ -54,13 +54,5 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
 	return true;
 }
 
-typedef u64 jump_label_t;
-
-struct jump_entry {
-	jump_label_t code;
-	jump_label_t target;
-	jump_label_t key;
-};
-
 #endif  /* __ASSEMBLY__ */
 #endif	/* __ASM_JUMP_LABEL_H */
diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c
index c2dd1ad3e648..903d17023d77 100644
--- a/arch/arm64/kernel/jump_label.c
+++ b/arch/arm64/kernel/jump_label.c
@@ -25,12 +25,12 @@
 void arch_jump_label_transform(struct jump_entry *entry,
 			       enum jump_label_type type)
 {
-	void *addr = (void *)entry->code;
+	void *addr = (void *)jump_entry_code(entry);
 	u32 insn;
 
 	if (type == JUMP_LABEL_JMP) {
-		insn = aarch64_insn_gen_branch_imm(entry->code,
-						   entry->target,
+		insn = aarch64_insn_gen_branch_imm(jump_entry_code(entry),
+						   jump_entry_target(entry),
 						   AARCH64_INSN_BRANCH_NOLINK);
 	} else {
 		insn = aarch64_insn_gen_nop();
-- 
2.11.0

[PATCH 3/5] arm64/kernel: jump_label: switch to relative references

[Will Deacon]

On Wed, Jun 27, 2018 at 06:06:02PM +0200, Ard Biesheuvel wrote:
> On a randomly chosen distro kernel build for arm64, vmlinux.o shows the
> following sections, containing jump label entries, and the associated
> RELA relocation records, respectively:
> 
>   ...
>   [38088] __jump_table      PROGBITS         0000000000000000  00e19f30
>        000000000002ea10  0000000000000000  WA       0     0     8
>   [38089] .rela__jump_table RELA             0000000000000000  01fd8bb0
>        000000000008be30  0000000000000018   I      38178   38088     8
>   ...
> 
> In other words, we have 190 KB worth of 'struct jump_entry' instances,
> and 573 KB worth of RELA entries to relocate each entry's code, target
> and key members. This means the RELA section occupies 10% of the .init
> segment, and the two sections combined represent 5% of vmlinux's entire
> memory footprint.
> 
> So let's switch from 64-bit absolute references to 32-bit relative
> references: this reduces the size of the __jump_table by 50%, and gets
> rid of the RELA section entirely.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/Kconfig                  |  1 +
>  arch/arm64/include/asm/jump_label.h | 16 ++++------------
>  arch/arm64/kernel/jump_label.c      |  6 +++---
>  3 files changed, 8 insertions(+), 15 deletions(-)

Looks good, cheers:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH 4/5] x86: jump_label: switch to jump_entry accessors

[Ard Biesheuvel]

In preparation of switching x86 to use place-relative references for
the code, target and key members of struct jump_entry, replace direct
references to the struct members with invocations of the new accessors.
This will allow us to make the switch by modifying the accessors only.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/kernel/jump_label.c | 43 ++++++++++++--------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index e56c95be2808..d64296092ef5 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -52,22 +52,24 @@ static void __jump_label_transform(struct jump_entry *entry,
 			 * Jump label is enabled for the first time.
 			 * So we expect a default_nop...
 			 */
-			if (unlikely(memcmp((void *)entry->code, default_nop, 5)
-				     != 0))
-				bug_at((void *)entry->code, __LINE__);
+			if (unlikely(memcmp((void *)jump_entry_code(entry),
+					    default_nop, 5) != 0))
+				bug_at((void *)jump_entry_code(entry),
+				       __LINE__);
 		} else {
 			/*
 			 * ...otherwise expect an ideal_nop. Otherwise
 			 * something went horribly wrong.
 			 */
-			if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
-				     != 0))
-				bug_at((void *)entry->code, __LINE__);
+			if (unlikely(memcmp((void *)jump_entry_code(entry),
+					    ideal_nop, 5) != 0))
+				bug_at((void *)jump_entry_code(entry),
+				       __LINE__);
 		}
 
 		code.jump = 0xe9;
-		code.offset = entry->target -
-				(entry->code + JUMP_LABEL_NOP_SIZE);
+		code.offset = jump_entry_target(entry) -
+			      (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
 	} else {
 		/*
 		 * We are disabling this jump label. If it is not what
@@ -76,14 +78,18 @@ static void __jump_label_transform(struct jump_entry *entry,
 		 * are converting the default nop to the ideal nop.
 		 */
 		if (init) {
-			if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
-				bug_at((void *)entry->code, __LINE__);
+			if (unlikely(memcmp((void *)jump_entry_code(entry),
+					    default_nop, 5) != 0))
+				bug_at((void *)jump_entry_code(entry),
+				       __LINE__);
 		} else {
 			code.jump = 0xe9;
-			code.offset = entry->target -
-				(entry->code + JUMP_LABEL_NOP_SIZE);
-			if (unlikely(memcmp((void *)entry->code, &code, 5) != 0))
-				bug_at((void *)entry->code, __LINE__);
+			code.offset = jump_entry_target(entry) -
+				(jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
+			if (unlikely(memcmp((void *)jump_entry_code(entry),
+				     &code, 5) != 0))
+				bug_at((void *)jump_entry_code(entry),
+				       __LINE__);
 		}
 		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
 	}
@@ -97,10 +103,13 @@ static void __jump_label_transform(struct jump_entry *entry,
 	 *
 	 */
 	if (poker)
-		(*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
+		(*poker)((void *)jump_entry_code(entry), &code,
+			 JUMP_LABEL_NOP_SIZE);
 	else
-		text_poke_bp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE,
-			     (void *)entry->code + JUMP_LABEL_NOP_SIZE);
+		text_poke_bp((void *)jump_entry_code(entry), &code,
+			     JUMP_LABEL_NOP_SIZE,
+			     (void *)jump_entry_code(entry) +
+			     JUMP_LABEL_NOP_SIZE);
 }
 
 void arch_jump_label_transform(struct jump_entry *entry,
-- 
2.11.0

[PATCH 4/5] x86: jump_label: switch to jump_entry accessors

[Peter Zijlstra]

On Wed, Jun 27, 2018 at 06:06:03PM +0200, Ard Biesheuvel wrote:
> In preparation of switching x86 to use place-relative references for
> the code, target and key members of struct jump_entry, replace direct
> references to the struct members with invocations of the new accessors.
> This will allow us to make the switch by modifying the accessors only.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

That just makes horrible code worse.. can't we do something like so
instead?


--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -42,52 +42,37 @@ static void __jump_label_transform(struc
 				   void *(*poker)(void *, const void *, size_t),
 				   int init)
 {
-	union jump_code_union code;
+	union jump_code_union jmp = {
+		{ .jump = 0xe9,
+		  .offset = jump_entry_target(entry) -
+			    (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE), }
+	};
 	const unsigned char default_nop[] = { STATIC_KEY_INIT_NOP };
 	const unsigned char *ideal_nop = ideal_nops[NOP_ATOMIC5];
+	const unsigned char *expect, *code;
+	int line;
 
 	if (type == JUMP_LABEL_JMP) {
 		if (init) {
-			/*
-			 * Jump label is enabled for the first time.
-			 * So we expect a default_nop...
-			 */
-			if (unlikely(memcmp((void *)entry->code, default_nop, 5)
-				     != 0))
-				bug_at((void *)entry->code, __LINE__);
+			expect = default_nop; line = __LINE__;
 		} else {
-			/*
-			 * ...otherwise expect an ideal_nop. Otherwise
-			 * something went horribly wrong.
-			 */
-			if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
-				     != 0))
-				bug_at((void *)entry->code, __LINE__);
+			expect = ideal_nop; line = __LINE__;
 		}
 
-		code.jump = 0xe9;
-		code.offset = entry->target -
-				(entry->code + JUMP_LABEL_NOP_SIZE);
+		code = &jmp.code;
 	} else {
-		/*
-		 * We are disabling this jump label. If it is not what
-		 * we think it is, then something must have gone wrong.
-		 * If this is the first initialization call, then we
-		 * are converting the default nop to the ideal nop.
-		 */
 		if (init) {
-			if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
-				bug_at((void *)entry->code, __LINE__);
+			expect = default_nop; line = __LINE__;
 		} else {
-			code.jump = 0xe9;
-			code.offset = entry->target -
-				(entry->code + JUMP_LABEL_NOP_SIZE);
-			if (unlikely(memcmp((void *)entry->code, &code, 5) != 0))
-				bug_at((void *)entry->code, __LINE__);
+			expect = &jmp.code; line = __LINE__;
 		}
-		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
+
+		code = ideal_nop;
 	}
 
+	if (memcmp((void *)jump_entry_code(entry), expect, JUMP_LABEL_NOP_SIZE))
+		bug_at((void *)jump_entry_code(entry), line);
+
 	/*
 	 * Make text_poke_bp() a default fallback poker.
 	 *
@@ -96,11 +81,13 @@ static void __jump_label_transform(struc
 	 * always nop being the 'currently valid' instruction
 	 *
 	 */
-	if (poker)
-		(*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
-	else
-		text_poke_bp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE,
-			     (void *)entry->code + JUMP_LABEL_NOP_SIZE);
+	if (poker) {
+		(*poker)((void *)jump_entry_code(entry), &code, JUMP_LABEL_NOP_SIZE);
+		return;
+	}
+
+	text_poke_bp((void *)jump_entry_code(entry), &code, JUMP_LABEL_NOP_SIZE,
+		     (void *)jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
 }
 
 void arch_jump_label_transform(struct jump_entry *entry,

[PATCH 4/5] x86: jump_label: switch to jump_entry accessors

[Ard Biesheuvel]

On 28 June 2018 at 11:11, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Jun 27, 2018 at 06:06:03PM +0200, Ard Biesheuvel wrote:
>> In preparation of switching x86 to use place-relative references for
>> the code, target and key members of struct jump_entry, replace direct
>> references to the struct members with invocations of the new accessors.
>> This will allow us to make the switch by modifying the accessors only.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> That just makes horrible code worse.. can't we do something like so
> instead?
>

Sure, I will incorporate that for v2.

>
> --- a/arch/x86/kernel/jump_label.c
> +++ b/arch/x86/kernel/jump_label.c
> @@ -42,52 +42,37 @@ static void __jump_label_transform(struc
>                                    void *(*poker)(void *, const void *, size_t),
>                                    int init)
>  {
> -       union jump_code_union code;
> +       union jump_code_union jmp = {
> +               { .jump = 0xe9,
> +                 .offset = jump_entry_target(entry) -
> +                           (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE), }
> +       };
>         const unsigned char default_nop[] = { STATIC_KEY_INIT_NOP };
>         const unsigned char *ideal_nop = ideal_nops[NOP_ATOMIC5];
> +       const unsigned char *expect, *code;
> +       int line;
>
>         if (type == JUMP_LABEL_JMP) {
>                 if (init) {
> -                       /*
> -                        * Jump label is enabled for the first time.
> -                        * So we expect a default_nop...
> -                        */
> -                       if (unlikely(memcmp((void *)entry->code, default_nop, 5)
> -                                    != 0))
> -                               bug_at((void *)entry->code, __LINE__);
> +                       expect = default_nop; line = __LINE__;
>                 } else {
> -                       /*
> -                        * ...otherwise expect an ideal_nop. Otherwise
> -                        * something went horribly wrong.
> -                        */
> -                       if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
> -                                    != 0))
> -                               bug_at((void *)entry->code, __LINE__);
> +                       expect = ideal_nop; line = __LINE__;
>                 }
>
> -               code.jump = 0xe9;
> -               code.offset = entry->target -
> -                               (entry->code + JUMP_LABEL_NOP_SIZE);
> +               code = &jmp.code;
>         } else {
> -               /*
> -                * We are disabling this jump label. If it is not what
> -                * we think it is, then something must have gone wrong.
> -                * If this is the first initialization call, then we
> -                * are converting the default nop to the ideal nop.
> -                */
>                 if (init) {
> -                       if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
> -                               bug_at((void *)entry->code, __LINE__);
> +                       expect = default_nop; line = __LINE__;
>                 } else {
> -                       code.jump = 0xe9;
> -                       code.offset = entry->target -
> -                               (entry->code + JUMP_LABEL_NOP_SIZE);
> -                       if (unlikely(memcmp((void *)entry->code, &code, 5) != 0))
> -                               bug_at((void *)entry->code, __LINE__);
> +                       expect = &jmp.code; line = __LINE__;
>                 }
> -               memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
> +
> +               code = ideal_nop;
>         }
>
> +       if (memcmp((void *)jump_entry_code(entry), expect, JUMP_LABEL_NOP_SIZE))
> +               bug_at((void *)jump_entry_code(entry), line);
> +
>         /*
>          * Make text_poke_bp() a default fallback poker.
>          *
> @@ -96,11 +81,13 @@ static void __jump_label_transform(struc
>          * always nop being the 'currently valid' instruction
>          *
>          */
> -       if (poker)
> -               (*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
> -       else
> -               text_poke_bp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE,
> -                            (void *)entry->code + JUMP_LABEL_NOP_SIZE);
> +       if (poker) {
> +               (*poker)((void *)jump_entry_code(entry), &code, JUMP_LABEL_NOP_SIZE);
> +               return;
> +       }
> +
> +       text_poke_bp((void *)jump_entry_code(entry), &code, JUMP_LABEL_NOP_SIZE,
> +                    (void *)jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
>  }
>
>  void arch_jump_label_transform(struct jump_entry *entry,

[PATCH 5/5] x86/kernel: jump_table: use relative references

[Ard Biesheuvel]

Similar to the arm64 case, 64-bit x86 can benefit from using 32-bit
relative references rather than 64-bit absolute ones when emitting
struct jump_entry instances. Not only does this reduce the memory
footprint of the entries themselves by 50%, it also removes the need
for carrying relocation metadata on relocatable builds (i.e., for KASLR)
which saves a fair chunk of .init space as well (although the savings
are not as dramatic as on arm64)

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/Kconfig                  |  1 +
 arch/x86/include/asm/jump_label.h | 28 ++++++--------------
 tools/objtool/special.c           |  4 +--
 3 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e10a3542db7e..dd71258ec1cc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -118,6 +118,7 @@ config X86
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_HUGE_VMAP		if X86_64 || X86_PAE
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_ARCH_JUMP_LABEL_RELATIVE
 	select HAVE_ARCH_KASAN			if X86_64
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_MMAP_RND_BITS		if MMU
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 8c0de4282659..d0f1f25b41d5 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -36,8 +36,8 @@ static __always_inline bool arch_static_branch(struct static_key *key, bool bran
 	asm_volatile_goto("1:"
 		".byte " __stringify(STATIC_KEY_INIT_NOP) "\n\t"
 		".pushsection __jump_table,  \"aw\" \n\t"
-		_ASM_ALIGN "\n\t"
-		_ASM_PTR "1b, %l[l_yes], %c0 + %c1 \n\t"
+		".balign 4\n\t"
+		".long 1b - ., %l[l_yes] - ., %c0 + %c1 - .\n\t"
 		".popsection \n\t"
 		: :  "i" (key), "i" (branch) : : l_yes);
 
@@ -52,8 +52,8 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
 		".byte 0xe9\n\t .long %l[l_yes] - 2f\n\t"
 		"2:\n\t"
 		".pushsection __jump_table,  \"aw\" \n\t"
-		_ASM_ALIGN "\n\t"
-		_ASM_PTR "1b, %l[l_yes], %c0 + %c1 \n\t"
+		".balign 4\n\t"
+		".long 1b - ., %l[l_yes] - ., %c0 + %c1 - .\n\t"
 		".popsection \n\t"
 		: :  "i" (key), "i" (branch) : : l_yes);
 
@@ -62,18 +62,6 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
 	return true;
 }
 
-#ifdef CONFIG_X86_64
-typedef u64 jump_label_t;
-#else
-typedef u32 jump_label_t;
-#endif
-
-struct jump_entry {
-	jump_label_t code;
-	jump_label_t target;
-	jump_label_t key;
-};
-
 #else	/* __ASSEMBLY__ */
 
 .macro STATIC_JUMP_IF_TRUE target, key, def
@@ -87,8 +75,8 @@ struct jump_entry {
 	.byte		STATIC_KEY_INIT_NOP
 	.endif
 	.pushsection __jump_table, "aw"
-	_ASM_ALIGN
-	_ASM_PTR	.Lstatic_jump_\@, \target, \key
+	.balign		4
+	.long		.Lstatic_jump_\@ - ., \target - ., \key - .
 	.popsection
 .endm
 
@@ -103,8 +91,8 @@ struct jump_entry {
 .Lstatic_jump_after_\@:
 	.endif
 	.pushsection __jump_table, "aw"
-	_ASM_ALIGN
-	_ASM_PTR	.Lstatic_jump_\@, \target, \key + 1
+	.balign		4
+	.long		.Lstatic_jump_\@ - ., \target - ., \key + 1 - .
 	.popsection
 .endm
 
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index 84f001d52322..98ae55b39037 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -30,9 +30,9 @@
 #define EX_ORIG_OFFSET		0
 #define EX_NEW_OFFSET		4
 
-#define JUMP_ENTRY_SIZE		24
+#define JUMP_ENTRY_SIZE		12
 #define JUMP_ORIG_OFFSET	0
-#define JUMP_NEW_OFFSET		8
+#define JUMP_NEW_OFFSET		4
 
 #define ALT_ENTRY_SIZE		13
 #define ALT_ORIG_OFFSET		0
-- 
2.11.0

[PATCH 5/5] x86/kernel: jump_table: use relative references

[Peter Zijlstra]

On Wed, Jun 27, 2018 at 06:06:04PM +0200, Ard Biesheuvel wrote:
> Similar to the arm64 case, 64-bit x86 can benefit from using 32-bit
> relative references rather than 64-bit absolute ones when emitting
> struct jump_entry instances. Not only does this reduce the memory
> footprint of the entries themselves by 50%, it also removes the need
> for carrying relocation metadata on relocatable builds (i.e., for KASLR)
> which saves a fair chunk of .init space as well (although the savings
> are not as dramatic as on arm64)

This will conflict with:

  https://lkml.kernel.org/r/20180622172212.199633-10-namit at vmware.com

[PATCH 5/5] x86/kernel: jump_table: use relative references

[Ard Biesheuvel]

On 28 June 2018 at 10:31, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Jun 27, 2018 at 06:06:04PM +0200, Ard Biesheuvel wrote:
>> Similar to the arm64 case, 64-bit x86 can benefit from using 32-bit
>> relative references rather than 64-bit absolute ones when emitting
>> struct jump_entry instances. Not only does this reduce the memory
>> footprint of the entries themselves by 50%, it also removes the need
>> for carrying relocation metadata on relocatable builds (i.e., for KASLR)
>> which saves a fair chunk of .init space as well (although the savings
>> are not as dramatic as on arm64)
>
> This will conflict with:
>
>   https://lkml.kernel.org/r/20180622172212.199633-10-namit at vmware.com

Thanks for the head's up. Fortunately, it does not conflict
fundamentally, so it should be a straight-forward rebase after that
code is merged.

Do you think this is likely to get merged for v4.19?

[PATCH 5/5] x86/kernel: jump_table: use relative references

[Peter Zijlstra]

On Thu, Jun 28, 2018 at 10:34:54AM +0200, Ard Biesheuvel wrote:
> On 28 June 2018 at 10:31, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Wed, Jun 27, 2018 at 06:06:04PM +0200, Ard Biesheuvel wrote:
> >> Similar to the arm64 case, 64-bit x86 can benefit from using 32-bit
> >> relative references rather than 64-bit absolute ones when emitting
> >> struct jump_entry instances. Not only does this reduce the memory
> >> footprint of the entries themselves by 50%, it also removes the need
> >> for carrying relocation metadata on relocatable builds (i.e., for KASLR)
> >> which saves a fair chunk of .init space as well (although the savings
> >> are not as dramatic as on arm64)
> >
> > This will conflict with:
> >
> >   https://lkml.kernel.org/r/20180622172212.199633-10-namit at vmware.com
> 
> Thanks for the head's up. Fortunately, it does not conflict
> fundamentally, so it should be a straight-forward rebase after that
> code is merged.

Yeah, shouldn't be hard to cure.

There's another patch set that might have a little conflict, but that's
not near ready I think, so that'll have to just cope with things
shifting underneath (and there too, the fixup shouldn't be hard).

> Do you think this is likely to get merged for v4.19?

I'm thinking it is near ready so it might, but I'm not in charge of
those bits :-)