[PATCH 0/4] resend build sanity checking additions

[PATCH 0/4] resend build sanity checking additions

[Nicholas Piggin]

I've updated these in response to feedback and some wider testing
(that uncovered a few other issues).

Thanks,
Nick

Nicholas Piggin (4):
  powerpc: use the new post-link pass to check relocations
  powerpc: add arch/powerpc/tools directory
  powerpc/64s: tool to flag direct branches from unrelocated interrupt
    vectors
  powerpc/64: tool to check head sections location sanity

 arch/powerpc/Makefile                              | 13 +---
 arch/powerpc/Makefile.postlink                     | 47 +++++++++++++++
 arch/powerpc/include/asm/head-64.h                 |  4 +-
 arch/powerpc/kernel/vmlinux.lds.S                  | 22 -------
 .../gcc-check-mprofile-kernel.sh                   |  0
 arch/powerpc/tools/head_check.sh                   | 70 ++++++++++++++++++++++
 arch/powerpc/{ => tools}/relocs_check.sh           |  0
 arch/powerpc/tools/unrel_branch_check.sh           | 57 ++++++++++++++++++
 8 files changed, 177 insertions(+), 36 deletions(-)
 create mode 100644 arch/powerpc/Makefile.postlink
 rename arch/powerpc/{scripts => tools}/gcc-check-mprofile-kernel.sh (100%)
 create mode 100755 arch/powerpc/tools/head_check.sh
 rename arch/powerpc/{ => tools}/relocs_check.sh (100%)
 create mode 100755 arch/powerpc/tools/unrel_branch_check.sh

-- 
2.10.2

[PATCH 1/4] powerpc: use the new post-link pass to check relocations

[Nicholas Piggin]

Currently powerpc has to introduce a dependency on its default
build target zImage in order to run a relocation check pass
over the linked vmlinux. This is deficient because the check
is not run if the plain vmlinux target is built, or if one of
the other boot targets is built.

Switch to using the kbuild post-link pass in order to run this
check. In future powerpc will use this to do more complicated
operations, but initially using it for something simple is a
good first step.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/Makefile          | 11 -----------
 arch/powerpc/Makefile.postlink | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 11 deletions(-)
 create mode 100644 arch/powerpc/Makefile.postlink

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 8828807..868b9ec 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -270,17 +270,6 @@ PHONY += $(BOOT_TARGETS1) $(BOOT_TARGETS2)
 
 boot := arch/$(ARCH)/boot
 
-ifeq ($(CONFIG_RELOCATABLE),y)
-quiet_cmd_relocs_check = CALL    $<
-      cmd_relocs_check = $(CONFIG_SHELL) $< "$(OBJDUMP)" "$(obj)/vmlinux"
-
-PHONY += relocs_check
-relocs_check: arch/powerpc/relocs_check.sh vmlinux
-	$(call cmd,relocs_check)
-
-zImage: relocs_check
-endif
-
 $(BOOT_TARGETS1): vmlinux
 	$(Q)$(MAKE) ARCH=ppc64 $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 $(BOOT_TARGETS2): vmlinux
diff --git a/arch/powerpc/Makefile.postlink b/arch/powerpc/Makefile.postlink
new file mode 100644
index 0000000..8c0170e
--- /dev/null
+++ b/arch/powerpc/Makefile.postlink
@@ -0,0 +1,34 @@
+# ===========================================================================
+# Post-link powerpc pass
+# ===========================================================================
+#
+# 1. Check that vmlinux relocations look sane
+
+PHONY := __archpost
+__archpost:
+
+include include/config/auto.conf
+include scripts/Kbuild.include
+
+quiet_cmd_relocs_check = CHKREL  $@
+      cmd_relocs_check = $(CONFIG_SHELL) $(srctree)/arch/powerpc/relocs_check.sh "$(OBJDUMP)" "$@"
+
+# `@true` prevents complaint when there is nothing to be done
+
+vmlinux: FORCE
+	@true
+ifdef CONFIG_RELOCATABLE
+	$(call if_changed,relocs_check)
+endif
+
+%.ko: FORCE
+	@true
+
+clean:
+	@true
+
+PHONY += FORCE clean
+
+FORCE:
+
+.PHONY: $(PHONY)
-- 
2.10.2

