x86/asm: Compile-time asm code validation

The previous version of this patch set was named "Compile-time stack frame pointer validation". I changed the subject from "frame pointer validation" to "asm code validation" because the focus of the patch set has changed to be less frame pointer-focused and more asm-focused. I also renamed the tool to asmvalidate (it was previously called stackvalidate) and basically rewrote most of the code.

The goal of asm validation is to enforce sane rules on asm code: all callable asm functions must be self-contained and properly annotated.

Some of the benefits are:

- Frame pointers are more reliable.

- DWARF CFI metadata can be autogenerated (coming soon).

- The asm code becomes less like spaghetti, more like C, and easier to comprehend.

The asmvalidate tool runs on every compiled .S file, and enforces the following rules:

1. Each callable function must be annotated with the ELF STT_FUNC type. This is typically done using the existing ENTRY/ENDPROC macros. If asmvalidate finds a return instruction outside of a function, it flags an error, since that usually indicates callable code which should be annotated accordingly.

2. Each callable function must never leave its own bounds (i.e. with a jump to outside the function) except when returning.

3. Each callable non-leaf function must have frame pointer logic (if required by CONFIG_FRAME_POINTER or the architecture's back chain rules). This should by done by the FP_SAVE/FP_RESTORE macros.

It currently only supports x86_64, but the code is generic and designed for it to be easy to plug in support for other architectures.

There are still a lot of outstanding warnings (which I'll paste as a reply to this email). Once those are all cleaned up, we can change the warnings to build errors and change the default to CONFIG_ASM_VALIDATION=y so the asm code stays clean.

The first patch adds some frame pointer macros. The second patch adds asmvalidate support. The rest of the patches have fixes for (some of) the reported warnings.

These patches are based on tip/master.

[1] http://lkml.kernel.org/r/[email protected]

v5:
- stackvalidate -> asmvalidate
- frame pointers only required for non-leaf functions
- check for the use of the FP_SAVE/RESTORE macros instead of manually analyzing code to detect frame pointer usage
- additional checks to ensure each function doesn't leave its boundaries
- make the macros simpler and more flexible
- support for analyzing ALTERNATIVE macros
- simplified the arch interfaces in scripts/asmvalidate/arch.h
- fixed some asmvalidate warnings
- rebased onto latest tip asm cleanups
- many more small changes

v4:
- Changed the default to CONFIG_STACK_VALIDATION=n, until all the asm code can get cleaned up.
- Fixed a stackvalidate error path exit code issue found by Michal Marek.

v3:
- Added a patch to make the push/pop CFI macros arch-independent, as suggested by H. Peter Anvin

v2:
- Fixed memory leaks reported by Petr Mladek

Josh Poimboeuf (10):
x86/asm: Add FP_SAVE/RESTORE frame pointer macros
x86: Compile-time asm code validation
x86/asm/entry: Fix asmvalidate warnings for entry_64_compat.S x86/asm/crypto: Fix asmvalidate warnings for aesni-intel_asm.S x86/asm/crypto: Fix asmvalidate warnings for ghash-clmulni-intel_asm.S x86/asm/efi: Fix asmvalidate warnings for efi_stub_64.S x86/asm/acpi: Fix asmvalidate warnings for wakeup_64.S x86/asm/head: Fix asmvalidate warnings for head_64.S x86/asm/lib: Fix asmvalidate warnings for lib functions x86/asm/lib: Fix asmvalidate warnings for rwsem.S

 MAINTAINERS                               |   6 +
 arch/Kconfig                              |   3 +
 arch/x86/Kconfig                          |   1 +
 arch/x86/Makefile                         |   6 +-
 arch/x86/crypto/aesni-intel_asm.S         |  19 ++
 arch/x86/crypto/ghash-clmulni-intel_asm.S |   5 +
 arch/x86/entry/entry_64_compat.S          |  35 +--
 arch/x86/include/asm/func.h               |  62 ++++++
 arch/x86/kernel/acpi/wakeup_64.S          |  13 +-
 arch/x86/kernel/head_64.S                 |   7 +-
 arch/x86/lib/clear_page_64.S              |   9 +-
 arch/x86/lib/copy_page_64.S               |   5 +-
 arch/x86/lib/memcpy_64.S                  |  10 +-
 arch/x86/lib/memset_64.S                  |  10 +-
 arch/x86/lib/rwsem.S                      |  11 +-
 arch/x86/platform/efi/efi_stub_64.S       |   3 +
 lib/Kconfig.debug                         |  21 ++
 scripts/Makefile                          |   1 +
 scripts/Makefile.build                    |  23 +-
 scripts/asmvalidate/Makefile              |  17 ++
 scripts/asmvalidate/arch-x86.c            | 283 ++++++++++++++++++++++++
 scripts/asmvalidate/arch.h                |  40 ++++
 scripts/asmvalidate/asmvalidate.c         | 324 +++++++++++++++++++++++++++
 scripts/asmvalidate/elf.c                 | 354 ++++++++++++++++++++++++++++++
 scripts/asmvalidate/elf.h                 |  74 +++++++
 scripts/asmvalidate/list.h                | 217 ++++++++++++++++++
 26 files changed, 1509 insertions(+), 50 deletions(-)
 create mode 100644 arch/x86/include/asm/func.h
 create mode 100644 scripts/asmvalidate/Makefile
 create mode 100644 scripts/asmvalidate/arch-x86.c
 create mode 100644 scripts/asmvalidate/arch.h
 create mode 100644 scripts/asmvalidate/asmvalidate.c
 create mode 100644 scripts/asmvalidate/elf.c
 create mode 100644 scripts/asmvalidate/elf.h
 create mode 100644 scripts/asmvalidate/list.h

2.1.0

Add the FP_SAVE and FP_RESTORE asm macros, which can be used to save and restore the frame pointer.

---
 arch/x86/include/asm/func.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 arch/x86/include/asm/func.h

new file mode 100644
index 0000000..4d62782
@@ -0,0 +1,24 @@
+#ifndef _ASM_X86_FUNC_H
+#define _ASM_X86_FUNC_H
+
+#include <linux/linkage.h>
+#include <asm/asm.h>
+
+/*
+ * These are frame pointer save and restore macros.  They should be used by
+ * every callable non-leaf asm function.
+ */
+.macro FP_SAVE name
+	.if CONFIG_FRAME_POINTER
+		push %_ASM_BP
+		_ASM_MOV %_ASM_SP, %_ASM_BP
+	.endif
+.endm
+
+.macro FP_RESTORE name
+	.if CONFIG_FRAME_POINTER
+		pop %_ASM_BP
+	.endif
+.endm
+
+#endif /* _ASM_X86_FUNC_H */

2.1.0

Add a changelog, which can be used to tell what changed.

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Fair enough :-)

These are gnu assembler macros. This file is only included by .S files.

Josh

We have quite a few of .h headers in the kernel already which are supposed to be included only by .S files, so what exactly is the problem you are seeing here?

Jiri Kosina
SUSE Labs

Such as...? Can this be merged into one of them so that we don't have a separate file? For example "frame.h"?

(I thought asm includes traditionally had different extension, but I may be mistaken).

And at the very least, dwarf2.h includes

#ifndef __ASSEMBLY__
#warning "asm/dwarf2.h should be only included in pure assembly files" #endif

and guards stuff that would not compile in C with

#ifdef __ASSEMBLY__
....

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

arch/x86/include/asm/calling.h

is seems to be the only one (on x86 arch) which doesn't have the __ASSEMBLY__ guard.

Jiri Kosina
SUSE Labs

Ok, I'll move the FP_SAVE/RESTORE stuff into frame.h (which seems to be completely unused). And I'll make sure to use "#ifdef __ASSEMBLY__".

Josh

Add a new CONFIG_ASM_VALIDATION option which adds an asmvalidate host tool which runs on every compiled .S file. Its goal is to enforce sane rules on all asm code, so that stack debug metadata (frame/back chain pointers and/or DWARF CFI metadata) can be made reliable.

It enforces the following rules:

1. Each callable function must be annotated with the ELF STT_FUNC type. This is typically done using the ENTRY/ENDPROC macros. If asmvalidate finds a return instruction outside of a function, it flags an error, since that usually indicates callable code which should be annotated accordingly.

2. Each callable function must never leave its own bounds (i.e. with a jump to outside the function) except when returning.

3. Each callable non-leaf function must have frame pointer logic (if required by CONFIG_FRAME_POINTER or the architecture's back chain rules). This can be done with the new FP_SAVE/FP_RESTORE macros.

It currently only supports x86_64. It *almost* supports x86_32, but the stackvalidate code doesn't yet know how to deal with 32-bit REL relocations for the return whitelists. I tried to make the code generic so that support for other architectures can be plugged in pretty easily.

As a first step, CONFIG_ASM_VALIDATION is disabled by default, and all reported non-compliances result in warnings. Once we get them all cleaned up, we can change the default to CONFIG_ASM_VALIDATION=y and change the warnings to build errors so the asm code can stay clean.

---
 MAINTAINERS                       |   6 +
 arch/Kconfig                      |   3 +
 arch/x86/Kconfig                  |   1 +
 arch/x86/Makefile                 |   6 +-
 arch/x86/include/asm/func.h       |  32 ++++
 lib/Kconfig.debug                 |  21 +++
 scripts/Makefile                  |   1 +
 scripts/Makefile.build            |  23 ++-
 scripts/asmvalidate/Makefile      |  17 ++
 scripts/asmvalidate/arch-x86.c    | 283 ++++++++++++++++++++++++++++++
 scripts/asmvalidate/arch.h        |  40 +++++
 scripts/asmvalidate/asmvalidate.c | 324 ++++++++++++++++++++++++++++++++++
 scripts/asmvalidate/elf.c         | 354 ++++++++++++++++++++++++++++++++++++++
 scripts/asmvalidate/elf.h         |  74 ++++++++
 scripts/asmvalidate/list.h        | 217 +++++++++++++++++++++++
 15 files changed, 1399 insertions(+), 3 deletions(-)
 create mode 100644 scripts/asmvalidate/Makefile
 create mode 100644 scripts/asmvalidate/arch-x86.c
 create mode 100644 scripts/asmvalidate/arch.h
 create mode 100644 scripts/asmvalidate/asmvalidate.c
 create mode 100644 scripts/asmvalidate/elf.c
 create mode 100644 scripts/asmvalidate/elf.h
 create mode 100644 scripts/asmvalidate/list.h

index 469cd4d..831bf8b 100644
@@ -1664,6 +1664,12 @@ S:	Maintained
 F:	Documentation/hwmon/asc7621
 F:	drivers/hwmon/asc7621.c
 
+ASM VALIDATION
+M:	Josh Poimboeuf <[email protected]>
+S:	Supported
+F:	scripts/asmvalidate/
+F:	arch/x86/include/asm/func.h
+
 ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
 M:	Corentin Chary <[email protected]>
 L:	[email protected]

index a65eafb..8d85326 100644
@@ -499,6 +499,9 @@ config ARCH_HAS_ELF_RANDOMIZE
 	  - arch_mmap_rnd()
 	  - arch_randomize_brk()
 
+config HAVE_ASM_VALIDATION
+	bool
+
 #
 # ABI hall of shame
 #

index 228aa35..a85e149 100644
@@ -144,6 +144,7 @@ config X86
 	select VIRT_TO_BUS
 	select X86_DEV_DMA_OPS			if X86_64
 	select X86_FEATURE_NAMES		if PROC_FS
+	select HAVE_ASM_VALIDATION		if X86_64
 
 config INSTRUCTION_DECODER
 	def_bool y

index 118e6de..e2dd40e 100644
@@ -174,9 +174,13 @@ KBUILD_CFLAGS += $(call cc-option,-mno-avx,)
 KBUILD_CFLAGS += $(mflags-y)
 KBUILD_AFLAGS += $(mflags-y)
 
-archscripts: scripts_basic
+archscripts: scripts_basic $(objtree)/arch/x86/lib/inat-tables.c
 	$(Q)$(MAKE) $(build)=arch/x86/tools relocs
 
+# this file is needed early by scripts/asmvalidate
+$(objtree)/arch/x86/lib/inat-tables.c:
+	$(Q)$(MAKE) $(build)=arch/x86/lib $@
+
 ###
 # Syscall table generation
 

index 4d62782..52b3225 100644
@@ -9,6 +9,14 @@
  * every callable non-leaf asm function.
  */
 .macro FP_SAVE name
+	.if CONFIG_ASM_VALIDATION
+		163:
+		.pushsection __asmvalidate_fp_save, "ae"
+		_ASM_ALIGN
+		.long 163b - .
+		.popsection
+	.endif
+
 	.if CONFIG_FRAME_POINTER
 		push %_ASM_BP
 		_ASM_MOV %_ASM_SP, %_ASM_BP

@@ -19,6 +27,30 @@
 	.if CONFIG_FRAME_POINTER
 		pop %_ASM_BP
 	.endif
+
+	.if CONFIG_ASM_VALIDATION
+		163:
+		.pushsection __asmvalidate_fp_restore, "ae"
+		_ASM_ALIGN
+		.long 163b - .
+		.popsection
+	.endif
+.endm
+
+/*
+ * This macro tells the asm validation script to ignore the instruction
+ * immediately after the macro.  It should only be used in special cases where
+ * you're 100% sure that the asm validation constraints don't need to be
+ * adhered to.  Use with caution!
+ */
+.macro ASMVALIDATE_IGNORE
+	.if CONFIG_ASM_VALIDATION
+		163:
+		.pushsection __asmvalidate_ignore, "ae"
+		_ASM_ALIGN
+		.long 163b - .
+		.popsection
+	.endif
 .endm
 
 #endif /* _ASM_X86_FUNC_H */

index b908048..119dfd1 100644
@@ -332,6 +332,27 @@ config FRAME_POINTER
 	  larger and slower, but it gives very useful debugging information
 	  in case of kernel bugs. (precise oopses/stacktraces/warnings)
 
+
+config ASM_VALIDATION
+	bool "Enable compile-time asm code validation"
+	depends on HAVE_ASM_VALIDATION
+	default n
+	help
+	  Add compile-time checks to enforce sane rules on all asm code, so
+	  that stack debug metadata can be more reliable.  Enforces the
+	  following rules:
+
+	  1. Each callable function must be annotated with the ELF STT_FUNC
+	     type.
+
+	  2. Each callable function must never leave its own bounds except when
+	     returning.
+
+	  3. Each callable non-leaf function must have frame pointer logic (if
+	     required by CONFIG_FRAME_POINTER or the architecture's back chain
+	     rules.
+
+
 config DEBUG_FORCE_WEAK_PER_CPU
 	bool "Force weak per-cpu definitions"
 	depends on DEBUG_KERNEL

index 2016a64..c4c8350 100644
@@ -37,6 +37,7 @@ subdir-y                     += mod
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
 subdir-$(CONFIG_DTC)         += dtc
 subdir-$(CONFIG_GDB_SCRIPTS) += gdb
+subdir-$(CONFIG_ASM_VALIDATION) += asmvalidate
 
 # Let clean descend into subdirs
 subdir-	+= basic kconfig package

index 01df30a..8bf1085 100644
@@ -253,6 +253,25 @@ define rule_cc_o_c
 	mv -f $(dot-target).tmp $(dot-target).cmd
 endef
 
+ifdef CONFIG_ASM_VALIDATION
+asmvalidate = $(objtree)/scripts/asmvalidate/asmvalidate
+cmd_asmvalidate =							  \
+	case $(@) in							  \
+		arch/x86/purgatory/*) ;;				  \
+		arch/x86/realmode/rm/*) ;;				  \
+		*) $(asmvalidate) "$(@)"; \
+	esac;
+endif
+
+define rule_as_o_S
+	$(call echo-cmd,as_o_S) $(cmd_as_o_S);				  \
+	$(cmd_asmvalidate)						  \
+	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,as_o_S)' >    \
+	                                              $(dot-target).tmp;  \
+	rm -f $(depfile);						  \
+	mv -f $(dot-target).tmp $(dot-target).cmd
+endef
+
 # Built-in and composite module parts
 $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call cmd,force_checksrc)

@@ -290,8 +309,8 @@ $(obj)/%.s: $(src)/%.S FORCE
 quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
 cmd_as_o_S       = $(CC) $(a_flags) -c -o $@ $<
 
-$(obj)/%.o: $(src)/%.S FORCE
-	$(call if_changed_dep,as_o_S)
+$(obj)/%.o: $(src)/%.S $(asmvalidate) FORCE
+	$(call if_changed_rule,as_o_S)
 
 targets += $(real-objs-y) $(real-objs-m) $(lib-y)
 targets += $(extra-y) $(MAKECMDGOALS) $(always)

new file mode 100644
index 0000000..a202ad6
@@ -0,0 +1,17 @@
+hostprogs-y := asmvalidate
+always := $(hostprogs-y)
+
+asmvalidate-objs := asmvalidate.o elf.o
+
+HOSTCFLAGS += -Werror
+HOSTLOADLIBES_asmvalidate := -lelf
+
+ifdef CONFIG_X86
+
+asmvalidate-objs += arch-x86.o
+
+HOSTCFLAGS_arch-x86.o := -I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include/ -I$(srctree)/arch/x86/lib/
+
+$(obj)/arch-x86.o: $(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c $(srctree)/arch/x86/include/asm/inat_types.h $(srctree)/arch/x86/include/asm/inat.h $(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c
+
+endif

new file mode 100644
index 0000000..da85a18
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2015 Josh Poimboeuf <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _ARCH_H_
+#define _ARCH_H_
+
+#include <stdbool.h>
+#include "elf.h"
+
+struct arch_insn {
+	bool call, ret, jump;
+	unsigned int len;
+
+	/* jump-specific variables */
+	bool unconditional;
+	unsigned long dest;
+};
+
+/* decode an instruction and populate the arch_insn accordingly */
+int arch_insn_decode(struct elf *elf, struct arch_insn *arch_insn,
+		     unsigned long addr, unsigned int maxlen);
+
+/* validate any .altinstructions sections */
+int arch_validate_alternatives(struct elf *elf);
+
+#endif /* _ARCH_H_ */

2.1.0

Won't that break with sibling/tail calls? GCC can generate those, and the ia32_ptregs_common label is an example of such a thing.

I'd rather have the script understand tail calls and possibly require that ia32_ptregs_common have a dummy frame pointer save in front before the label if needed.

--Andy

Yes, asmvalidate will flag a warning for tail calls.

Why do you prefer tail calls there? See patch 3 for how I handled that for ia32_ptregs_common (I duplicated the code with macros).

I think adding support for tail calls in the tooling would be tricky. So I'm just trying to figure out if there's a good reason to keep them.

Josh

To save code size by deduplicating common tails. The code currently does that, and it would be nice to avoid bloating the code to keep the validator happy.

I imagine that an automatic CFI annotation adder would walk through functions one instruction at a time and keep track of the frame state. If so, then it could verify that common jump targets had identical state and continue walking through them and annotating. I think this would get this case right, and it might be necessary anyway to handle jumps within functions.

--Andy

Well, I wonder whether it's really worth sacrificing code readability and consistency, and maybe some improved i-cache locality, to save a few hundred bytes of code size.

This would definitely add complexity to both asmvalidate and the CFI generator. In fact it sounds like it would push the CFI generator out of its current awk script territory and more into complex C code territory.

Josh

I should also mention that my proposed ia32_ptregs_common patch, which duplicated the needed code, was more optimized for performance than code size.

But if you're more worried about code size, we could turn ia32_ptregs_common into a proper callable function, and then replace

jmp ia32_ptregs_common

with:

call ia32_ptregs_common
ret

So it becomes a regular call instead of a tail call. It only adds a few instructions and the function is self-contained. Would that be good enough?

Josh

No, debug info should not slow down the kernel, especially not code we write in assembly partly for performance.

Thanks,

Ingo

Ok. I'll work on adding support for external jumps.

Josh

I'd count that as a plus: awk isn't a common skillset while C is, and properly written it doesn't have to be _that_ complex.

Thanks,

Ingo

The thing is, C is quite painful for text processing. And I think we'd have to do the analysis at the source text level in order to generate the .cfi_* instructions to pass to the gnu assembler.

C would definitely make more sense when analyzing object code. In fact, asmvalidate is written in C. But then I guess we'd have to re-implement the .cfi stuff and populate the DWARF sections manually instead of letting the assembler do it.

Josh

Was doing all this directly in the assembler considered? That is, e.g., add some knob that makes it error/warn in the same conditions you're making the validator catch. For tail calls, you'd e.g., add some new ".nonlocal" directive that you'd use to whitelist the following jump. And then if it's possible run a CFI generator as a separate step over the source, it sounds like it should also be possible to have the assembler do it instead too (again with some new high level directive to trigger/help it).

Thanks,
Pedro Alves

In general I think doing these types of things in the assembler would be a good idea. Missing or inaccurate debug data for asm code seems to be a common problem for other projects as well. As Andy pointed out, they're doing similar things in musl [1]. So it might be useful to add an option to the assembler which validates that the code conforms to certain structural rules, and then inserts frame pointer and/or .cfi directives.

That said, the kernel has much more custom features than other projects. There are some sneaky macros, like _ASM_EXTABLE and ALTERNATIVE, which hide code in various sections. Unless we're able to somehow either stop using these macros or isolate them to a few places, I doubt that such a general purpose assembler option would work.

[1] http://www.openwall.com/lists/musl/2015/05/31/5

Josh

Thanks for the pointer.

How does the asmvalidator handle these?

Thanks,
Pedro Alves

They're not easy to deal with...

The ALTERNATIVE macro creates some instructions which can be patched in at runtime, to replace some original instructions, if the CPU supports certain features. So we have to look up those replacement instructions in another section and consider them to be potentially part of the original function when doing the analysis and generation.

The _ASM_EXTABLE macro creates code which is executed after an exception. Similarly to the ALTERNATIVE macro, we have to look up those instructions and consider them to be part of the original function.

Josh

It only validates hand-written assembly, so how it could?

Vojtech Pavlik
Director SUSE Labs

It's a hand-written tail call.

--Andy

Fix the following asmvalidate warnings:

asmvalidate: arch/x86/entry/entry_64_compat.o: native_usergs_sysret32(): unsupported fallthrough at end of function
asmvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0xcf: unsupported jump to outside of function
asmvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x113: unsupported jump to outside of function
asmvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat()+0x16d: unsupported jump to outside of function
asmvalidate: arch/x86/entry/entry_64_compat.o: entry_SYSENTER_compat(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/entry/entry_64_compat.o: .entry.text+0x56e: return instruction outside of a function

1. native_usergs_sysret32 is redirected to from a jump rather than a call, so it shouldn't be annotated as a function. Change ENDPROC -> END accordingly.

2. Ditto for entry_SYSENTER_compat.

3. The stub functions can be called, so annotate them as functions with ENTRY/ENDPROC.

4. The stub functions aren't leaf functions, so save/restore the frame pointer with FP_SAVE/RESTORE.

5. The stub functions all jump outside of their respective functions' boundaries to the ia32_ptregs_common label. Change them to be self-contained so they stay within their boundaries.

---
 arch/x86/entry/entry_64_compat.S | 35 +++++++++++++++++++----------------
 arch/x86/include/asm/func.h      |  6 ++++++
 2 files changed, 25 insertions(+), 16 deletions(-)

index bb187a6..07f5ae8 100644
@@ -13,6 +13,7 @@
 #include <asm/irqflags.h>
 #include <asm/asm.h>
 #include <asm/smap.h>
+#include <asm/func.h>
 #include <linux/linkage.h>
 #include <linux/err.h>
 

@@ -32,7 +33,7 @@
 ENTRY(native_usergs_sysret32)
 	swapgs
 	sysretl
-ENDPROC(native_usergs_sysret32)
+END(native_usergs_sysret32)
 #endif
 
 /*

@@ -270,7 +271,7 @@ sysenter_tracesys:
 
 	RESTORE_EXTRA_REGS
 	jmp	sysenter_do_call
-ENDPROC(entry_SYSENTER_compat)
+END(entry_SYSENTER_compat)
 
 /*
  * 32-bit SYSCALL instruction entry.

@@ -523,10 +524,15 @@ ia32_tracesys:
 END(entry_INT80_compat)
 
 	.macro PTREGSCALL label, func
-	ALIGN
-GLOBAL(\label)
-	leaq	\func(%rip), %rax
-	jmp	ia32_ptregs_common
+ENTRY(\label)
+	FP_SAVE
+	leaq	\func(%rip),%rax
+	SAVE_EXTRA_REGS(8+FP_SIZE)
+	call	*%rax
+	RESTORE_EXTRA_REGS(8+FP_SIZE)
+	FP_RESTORE
+	ret
+ENDPROC(\label)
 	.endm
 
 	PTREGSCALL stub32_rt_sigreturn,	sys32_rt_sigreturn

@@ -534,9 +540,9 @@ GLOBAL(\label)
 	PTREGSCALL stub32_fork,		sys_fork
 	PTREGSCALL stub32_vfork,	sys_vfork
 
-	ALIGN
-GLOBAL(stub32_clone)
-	leaq	sys_clone(%rip), %rax
+ENTRY(stub32_clone)
+	FP_SAVE
+	leaq	sys_clone(%rip),%rax
 	/*
 	 * The 32-bit clone ABI is: clone(..., int tls_val, int *child_tidptr).
 	 * The 64-bit clone ABI is: clone(..., int *child_tidptr, int tls_val).

@@ -545,12 +551,9 @@ GLOBAL(stub32_clone)
 	 * so we need to swap arguments here before calling it:
 	 */
 	xchg	%r8, %rcx
-	jmp	ia32_ptregs_common
-
-	ALIGN
-ia32_ptregs_common:
-	SAVE_EXTRA_REGS 8
+	SAVE_EXTRA_REGS(8+FP_SIZE)
 	call	*%rax
-	RESTORE_EXTRA_REGS 8
+	RESTORE_EXTRA_REGS(8+FP_SIZE)
+	FP_RESTORE
 	ret
-END(ia32_ptregs_common)
+ENDPROC(stub32_clone)

index 52b3225..1d923bd 100644
@@ -37,6 +37,12 @@
 	.endif
 .endm
 
+#ifdef CONFIG_FRAME_POINTER
+#define FP_SIZE __ASM_SEL(4, 8)
+#else
+#define FP_SIZE 0
+#endif
+
 /*
  * This macro tells the asm validation script to ignore the instruction
  * immediately after the macro.  It should only be used in special cases where

2.1.0

Fix the following asmvalidate warnings:

asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_set_key(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_enc(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_dec(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ecb_enc(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ecb_dec(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_cbc_enc(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_cbc_dec(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_ctr_enc(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/aesni-intel_asm.o: aesni_xts_crypt8(): missing FP_SAVE/RESTORE macros

These are all non-leaf callable functions, so save/restore the frame pointer with FP_SAVE/RESTORE.

---
 arch/x86/crypto/aesni-intel_asm.S | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

index 6bd2c6c..83465f9a 100644
@@ -31,6 +31,7 @@
 
 #include <linux/linkage.h>
 #include <asm/inst.h>
+#include <asm/func.h>
 
 /*
  * The following macros are used to move an (un)aligned 16 byte value to/from

@@ -1800,6 +1801,7 @@ ENDPROC(_key_expansion_256b)
  *                   unsigned int key_len)
  */
 ENTRY(aesni_set_key)
+	FP_SAVE
 #ifndef __x86_64__
 	pushl KEYP
 	movl 8(%esp), KEYP		# ctx

@@ -1905,6 +1907,7 @@ ENTRY(aesni_set_key)
 #ifndef __x86_64__
 	popl KEYP
 #endif
+	FP_RESTORE
 	ret
 ENDPROC(aesni_set_key)
 

@@ -1912,6 +1915,7 @@ ENDPROC(aesni_set_key)
  * void aesni_enc(struct crypto_aes_ctx *ctx, u8 *dst, const u8 *src)
  */
 ENTRY(aesni_enc)
+	FP_SAVE
 #ifndef __x86_64__
 	pushl KEYP
 	pushl KLEN

@@ -1927,6 +1931,7 @@ ENTRY(aesni_enc)
 	popl KLEN
 	popl KEYP
 #endif
+	FP_RESTORE
 	ret
 ENDPROC(aesni_enc)
 

@@ -2101,6 +2106,7 @@ ENDPROC(_aesni_enc4)
  * void aesni_dec (struct crypto_aes_ctx *ctx, u8 *dst, const u8 *src)
  */
 ENTRY(aesni_dec)
+	FP_SAVE
 #ifndef __x86_64__
 	pushl KEYP
 	pushl KLEN

@@ -2117,6 +2123,7 @@ ENTRY(aesni_dec)
 	popl KLEN
 	popl KEYP
 #endif
+	FP_RESTORE
 	ret
 ENDPROC(aesni_dec)
 

@@ -2292,6 +2299,7 @@ ENDPROC(_aesni_dec4)
  *		      size_t len)
  */
 ENTRY(aesni_ecb_enc)
+	FP_SAVE
 #ifndef __x86_64__
 	pushl LEN
 	pushl KEYP

@@ -2342,6 +2350,7 @@ ENTRY(aesni_ecb_enc)
 	popl KEYP
 	popl LEN
 #endif
+	FP_RESTORE
 	ret
 ENDPROC(aesni_ecb_enc)
 

@@ -2350,6 +2359,7 @@ ENDPROC(aesni_ecb_enc)
  *		      size_t len);
  */
 ENTRY(aesni_ecb_dec)
+	FP_SAVE
 #ifndef __x86_64__
 	pushl LEN
 	pushl KEYP

@@ -2401,6 +2411,7 @@ ENTRY(aesni_ecb_dec)
 	popl KEYP
 	popl LEN
 #endif
+	FP_RESTORE
 	ret
 ENDPROC(aesni_ecb_dec)
 

@@ -2409,6 +2420,7 @@ ENDPROC(aesni_ecb_dec)
  *		      size_t len, u8 *iv)
  */
 ENTRY(aesni_cbc_enc)
+	FP_SAVE
 #ifndef __x86_64__
 	pushl IVP
 	pushl LEN

@@ -2443,6 +2455,7 @@ ENTRY(aesni_cbc_enc)
 	popl LEN
 	popl IVP
 #endif
+	FP_RESTORE
 	ret
 ENDPROC(aesni_cbc_enc)
 

@@ -2451,6 +2464,7 @@ ENDPROC(aesni_cbc_enc)
  *		      size_t len, u8 *iv)
  */
 ENTRY(aesni_cbc_dec)
+	FP_SAVE
 #ifndef __x86_64__
 	pushl IVP
 	pushl LEN

@@ -2534,6 +2548,7 @@ ENTRY(aesni_cbc_dec)
 	popl LEN
 	popl IVP
 #endif
+	FP_RESTORE
 	ret
 ENDPROC(aesni_cbc_dec)
 

@@ -2598,6 +2613,7 @@ ENDPROC(_aesni_inc)
  *		      size_t len, u8 *iv)
  */
 ENTRY(aesni_ctr_enc)
+	FP_SAVE
 	cmp $16, LEN
 	jb .Lctr_enc_just_ret
 	mov 480(KEYP), KLEN

@@ -2651,6 +2667,7 @@ ENTRY(aesni_ctr_enc)
 .Lctr_enc_ret:
 	movups IV, (IVP)
 .Lctr_enc_just_ret:
+	FP_RESTORE
 	ret
 ENDPROC(aesni_ctr_enc)
 

@@ -2677,6 +2694,7 @@ ENDPROC(aesni_ctr_enc)
  *			 bool enc, u8 *iv)
  */
 ENTRY(aesni_xts_crypt8)
+	FP_SAVE
 	cmpb $0, %cl
 	movl $0, %ecx
 	movl $240, %r10d

@@ -2777,6 +2795,7 @@ ENTRY(aesni_xts_crypt8)
 	pxor INC, STATE4
 	movdqu STATE4, 0x70(OUTP)
 
+	FP_RESTORE
 	ret
 ENDPROC(aesni_xts_crypt8)
 

2.1.0

Fix the following asmvalidate warnings:

asmvalidate: arch/x86/crypto/ghash-clmulni-intel_asm.o: clmul_ghash_mul(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/ghash-clmulni-intel_asm.o: clmul_ghash_update(): missing FP_SAVE/RESTORE macros

These are non-leaf callable functions, so save/restore the frame pointer with FP_SAVE/RESTORE.

---
 arch/x86/crypto/ghash-clmulni-intel_asm.S | 5 +++++
 1 file changed, 5 insertions(+)

index 5d1e007..e5b76b1 100644
@@ -18,6 +18,7 @@
 
 #include <linux/linkage.h>
 #include <asm/inst.h>
+#include <asm/func.h>
 
 .data
 

@@ -94,6 +95,7 @@ ENDPROC(__clmul_gf128mul_ble)
 
 /* void clmul_ghash_mul(char *dst, const u128 *shash) */
 ENTRY(clmul_ghash_mul)
+	FP_SAVE
 	movups (%rdi), DATA
 	movups (%rsi), SHASH
 	movaps .Lbswap_mask, BSWAP

@@ -101,6 +103,7 @@ ENTRY(clmul_ghash_mul)
 	call __clmul_gf128mul_ble
 	PSHUFB_XMM BSWAP DATA
 	movups DATA, (%rdi)
+	FP_RESTORE
 	ret
 ENDPROC(clmul_ghash_mul)
 

@@ -109,6 +112,7 @@ ENDPROC(clmul_ghash_mul)
  *			   const u128 *shash);
  */
 ENTRY(clmul_ghash_update)