[PATCH 2/4] powerpc: add arch/powerpc/tools directory

[Nicholas Piggin]

Move a couple of existing scripts under there. Remove scripts directory:
a script is a tool, a tool is not a script.

Signed-off-by: Nick Piggin <npiggin@gmail.com>
---
 arch/powerpc/Makefile                                        | 2 +-
 arch/powerpc/Makefile.postlink                               | 2 +-
 arch/powerpc/{scripts => tools}/gcc-check-mprofile-kernel.sh | 0
 arch/powerpc/{ => tools}/relocs_check.sh                     | 0
 4 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/powerpc/{scripts => tools}/gcc-check-mprofile-kernel.sh (100%)
 rename arch/powerpc/{ => tools}/relocs_check.sh (100%)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 868b9ec..6575b7e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -133,7 +133,7 @@ CFLAGS-$(CONFIG_GENERIC_CPU) += -mcpu=powerpc64
 endif
 
 ifdef CONFIG_MPROFILE_KERNEL
-    ifeq ($(shell $(srctree)/arch/powerpc/scripts/gcc-check-mprofile-kernel.sh $(CC) -I$(srctree)/include -D__KERNEL__),OK)
+    ifeq ($(shell $(srctree)/arch/powerpc/tools/gcc-check-mprofile-kernel.sh $(CC) -I$(srctree)/include -D__KERNEL__),OK)
         CC_FLAGS_FTRACE := -pg -mprofile-kernel
         KBUILD_CPPFLAGS += -DCC_USING_MPROFILE_KERNEL
     else
diff --git a/arch/powerpc/Makefile.postlink b/arch/powerpc/Makefile.postlink
index 8c0170e..3c22d64 100644
--- a/arch/powerpc/Makefile.postlink
+++ b/arch/powerpc/Makefile.postlink
@@ -11,7 +11,7 @@ include include/config/auto.conf
 include scripts/Kbuild.include
 
 quiet_cmd_relocs_check = CHKREL  $@
-      cmd_relocs_check = $(CONFIG_SHELL) $(srctree)/arch/powerpc/relocs_check.sh "$(OBJDUMP)" "$@"
+      cmd_relocs_check = $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$@"
 
 # `@true` prevents complaint when there is nothing to be done
 
diff --git a/arch/powerpc/scripts/gcc-check-mprofile-kernel.sh b/arch/powerpc/tools/gcc-check-mprofile-kernel.sh
similarity index 100%
rename from arch/powerpc/scripts/gcc-check-mprofile-kernel.sh
rename to arch/powerpc/tools/gcc-check-mprofile-kernel.sh
diff --git a/arch/powerpc/relocs_check.sh b/arch/powerpc/tools/relocs_check.sh
similarity index 100%
rename from arch/powerpc/relocs_check.sh
rename to arch/powerpc/tools/relocs_check.sh
-- 
2.10.2

[PATCH 3/4] powerpc/64s: tool to flag direct branches from unrelocated interrupt vectors

[Nicholas Piggin]

Direct banches from code below __end_interrupts to code above
__end_interrupts when built with CONFIG_RELOCATABLE are disallowed
because they will break when the kernel is not located at 0.

Sample output:

    WARNING: Unrelocated relative branches
    c000000000000118 bl-> 0xc000000000038fb8 <pnv_restore_hyp_resource>
    c00000000000013c b-> 0xc0000000001068a4 <kvm_start_guest>
    c000000000000148 b-> 0xc00000000003919c <pnv_wakeup_loss>
    c00000000000014c b-> 0xc00000000003923c <pnv_wakeup_noloss>
    c0000000000005a4 b-> 0xc000000000106ffc <kvmppc_interrupt_hv>
    c000000000001af0 b-> 0xc000000000106ffc <kvmppc_interrupt_hv>
    c000000000001b24 b-> 0xc000000000106ffc <kvmppc_interrupt_hv>
    c000000000001b58 b-> 0xc000000000106ffc <kvmppc_interrupt_hv>

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/Makefile.postlink           |  9 ++++-
 arch/powerpc/tools/unrel_branch_check.sh | 57 ++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100755 arch/powerpc/tools/unrel_branch_check.sh

diff --git a/arch/powerpc/Makefile.postlink b/arch/powerpc/Makefile.postlink
index 3c22d64..1f9f6e6 100644
--- a/arch/powerpc/Makefile.postlink
+++ b/arch/powerpc/Makefile.postlink
@@ -11,7 +11,14 @@ include include/config/auto.conf
 include scripts/Kbuild.include
 
 quiet_cmd_relocs_check = CHKREL  $@
-      cmd_relocs_check = $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$@"
+ifdef CONFIG_PPC_BOOK3S_64
+      cmd_relocs_check =						\
+	$(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$@" ; \
+	$(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/unrel_branch_check.sh "$(OBJDUMP)" "$@"
+else
+      cmd_relocs_check =						\
+	$(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$@"
+endif
 
 # `@true` prevents complaint when there is nothing to be done
 
diff --git a/arch/powerpc/tools/unrel_branch_check.sh b/arch/powerpc/tools/unrel_branch_check.sh
new file mode 100755
index 0000000..1e972df
--- /dev/null
+++ b/arch/powerpc/tools/unrel_branch_check.sh
@@ -0,0 +1,57 @@
+# Copyright © 2016 IBM Corporation
+#
+# 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 script checks the relocations of a vmlinux for "suspicious"
+# branches from unrelocated code (head_64.S code).
+
+# Turn this on if you want more debug output:
+# set -x
+
+# Have Kbuild supply the path to objdump so we handle cross compilation.
+objdump="$1"
+vmlinux="$2"
+
+#__end_interrupts should be located within the first 64K
+
+end_intr=0x$(
+"$objdump" -R "$vmlinux" -d --start-address=0xc000000000000000		\
+		 --stop-address=0xc000000000010000 |
+grep '\<__end_interrupts>:' |
+awk '{print $1}'
+)
+
+BRANCHES=$(
+"$objdump" -R "$vmlinux" -D --start-address=0xc000000000000000		\
+		--stop-address=${end_intr} |
+grep -e "^c[0-9a-f]*:[[:space:]]*\([0-9a-f][0-9a-f][[:space:]]\)\{4\}[[:space:]]*b" |
+grep -v '\<__start_initialization_multiplatform>' |
+grep -v -e 'b.\?.\?ctr' |
+grep -v -e 'b.\?.\?lr' |
+sed 's/://' |
+awk '{ print $1 ":" $6 ":0x" $7 ":" $8 " "}'
+)
+
+for tuple in $BRANCHES
+do
+	from=`echo $tuple | cut -d':' -f1`
+	branch=`echo $tuple | cut -d':' -f2`
+	to=`echo $tuple | cut -d':' -f3 | sed 's/cr[0-7],//'`
+	sym=`echo $tuple | cut -d':' -f4`
+
+	if (( $to > $end_intr ))
+	then
+		if [ -z "$bad_branches" ]; then
+			echo "WARNING: Unrelocated relative branches"
+			bad_branches="yes"
+		fi
+		echo "$from $branch-> $to $sym"
+	fi
+done
+
+if [ -z "$bad_branches" ]; then
+	exit 0
+fi
-- 
2.10.2

[PATCH 4/4] powerpc/64: tool to check head sections location sanity

[Nicholas Piggin]

Use a tool to check the location of "fixed sections" is where we
expected them, which catches cases the linker script can't (stubs
being added to start of .text section), and which ends up being
neater.

Sample output:

  ERROR: start_text address is c000000000008100, should be c000000000008000
  ERROR: see comments in arch/powerpc/tools/head_check.sh

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/Makefile.postlink     |  8 ++++-
 arch/powerpc/include/asm/head-64.h |  4 +--
 arch/powerpc/kernel/vmlinux.lds.S  | 22 ------------
 arch/powerpc/tools/head_check.sh   | 70 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 25 deletions(-)
 create mode 100755 arch/powerpc/tools/head_check.sh

diff --git a/arch/powerpc/Makefile.postlink b/arch/powerpc/Makefile.postlink
index 1f9f6e6..4c1eafd 100644
--- a/arch/powerpc/Makefile.postlink
+++ b/arch/powerpc/Makefile.postlink
@@ -10,6 +10,9 @@ __archpost:
 include include/config/auto.conf
 include scripts/Kbuild.include
 
+quiet_cmd_head_check = CHKHEAD $@
+      cmd_head_check = $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/head_check.sh "$(NM)" "$@"
+
 quiet_cmd_relocs_check = CHKREL  $@
 ifdef CONFIG_PPC_BOOK3S_64
       cmd_relocs_check =						\
@@ -24,6 +27,9 @@ endif
 
 vmlinux: FORCE
 	@true
+ifdef CONFIG_PPC64
+	$(call cmd,head_check)
+endif
 ifdef CONFIG_RELOCATABLE
 	$(call if_changed,relocs_check)
 endif
@@ -32,7 +38,7 @@ endif
 	@true
 
 clean:
-	@true
+	rm -f .tmp_symbols.txt
 
 PHONY += FORCE clean
 
diff --git a/arch/powerpc/include/asm/head-64.h b/arch/powerpc/include/asm/head-64.h
index ab90c2f..492ebe7 100644
--- a/arch/powerpc/include/asm/head-64.h
+++ b/arch/powerpc/include/asm/head-64.h
@@ -49,8 +49,8 @@
  *   CLOSE_FIXED_SECTION() or elsewhere, there may be something
  *   unexpected being added there. Remove the '. = x_len' line, rebuild, and
  *   check what is pushing the section down.
- * - If the build dies in linking, check arch/powerpc/kernel/vmlinux.lds.S
- *   for instructions.
+ * - If the build dies in linking, check arch/powerpc/tools/head_check.sh
+ *   comments.
  * - If the kernel crashes or hangs in very early boot, it could be linker
  *   stubs at the start of the main text.
  */
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 7394b77..bcd2a52 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -58,7 +58,6 @@ SECTIONS
 #ifdef CONFIG_PPC64
 		KEEP(*(.head.text.first_256B));
 #ifdef CONFIG_PPC_BOOK3E
-# define END_FIXED	0x100
 #else
 		KEEP(*(.head.text.real_vectors));
 		*(.head.text.real_trampolines);
@@ -66,34 +65,13 @@ SECTIONS
 		*(.head.text.virt_trampolines);
 # if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_POWERNV)
 		KEEP(*(.head.data.fwnmi_page));
-#  define END_FIXED	0x8000
-# else
-#  define END_FIXED	0x7000
 # endif
 #endif
-		ASSERT((. == END_FIXED), "vmlinux.lds.S: fixed section overflow error");
 #else /* !CONFIG_PPC64 */
 		HEAD_TEXT
 #endif
 	} :kernel
 