+	FP_SAVE
 	cmp $16, %rdx
 	jb .Lupdate_just_ret	# check length
 	movaps .Lbswap_mask, BSWAP

@@ -128,5 +132,6 @@ ENTRY(clmul_ghash_update)
 	PSHUFB_XMM BSWAP DATA
 	movups DATA, (%rdi)
 .Lupdate_just_ret:
+	FP_RESTORE
 	ret
 ENDPROC(clmul_ghash_update)

2.1.0

Fix the following asmvalidate warnings:

asmvalidate: arch/x86/platform/efi/efi_stub_64.o: efi_call(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/boot/compressed/efi_stub_64.o: efi_call(): missing FP_SAVE/RESTORE macros

efi_call() is a non-leaf callable function, so save/restore the frame pointer with FP_SAVE/RESTORE.

---
 arch/x86/platform/efi/efi_stub_64.S | 3 +++
 1 file changed, 3 insertions(+)

index 86d0f9e..0ca6bfb 100644
@@ -11,6 +11,7 @@
 #include <asm/msr.h>
 #include <asm/processor-flags.h>
 #include <asm/page_types.h>
+#include <asm/func.h>
 
 #define SAVE_XMM			\
 	mov %rsp, %rax;			\

@@ -74,6 +75,7 @@
 	.endm
 
 ENTRY(efi_call)
+	FP_SAVE
 	SAVE_XMM
 	mov (%rsp), %rax
 	mov 8(%rax), %rax

@@ -88,6 +90,7 @@ ENTRY(efi_call)
 	RESTORE_PGT
 	addq $48, %rsp
 	RESTORE_XMM
+	FP_RESTORE
 	ret
 ENDPROC(efi_call)
 

2.1.0

Yeah, fair enough. Though it's worth noting that because we're calling firmware functions, which use the Microsoft ABI, %rbp will be saved by the callee function if used. Anyway,

And since I know Boris in particular has poked around in this area, an ACK from him would be worth alot.

Matt Fleming, Intel Open Source Technology Center

Yeah, just looked at the spec.

But you know how we don't trust specs. So we get additional paranoid security that callee won't futz with RBP because we save it before calling. But we pay the additional penalty in the CONFIG_FRAME_POINTER case.

Oh what the hell, the 3 additional insns shouldn't be noticeable.

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Fix the following asmvalidate warnings:

asmvalidate: arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64()+0x15: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64()+0x55: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/acpi/wakeup_64.o: wakeup_long64(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel()+0x9a: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel()+0x116: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/acpi/wakeup_64.o: do_suspend_lowlevel(): missing FP_SAVE/RESTORE macros

1. wakeup_long64() isn't a function that can be called. It's actually redirected to via a return instruction in the entry code. It shouldn't be annotated as a callable function. Change ENDPROC -> PROC accordingly.

2. do_suspend_lowlevel() is a non-leaf callable function, so save/restore the frame pointer with FP_SAVE/RESTORE.

3. Remove the unnecessary jump to .Lresume_point, as it just results in jumping to the next instruction (which is a nop because of the align). Otherwise asmvalidate gets confused by the jump.

4. Change the "jmp restore_processor_state" to a call instruction, because jumping outside the function's boundaries isn't allowed. Now restore_processor_state() will return back to do_suspend_lowlevel() instead of do_suspend_lowlevel()'s caller.

5. Remove superfluous rsp changes.

---
 arch/x86/kernel/acpi/wakeup_64.S | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

index 8c35df4..7e442be 100644
@@ -5,6 +5,7 @@
 #include <asm/page_types.h>
 #include <asm/msr.h>
 #include <asm/asm-offsets.h>
+#include <asm/func.h>
 
 # Copyright 2003 Pavel Machek <[email protected]>, distribute under GPLv2
 

@@ -33,13 +34,13 @@ ENTRY(wakeup_long64)
 
 	movq	saved_rip, %rax
 	jmp	*%rax
-ENDPROC(wakeup_long64)
+END(wakeup_long64)
 
 bogus_64_magic:
 	jmp	bogus_64_magic
 
 ENTRY(do_suspend_lowlevel)
-	subq	$8, %rsp
+	FP_SAVE
 	xorl	%eax, %eax
 	call	save_processor_state
 

@@ -70,12 +71,11 @@ ENTRY(do_suspend_lowlevel)
 	movq	%rdi, saved_rdi
 	movq	%rsi, saved_rsi
 
-	addq	$8, %rsp
 	movl	$3, %edi
 	xorl	%eax, %eax
 	call	x86_acpi_enter_sleep_state
+
 	/* in case something went wrong, restore the machine status and go on */
-	jmp	.Lresume_point
 
 	.align 4
 .Lresume_point:

@@ -108,8 +108,9 @@ ENTRY(do_suspend_lowlevel)
 	movq	pt_regs_r15(%rax), %r15
 
 	xorl	%eax, %eax
-	addq	$8, %rsp
-	jmp	restore_processor_state
+	call	restore_processor_state
+	FP_RESTORE
+	ret
 ENDPROC(do_suspend_lowlevel)
 
 .data

2.1.0

Hi!

But I see -> END.

It does not work with the frame pointer itself. Is FP_SAVE/RESTORE still neccessary? Will you need FP_RESTORE to wakeup_long64, then?

It also results in flushing the pipeline. Ok, I guess this one is unneccessary.

Did you test the changes?

Do you plan to make similar changes to wakeup_32.S?

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Oops! It should say -> END.

wakeup_long64 jumps to .Lresume_point, which does the FP_RESTORE.

Yes, I verified that it didn't break suspend/resume on my system.

Currently, asmvalidate is x86_64 only, so I'm only fixing the 64-bit stuff right now.

Hm, I'm looking hard, but I still don't see a problem with that code. It's saving rsp to the saved_context struct. As I mentioned above, it's ok for the wakeup_long64 path to restore the same rsp value, since it jumps to .Lresume_point which has FP_RESTORE.

Why?

Josh

Hi!

Ok, so I can not see anything wrong, either. I'd like to understand why the original code manipulated %rsp, but...

If you did testing with frame pointer on, you can get my

Well, you are "improving debuggability", afaict. It worked well before.

It is both smaller and faster than the new code. But...

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Actually first things first. Purpose of warnings is to pinpoint errors. Do you believe there are some errors in wakeup_64.S?
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

The "errors" are that it doesn't conform with the guidelines outlined in the cover letter. Specifically, wakeup_long64() is improperly annotated, and do_suspend_lowlevel() doesn't honor CONFIG_FRAME_POINTER.

Josh

Please create a file for this in Documentation/x86/, outlining the common cases of such .S debug info problems and the effects this has on the stack backtrace output.

Thanks,

Ingo

Fix the following asmvalidate warnings:

asmvalidate: arch/x86/kernel/head_64.o: start_cpu0(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x4: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xd: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x16: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x1f: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x28: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x31: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x3a: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x43: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x4a: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x55: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x5c: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x65: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x6e: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x77: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x80: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x8b: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x94: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x9b: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xa6: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xaf: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xb8: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xc1: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xca: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xd3: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xdc: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xe5: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xee: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0xf7: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x100: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x109: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x112: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array()+0x11b: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_array(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_common()+0xbb: unsupported jump to outside of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_common(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: early_idt_handler_common(): missing FP_SAVE/RESTORE macros

1. start_cpu0() isn't a normal callable function because it doesn't return to its caller. Change ENDPROC -> END accordingly.

2. early_idt_handler_array() and early_idt_handler_common() aren't callable functions. Change ENDPROC -> END accordingly.

---
 arch/x86/kernel/head_64.S | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

index e5c27f7..8ba22cf 100644
@@ -20,6 +20,7 @@
 #include <asm/processor-flags.h>
 #include <asm/percpu.h>
 #include <asm/nops.h>
+#include <asm/func.h>
 
 #ifdef CONFIG_PARAVIRT
 #include <asm/asm-offsets.h>

@@ -301,7 +302,7 @@ ENTRY(start_cpu0)
 	pushq	$__KERNEL_CS	# set correct cs
 	pushq	%rax		# target address in negative space
 	lretq
-ENDPROC(start_cpu0)
+END(start_cpu0)
 #endif
 
 	/* SMP bootup changes these two */

@@ -336,7 +337,7 @@ ENTRY(early_idt_handler_array)
 	i = i + 1
 	.fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
 	.endr
-ENDPROC(early_idt_handler_array)
+END(early_idt_handler_array)
 
 early_idt_handler_common:
 	/*

@@ -414,7 +415,7 @@ early_idt_handler_common:
 .Lis_nmi:
 	addq $16,%rsp		# drop vector number and error code
 	INTERRUPT_RETURN
-ENDPROC(early_idt_handler_common)
+END(early_idt_handler_common)
 
 	__INITDATA
 

2.1.0

Fix the following asmvalidate warnings:

asmvalidate: arch/x86/lib/clear_page_64.o: clear_page()+0x0: unsupported jump to outside of function
asmvalidate: arch/x86/lib/clear_page_64.o: alternative jump to outside the scope of original function clear_page
asmvalidate: arch/x86/lib/copy_page_64.o: copy_page()+0x0: unsupported jump to outside of function
asmvalidate: arch/x86/lib/memcpy_64.o: memcpy()+0x0: unsupported jump to outside of function
asmvalidate: arch/x86/lib/memcpy_64.o: __memcpy()+0x0: unsupported jump to outside of function
asmvalidate: arch/x86/lib/memcpy_64.o: alternative jump to outside the scope of original function memcpy
asmvalidate: arch/x86/lib/memset_64.o: memset()+0x0: unsupported jump to outside of function
asmvalidate: arch/x86/lib/memset_64.o: __memset()+0x0: unsupported jump to outside of function
asmvalidate: arch/x86/lib/memset_64.o: alternative jump to outside the scope of original function memset

Change the annotations for clear_page(), copy_page(), memcpy(), and memset() so that they don't jump outside of their function boundaries.

---
 arch/x86/lib/clear_page_64.S |  9 +++------
 arch/x86/lib/copy_page_64.S  |  5 ++---
 arch/x86/lib/memcpy_64.S     | 10 ++++------
 arch/x86/lib/memset_64.S     | 10 ++++------
 4 files changed, 13 insertions(+), 21 deletions(-)

index a2fe51b..c342566 100644
@@ -22,10 +22,8 @@ ENTRY(clear_page)
 	xorl %eax,%eax
 	rep stosq
 	ret
-ENDPROC(clear_page)
-
-ENTRY(clear_page_orig)
 
+clear_page_orig:
 	xorl   %eax,%eax
 	movl   $4096/64,%ecx
 	.p2align 4

@@ -44,11 +42,10 @@ ENTRY(clear_page_orig)
 	jnz	.Lloop
 	nop
 	ret
-ENDPROC(clear_page_orig)
 
-ENTRY(clear_page_c_e)
+clear_page_c_e:
 	movl $4096,%ecx
 	xorl %eax,%eax
 	rep stosb
 	ret
-ENDPROC(clear_page_c_e)
+ENDPROC(clear_page)

index 009f982..81d5cba 100644
@@ -16,9 +16,8 @@ ENTRY(copy_page)
 	movl	$4096/8, %ecx
 	rep	movsq
 	ret
-ENDPROC(copy_page)
 
-ENTRY(copy_page_regs)
+copy_page_regs:
 	subq	$2*8,	%rsp
 	movq	%rbx,	(%rsp)
 	movq	%r12,	1*8(%rsp)

@@ -83,4 +82,4 @@ ENTRY(copy_page_regs)
 	movq	1*8(%rsp), %r12
 	addq	$2*8, %rsp
 	ret
-ENDPROC(copy_page_regs)
+ENDPROC(copy_page)

index 16698bb..64d00ec 100644
@@ -37,21 +37,18 @@ ENTRY(memcpy)
 	movl %edx, %ecx
 	rep movsb
 	ret
-ENDPROC(memcpy)
-ENDPROC(__memcpy)
 
 /*
  * memcpy_erms() - enhanced fast string memcpy. This is faster and
  * simpler than memcpy. Use memcpy_erms when possible.
  */
-ENTRY(memcpy_erms)
+memcpy_erms:
 	movq %rdi, %rax
 	movq %rdx, %rcx
 	rep movsb
 	ret
-ENDPROC(memcpy_erms)
 
-ENTRY(memcpy_orig)
+memcpy_orig:
 	movq %rdi, %rax
 
 	cmpq $0x20, %rdx

@@ -176,4 +173,5 @@ ENTRY(memcpy_orig)
 
 .Lend:
 	retq
-ENDPROC(memcpy_orig)
+ENDPROC(memcpy)
+ENDPROC(__memcpy)

index 2661fad..a0d9f3f 100644
@@ -41,8 +41,6 @@ ENTRY(__memset)
 	rep stosb
 	movq %r9,%rax
 	ret
-ENDPROC(memset)
-ENDPROC(__memset)
 
 /*
  * ISO C memset - set a memory block to a byte value. This function uses

@@ -55,16 +53,15 @@ ENDPROC(__memset)
  *
  * rax   original destination
  */
-ENTRY(memset_erms)
+memset_erms:
 	movq %rdi,%r9
 	movb %sil,%al
 	movq %rdx,%rcx
 	rep stosb
 	movq %r9,%rax
 	ret
-ENDPROC(memset_erms)
 
-ENTRY(memset_orig)
+memset_orig:
 	movq %rdi,%r10
 
 	/* expand byte value  */

@@ -135,4 +132,5 @@ ENTRY(memset_orig)
 	subq %r8,%rdx
 	jmp .Lafter_bad_alignment
 .Lfinal:
-ENDPROC(memset_orig)
+ENDPROC(memset)
+ENDPROC(__memset)

2.1.0

Fix the following asmvalidate warnings:

asmvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_read_failed(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/lib/rwsem.o: call_rwsem_down_write_failed(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/lib/rwsem.o: call_rwsem_wake(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/lib/rwsem.o: call_rwsem_downgrade_wake(): missing FP_SAVE/RESTORE macros

These are callable non-leaf functions, so save/restore the frame pointer with FP_SAVE/RESTORE.

---
 arch/x86/lib/rwsem.S | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

index 40027db..709bc9d 100644
@@ -15,6 +15,7 @@
 
 #include <linux/linkage.h>
 #include <asm/alternative-asm.h>
+#include <asm/func.h>
 
 #define __ASM_HALF_REG(reg)	__ASM_SEL(reg, e##reg)
 #define __ASM_HALF_SIZE(inst)	__ASM_SEL(inst##w, inst##l)

@@ -84,24 +85,29 @@
 
 /* Fix up special calling conventions */
 ENTRY(call_rwsem_down_read_failed)
+	FP_SAVE
 	save_common_regs
 	__ASM_SIZE(push,) %__ASM_REG(dx)
 	movq %rax,%rdi
 	call rwsem_down_read_failed
 	__ASM_SIZE(pop,) %__ASM_REG(dx)
 	restore_common_regs
+	FP_RESTORE
 	ret
 ENDPROC(call_rwsem_down_read_failed)
 
 ENTRY(call_rwsem_down_write_failed)
+	FP_SAVE
 	save_common_regs
 	movq %rax,%rdi
 	call rwsem_down_write_failed
 	restore_common_regs
+	FP_RESTORE
 	ret
 ENDPROC(call_rwsem_down_write_failed)
 
 ENTRY(call_rwsem_wake)
+	FP_SAVE
 	/* do nothing if still outstanding active readers */
 	__ASM_HALF_SIZE(dec) %__ASM_HALF_REG(dx)
 	jnz 1f

@@ -109,15 +115,18 @@ ENTRY(call_rwsem_wake)
 	movq %rax,%rdi
 	call rwsem_wake
 	restore_common_regs
-1:	ret
+1:	FP_RESTORE
+	ret
 ENDPROC(call_rwsem_wake)
 
 ENTRY(call_rwsem_downgrade_wake)
+	FP_SAVE
 	save_common_regs
 	__ASM_SIZE(push,) %__ASM_REG(dx)
 	movq %rax,%rdi
 	call rwsem_downgrade_wake
 	__ASM_SIZE(pop,) %__ASM_REG(dx)
 	restore_common_regs
+	FP_RESTORE
 	ret
 ENDPROC(call_rwsem_downgrade_wake)

2.1.0

Here are the 194 outstanding warnings I'm seeing with my Fedora kernel config. I'll keep chipping away at them.

asmvalidate: arch/x86/crypto/crc32c-pcl-intel-asm_64.o: crc_pcl()+0x84: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/crc32c-pcl-intel-asm_64.o: crc_pcl(): unsupported fallthrough at end of function
asmvalidate: arch/x86/crypto/sha1_avx2_x86_64_asm.o: sha1_transform_avx2()+0x645: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/sha1_avx2_x86_64_asm.o: sha1_transform_avx2()+0x1418: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/sha1_avx2_x86_64_asm.o: sha1_transform_avx2()+0x16e4: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/sha1_avx2_x86_64_asm.o: sha1_transform_avx2()+0x1a22: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: __camellia_enc_blk16(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: __camellia_dec_blk16(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_ecb_enc_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_ecb_dec_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_cbc_dec_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_ctr_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_xts_crypt_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_xts_enc_16way()+0xb: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_xts_enc_16way(): unsupported fallthrough at end of function
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_xts_dec_16way()+0x1e: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/camellia-aesni-avx-asm_64.o: camellia_xts_dec_16way(): unsupported fallthrough at end of function
asmvalidate: arch/x86/crypto/cast5-avx-x86_64-asm_64.o: cast5_ecb_enc_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast5-avx-x86_64-asm_64.o: cast5_ecb_dec_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast5-avx-x86_64-asm_64.o: cast5_cbc_dec_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast5-avx-x86_64-asm_64.o: cast5_ctr_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast6-avx-x86_64-asm_64.o: cast6_ecb_enc_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast6-avx-x86_64-asm_64.o: cast6_ecb_dec_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast6-avx-x86_64-asm_64.o: cast6_cbc_dec_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast6-avx-x86_64-asm_64.o: cast6_ctr_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast6-avx-x86_64-asm_64.o: cast6_xts_enc_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/cast6-avx-x86_64-asm_64.o: cast6_xts_dec_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/twofish-avx-x86_64-asm_64.o: twofish_ecb_enc_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/twofish-avx-x86_64-asm_64.o: twofish_ecb_dec_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/twofish-avx-x86_64-asm_64.o: twofish_cbc_dec_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/twofish-avx-x86_64-asm_64.o: twofish_ctr_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/twofish-avx-x86_64-asm_64.o: twofish_xts_enc_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/twofish-avx-x86_64-asm_64.o: twofish_xts_dec_8way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx-x86_64-asm_64.o: serpent_ecb_enc_8way_avx(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx-x86_64-asm_64.o: serpent_ecb_dec_8way_avx(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx-x86_64-asm_64.o: serpent_cbc_dec_8way_avx(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx-x86_64-asm_64.o: serpent_ctr_8way_avx(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx-x86_64-asm_64.o: serpent_xts_enc_8way_avx(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx-x86_64-asm_64.o: serpent_xts_dec_8way_avx(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: __camellia_enc_blk32(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: __camellia_dec_blk32(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_ecb_enc_32way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_ecb_dec_32way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_cbc_dec_32way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_ctr_32way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_xts_crypt_32way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_xts_enc_32way()+0xb: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_xts_enc_32way(): unsupported fallthrough at end of function
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_xts_dec_32way()+0x1e: unsupported jump to outside of function
asmvalidate: arch/x86/crypto/camellia-aesni-avx2-asm_64.o: camellia_xts_dec_32way(): unsupported fallthrough at end of function
asmvalidate: arch/x86/crypto/serpent-avx2-asm_64.o: serpent_ecb_enc_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx2-asm_64.o: serpent_ecb_dec_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx2-asm_64.o: serpent_cbc_dec_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx2-asm_64.o: serpent_ctr_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx2-asm_64.o: serpent_xts_enc_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/crypto/serpent-avx2-asm_64.o: serpent_xts_dec_16way(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/entry/entry_64.o: native_usergs_sysret64(): unsupported fallthrough at end of function
asmvalidate: arch/x86/entry/entry_64.o: .entry.text+0x399: return instruction outside of a function
asmvalidate: arch/x86/entry/entry_64.o: .entry.text+0x1ba9: return instruction outside of a function
asmvalidate: arch/x86/entry/entry_64.o: .entry.text+0x1bd5: return instruction outside of a function
asmvalidate: arch/x86/entry/entry_64.o: .entry.text+0x20e4: return instruction outside of a function
asmvalidate: arch/x86/entry/entry_64.o: .entry.text+0x21be: return instruction outside of a function
asmvalidate: arch/x86/entry/vdso/vdso32/int80.o: __kernel_sigreturn(): unsupported fallthrough at end of function
asmvalidate: arch/x86/entry/vdso/vdso32/int80.o: __kernel_rt_sigreturn(): unsupported fallthrough at end of function
asmvalidate: arch/x86/entry/vdso/vdso32/syscall.o: __kernel_sigreturn(): unsupported fallthrough at end of function
asmvalidate: arch/x86/entry/vdso/vdso32/syscall.o: __kernel_rt_sigreturn(): unsupported fallthrough at end of function
asmvalidate: arch/x86/entry/vdso/vdso32/sysenter.o: __kernel_sigreturn(): unsupported fallthrough at end of function
asmvalidate: arch/x86/entry/vdso/vdso32/sysenter.o: __kernel_rt_sigreturn(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/mcount_64.o: .entry.text+0x0: return instruction outside of a function
asmvalidate: arch/x86/kernel/mcount_64.o: .entry.text+0xbb: return instruction outside of a function
asmvalidate: arch/x86/kernel/mcount_64.o: .entry.text+0x2b7: return instruction outside of a function
asmvalidate: arch/x86/kernel/relocate_kernel_64.o: .text+0x6b: return instruction outside of a function
asmvalidate: arch/x86/kernel/relocate_kernel_64.o: .text+0xc7: return instruction outside of a function
asmvalidate: arch/x86/kernel/relocate_kernel_64.o: .text+0x110: return instruction outside of a function
asmvalidate: arch/x86/kernel/relocate_kernel_64.o: .text+0x145: return instruction outside of a function
asmvalidate: arch/x86/kernel/relocate_kernel_64.o: .text+0x1c4: return instruction outside of a function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_set_trap_table(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_mmu_update(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_set_gdt(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_stack_switch(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_set_callbacks(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_fpu_taskswitch(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_sched_op_compat(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_dom0_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_set_debugreg(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_get_debugreg(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_update_descriptor(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_memory_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_multicall(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_update_va_mapping(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_set_timer_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_event_channel_op_compat(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_xen_version(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_console_io(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_physdev_op_compat(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_grant_table_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_vm_assist(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_update_va_mapping_otherdomain(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_iret(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_vcpu_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_set_segment_base(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_mmuext_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_xsm_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_nmi_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_sched_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_callback_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_xenoprof_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_event_channel_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_physdev_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_hvm_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_sysctl(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_domctl(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_kexec_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_tmem_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_xc_reserved_op(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_mca(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_arch_1(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_arch_2(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_arch_3(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_arch_4(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_arch_5(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_arch_6(): unsupported fallthrough at end of function
asmvalidate: arch/x86/kernel/head_64.o: xen_hypercall_arch_7(): unsupported fallthrough at end of function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0x18: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0x34: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0x47: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0x74: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0xa7: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0xd3: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0xfa: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0x127: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0x14d: return instruction outside of a function
asmvalidate: arch/x86/net/bpf_jit.o: .text+0x16d: return instruction outside of a function
asmvalidate: arch/x86/platform/efi/efi_thunk_64.o: efi64_thunk(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/platform/efi/efi_thunk_64.o: efi_enter32(): unsupported fallthrough at end of function
asmvalidate: arch/x86/platform/efi/efi_thunk_64.o: efi_enter32(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/xen/xen-asm.o: xen_irq_enable_direct(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/xen/xen-asm.o: xen_restore_fl_direct(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/xen/xen-asm.o: .text+0x7f: return instruction outside of a function
asmvalidate: arch/x86/xen/xen-asm_64.o: .text+0xa: return instruction outside of a function
asmvalidate: arch/x86/xen/xen-asm_64.o: xen_syscall_target()+0xe: unsupported jump to outside of function
asmvalidate: arch/x86/xen/xen-asm_64.o: xen_syscall_target(): unsupported fallthrough at end of function
asmvalidate: arch/x86/xen/xen-asm_64.o: xen_syscall32_target()+0xe: unsupported jump to outside of function
asmvalidate: arch/x86/xen/xen-asm_64.o: xen_syscall32_target(): unsupported fallthrough at end of function
asmvalidate: arch/x86/xen/xen-asm_64.o: xen_sysenter_target()+0xe: unsupported jump to outside of function
asmvalidate: arch/x86/xen/xen-asm_64.o: xen_sysenter_target(): unsupported fallthrough at end of function
asmvalidate: arch/x86/power/hibernate_asm_64.o: .text+0x69: return instruction outside of a function
asmvalidate: arch/x86/power/hibernate_asm_64.o: .text+0x16d: return instruction outside of a function
asmvalidate: arch/x86/lib/copy_user_64.o: _copy_to_user()+0x15: unsupported jump to outside of function
asmvalidate: arch/x86/lib/copy_user_64.o: _copy_to_user()+0x1f: unsupported jump to outside of function
asmvalidate: arch/x86/lib/copy_user_64.o: _copy_to_user()+0x25: unsupported jump to outside of function
asmvalidate: arch/x86/lib/copy_user_64.o: _copy_to_user(): unsupported fallthrough at end of function
asmvalidate: arch/x86/lib/copy_user_64.o: _copy_from_user()+0x15: unsupported jump to outside of function
asmvalidate: arch/x86/lib/copy_user_64.o: _copy_from_user()+0x1f: unsupported jump to outside of function
asmvalidate: arch/x86/lib/copy_user_64.o: _copy_from_user()+0x25: unsupported jump to outside of function
asmvalidate: arch/x86/lib/copy_user_64.o: _copy_from_user(): unsupported fallthrough at end of function
asmvalidate: arch/x86/lib/copy_user_64.o: alternative jump to outside the scope of original function _copy_to_user
asmvalidate: arch/x86/lib/copy_user_64.o: alternative jump to outside the scope of original function _copy_to_user
asmvalidate: arch/x86/lib/copy_user_64.o: alternative jump to outside the scope of original function _copy_from_user
asmvalidate: arch/x86/lib/copy_user_64.o: alternative jump to outside the scope of original function _copy_from_user
asmvalidate: arch/x86/lib/csum-copy_64.o: csum_partial_copy_generic()+0x6: unsupported jump to outside of function
asmvalidate: arch/x86/lib/getuser.o: __get_user_1()+0x14: unsupported jump to outside of function
asmvalidate: arch/x86/lib/getuser.o: __get_user_2()+0x4: unsupported jump to outside of function
asmvalidate: arch/x86/lib/getuser.o: __get_user_2()+0x1e: unsupported jump to outside of function
asmvalidate: arch/x86/lib/getuser.o: __get_user_4()+0x4: unsupported jump to outside of function
asmvalidate: arch/x86/lib/getuser.o: __get_user_4()+0x1a: unsupported jump to outside of function
asmvalidate: arch/x86/lib/getuser.o: __get_user_8()+0x4: unsupported jump to outside of function
asmvalidate: arch/x86/lib/getuser.o: __get_user_8()+0x1a: unsupported jump to outside of function
asmvalidate: arch/x86/lib/getuser.o: .text+0xc5: return instruction outside of a function
asmvalidate: arch/x86/lib/putuser.o: __put_user_1()+0x14: unsupported jump to outside of function
asmvalidate: arch/x86/lib/putuser.o: __put_user_2()+0x1b: unsupported jump to outside of function
asmvalidate: arch/x86/lib/putuser.o: __put_user_4()+0x1b: unsupported jump to outside of function
asmvalidate: arch/x86/lib/putuser.o: __put_user_8()+0x1b: unsupported jump to outside of function
asmvalidate: arch/x86/lib/putuser.o: .text+0xc1: return instruction outside of a function
asmvalidate: arch/x86/boot/copy.o: copy_from_fs(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/boot/copy.o: copy_to_fs(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/boot/compressed/head_64.o: .text+0x16e: return instruction outside of a function
asmvalidate: arch/x86/boot/compressed/head_64.o: .text+0x172: return instruction outside of a function
asmvalidate: arch/x86/boot/compressed/head_64.o: startup_32()+0x38: unsupported jump to outside of function
asmvalidate: arch/x86/boot/compressed/head_64.o: startup_32(): unsupported fallthrough at end of function
asmvalidate: arch/x86/boot/compressed/head_64.o: startup_32(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/boot/compressed/head_64.o: efi32_stub_entry()+0x37: unsupported jump to outside of function
asmvalidate: arch/x86/boot/compressed/head_64.o: efi32_stub_entry(): unsupported fallthrough at end of function
asmvalidate: arch/x86/boot/compressed/head_64.o: efi32_stub_entry(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/boot/compressed/head_64.o: efi64_stub_entry()+0x1f: unsupported jump to outside of function
asmvalidate: arch/x86/boot/compressed/head_64.o: efi64_stub_entry(): unsupported fallthrough at end of function
asmvalidate: arch/x86/boot/compressed/efi_thunk_64.o: efi_enter32(): unsupported fallthrough at end of function
asmvalidate: arch/x86/boot/compressed/efi_thunk_64.o: efi_enter32(): missing FP_SAVE/RESTORE macros
asmvalidate: arch/x86/boot/header.o: die()+0x1: unsupported jump to outside of function
asmvalidate: arch/x86/boot/header.o: die(): unsupported fallthrough at end of function
asmvalidate: arch/x86/boot/pmjump.o: protected_mode_jump()+0x11: unsupported jump to outside of function
asmvalidate: arch/x86/boot/pmjump.o: protected_mode_jump(): unsupported fallthrough at end of function
asmvalidate: arch/x86/boot/pmjump.o: in_pm32()+0x1c: unsupported jump to outside of function
asmvalidate: arch/x86/boot/pmjump.o: in_pm32(): unsupported fallthrough at end of function

Josh

That prevents a lot of optimizations with out of line code.

In fact even gcc with the right options can generate code that violates this. Standard Linux constructions, such as exception handling, also violate this.

If your tool needs that your tool is broken.

BTW any other frame pointer requirement should be also optional, as it slows down a number of CPUs, such as Atoms.

-Andi

In most cases there are ways to keep the optimizations. For example:

- grow the function bounds to keep the jump internal
- duplicate the destination code inside the function
- convert the jump to a call

Also note that these rules only affect _callable_ functions, so the entry code and other non-function asm code can still be a pile of spaghetti (though I think Andy is working on improving that).

This tool only validates asm code, so I don't see how whatever gcc does is relevant.

Yes. This patch set is a first step towards being able to disable frame pointers. Once all callable functions are reasonably structured, we can start generating and validating DWARF CFI data. Then we can make a reliable DWARF unwinder and get rid of frame pointers.

Josh

So you mean moving it after the ret? That still means icache bloat.

That all won't work for a lot of cases.

Thank you for your kind words.

Whoever needs it would need it everywhere, right? If it's not needed for gcc then it shouldn't be needed for assembler code either.

BTW the way gcc handles it is to use the dwarf 4 extensions that can describe non contiguous sections.`

-Andi

[email protected] -- Speaking for myself only.

No, in most cases it just means changing the ELF annotations. See patch 9 for an example.

Hm, could you give an example?

Don't like spaghetti? :-)

Well, I don't see how that's really a logical conclusion. But we're probably being too vague here... Do you have any examples where you really need to jump outside of a callable function?

If we ignore C++, then 99% of the time, C functions are self-contained. The only exception I can think of is for switch statements, which sometimes have an external jump table.

Josh

Just a standard *_user exception handler.

What's special about assembler code?

It's not needed, but it's an optimization to optimize icache usage. It is optional (-freorder-blocks-and-partition)

In this case gcc splits the function into two (hot and cold)

It's actually a nice optimization and it would be sad from stopping the kernel from using it.

-Andi

[email protected] -- Speaking for myself only.

I'm afraid I don't follow. Exception handlers don't work via jump instructions, but rather via CPU exceptions.

Or are you talking about something else?

Ok, I'll bite:

- it's human-generated
- it's much simpler
- it doesn't have stack metadata by default
- it has far fewer constraints

But I don't see this line of discussion going anywhere without any real examples of why you need external jumps in asm functions...

Sorry if I wasn't clear, I was trying to ask for examples in kernel asm code.

Are you suggesting that we implement this gcc optimization in kernel asm code?

Josh

If you're wanting something like -freorder-blocks-and-partition for asm code, maybe we could implement something analagous to the likely()/unlikely() macros, to allow the hot and cold portions to be placed into different sections. (I could then teach asmvalidate how to validate such code.)

Would that be useful?

Josh

Eventually you have to handle dwarf4, because that's the only way to handle the gcc generated code.

So your tool just needs to understand the dwarf. It's ok to warn about assembler code that is not correctly annotated, and fix it then.

-Andi

[email protected] -- Speaking for myself only

Ok, I think we're talking about two different things. Maybe you're mixing up this patch set with some of the other things we've discussed like the DWARF CFI generation tool or the runtime DWARF unwinder.

This patch set is only concerned with enforcing sane rules on asm code. It has nothing to do with understanding DWARF. In fact, since Ingo reverted the CFI annotations, there is currently no DWARF in asm code at all. This patch set is a prerequisite for the other DWARF stuff we talked about previously.

Josh

Let's take an example:

102:

        .section .fixup,"ax"
        103:    addl %ecx,%edx                  /* ecx is zerorest also */
        jmp copy_user_handle_tail
       .previous

        _ASM_EXTABLE(100b,103b)
        _ASM_EXTABLE(101b,103b)

The exception handling code is part of the function, but it's out of line.

It was how Linux traditionally implemented locking code for example. Have the hot path handle the uncontended fast path, and the slow path call.

I don't know if there is much left of it (a lot of it was removed because it was hard to describe in dwarf3, needs dwarf4). But it seems bad to completely disallow it.

But yes eventually gcc generated code should use it again, because it's great for icache usage if you measure it correctly at run time (not the broken "size" approach that is unfortunately far too common)

-Andi

[email protected] -- Speaking for myself only

The jump instruction is in the .fixup section, not in the callable function itself. So it doesn't violate the asmvalidate rules.

This patch set has no relationship to gcc generated code whatsoever. So it doesn't disallow anything there.

For kernel asm code, AFAIK, such a mechanism for hot/cold path separation in separate sections doesn't exist today. So it's not "disallowed" there either. It's just apparently not currently done.

If somebody were to create such a mechanism, I think we could standardize it in such a way that it could be compatible with asmvalidate.

Josh

It still won't unwind correctly unless .pushsection somehow magically propagates CFI state. (Does it?)

Hopefully true. The entry code is full of tail calls, though.

--Andy

Andy Lutomirski
AMA Capital Management, LLC

I don't think it does. We'll probably need some intelligence in the CFI generation tooling to deal properly with the extable stuff.

Well, I wasn't talking specifically about tail calls here. But either way, as long as they're not in a callable function (which is the case for most of the entry code), asmvalidate doesn't care.

Josh

You have interesting definition of "clean".

The reason we sometimes have to use assembly is that it is impossible to write corresponding code in C, or that performance would be bad.

So... fixing these may have some sense, but I doubt enforcing "you can't write real assembly" is a good idea.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

"clean":

- reliably honors CONFIG_FRAME_POINTER
- reliably creates/generates DWARF CFI metadata
- doesn't break stack walking
- code is more readable

Agreed, but I don't see how this patch set prevents those things.

You can certainly still write real assembly. This just creates a few constraints. I really don't think they are very limiting.

Josh

Slightly off-topic, but this reminds me: when writing inline asm that needs to push to the stack (for whatever reason), it's incredibly messy to get the annotations right -- they're different depending on whether the previous frame base (is that what "CFA" is?) is currently sp + constant, in which case we need an annotation adjusting the constant or whether it's independent of sp (bp + constant), in which case we shouldn't adjust the offset. (If it's some other function of sp, we're screwed.)

Regardless of whether these types of annotations end up being done by hand or by script, should we consider asking the binutils people to give us some nice .cfi_adjust_for_push and .cfi_adjust_for_pop or similar directives?

See here for Jan Beulich's solution, which is incomprehensible to me:

http://thread.gmane.org/gmane.linux.kernel/1820765

--Andy

Hm, that's a tough one. Might be worth asking...

Another alternative would be to ask gcc to make a change to always setup the frame pointer for any function which has inline assembly, so that you know (hopefully) that CFA is based on bp.

Or, maybe there's already a way to force gcc to do that with the asm directive somehow?

<brain explodes>

Josh