-	/*
-	 * If the build dies here, it's likely code in head_64.S is referencing
-	 * labels it can't reach, and the linker inserting stubs without the
-	 * assembler's knowledge. To debug, remove the above assert and
-	 * rebuild. Look for branch stubs in the fixed section region.
-	 *
-	 * Linker stub generation could be allowed in "trampoline"
-	 * sections if absolutely necessary, but this would require
-	 * some rework of the fixed sections. Before resorting to this,
-	 * consider references that have sufficient addressing range,
-	 * (e.g., hand coded trampolines) so the linker does not have
-	 * to add stubs.
-	 *
-	 * Linker stubs at the top of the main text section are currently not
-	 * detected, and will result in a crash at boot due to offsets being
-	 * wrong.
-	 */
 #ifdef CONFIG_PPC64
 	/*
 	 * BLOCK(0) overrides the default output section alignment because
diff --git a/arch/powerpc/tools/head_check.sh b/arch/powerpc/tools/head_check.sh
new file mode 100755
index 0000000..11efe45
--- /dev/null
+++ b/arch/powerpc/tools/head_check.sh
@@ -0,0 +1,70 @@
+# Copyright © 2016 IBM Corporation
+
+# 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 script checks the head of a vmlinux for linker stubs that
+# break our placement of fixed-location code for 64-bit.
+
+# based on relocs_check.pl
+# Copyright © 2009 IBM Corporation
+
+# READ THIS
+#
+# If the build dies here, it's likely code in head_64.S or nearby is
+# referencing labels it can't reach, which results in the linker inserting
+# stubs without the assembler's knowledge. This can move code around in ways
+# that break the fixed location placement stuff (head-64.h). To debug,
+# disassemble the vmlinux and look for branch stubs (long_branch, plt_branch
+# etc) in the fixed section region (0 - 0x8000ish). Check what places are
+# calling those stubs.
+#
+# Linker stubs use the TOC pointer, so even if fixed section code could
+# tolerate them being inserted into head code, they can't be allowed in low
+# level entry code (boot, interrupt vectors, etc) until r2 is set up. This
+# could cause the kernel to die in early boot.
+
+# Turn this on if you want more debug output:
+# set -x
+
+if [ $# -lt 2 ]; then
+	echo "$0 [path to nm] [path to vmlinux]" 1>&2
+	exit 1
+fi
+
+# Have Kbuild supply the path to nm so we handle cross compilation.
+nm="$1"
+vmlinux="$2"
+
+$nm "$vmlinux" | grep -e " T _stext$" -e " t start_first_256B$" -e " a text_start$" -e " t start_text$" -m4 > .tmp_symbols.txt
+
+
+vma=$(cat .tmp_symbols.txt | grep " T _stext$" | cut -d' ' -f1)
+
+expected_start_head_addr=$vma
+
+start_head_addr=$(cat .tmp_symbols.txt | grep " t start_first_256B$" | cut -d' ' -f1)
+
+if [ "$start_head_addr" != "$expected_start_head_addr" ]; then
+	echo "ERROR: head code starts at $start_head_addr, should be 0"
+	echo "ERROR: see comments in arch/powerpc/tools/head_check.sh"
+
+	exit 1
+fi
+
+top_vma=$(echo $vma | cut -d'0' -f1)
+
+expected_start_text_addr=$(cat .tmp_symbols.txt | grep " a text_start$" | cut -d' ' -f1 | sed "s/^0/$top_vma/")
+
+start_text_addr=$(cat .tmp_symbols.txt | grep " t start_text$" | cut -d' ' -f1)
+
+if [ "$start_text_addr" != "$expected_start_text_addr" ]; then
+	echo "ERROR: start_text address is $start_text_addr, should be $expected_start_text_addr"
+	echo "ERROR: see comments in arch/powerpc/tools/head_check.sh"
+
+	exit 1
+fi
+
+rm .tmp_symbols.txt
-- 
2.10.2

Re: [1/4] powerpc: use the new post-link pass to check relocations

[Michael Ellerman]

On Sat, 2016-11-26 at 03:26:09 UTC, Nicholas Piggin wrote:
> Currently powerpc has to introduce a dependency on its default
> build target zImage in order to run a relocation check pass
> over the linked vmlinux. This is deficient because the check
> is not run if the plain vmlinux target is built, or if one of
> the other boot targets is built.
> 
> Switch to using the kbuild post-link pass in order to run this
> check. In future powerpc will use this to do more complicated
> operations, but initially using it for something simple is a
> good first step.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/f188d0524d7e09f7ec801df9761dc1

cheers

Re: [2/4] powerpc: add arch/powerpc/tools directory

[Michael Ellerman]

On Sat, 2016-11-26 at 03:26:10 UTC, Nicholas Piggin wrote:
> Move a couple of existing scripts under there. Remove scripts directory:
> a script is a tool, a tool is not a script.
> 
> Signed-off-by: Nick Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/b71c9ffb140556004caf7ba27083f9

cheers