[RFC PATCH 0/3] objtool: Add mcount sub-command

[RFC PATCH 0/3] objtool: Add mcount sub-command

[Sathvika Vasireddy]

This patchset adds support to implement 'objtool mcount' command.

Right now, objtool is built if CONFIG_STACK_VALIDATION is enabled.
And, '__mcount_loc' section is generated by objtool when --mcount 
option is passed to check sub-command. 

For architectures to be able to generate '__mcount_loc' section
without having to do stack validation, introduce 'mcount' as
a sub-command to objtool. This way, objtool is built for mcount 
if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL is enabled. Additionally, 
architectures can select HAVE_NOP_MCOUNT to be able to nop out
mcount call sites.  

TODO: Enable "objtool mcount" for clang LTO builds.

Sathvika Vasireddy (3):
  objtool: Move common code to utils.c
  objtool: Enable and implement 'mcount' subcommand
  objtool/mcount: Add powerpc specific functions

 Makefile                                      |   6 +
 arch/powerpc/Kconfig                          |   1 +
 arch/x86/Kconfig                              |   3 +-
 scripts/Makefile.build                        |  12 +
 tools/objtool/Build                           |   3 +
 tools/objtool/Makefile                        |   8 +-
 tools/objtool/arch/powerpc/Build              |   1 +
 tools/objtool/arch/powerpc/decode.c           |  51 +++++
 .../arch/powerpc/include/arch/cfi_regs.h      |  37 +++
 tools/objtool/arch/powerpc/include/arch/elf.h |   8 +
 tools/objtool/builtin-mcount.c                |  74 ++++++
 tools/objtool/check.c                         | 178 +--------------
 tools/objtool/include/objtool/builtin.h       |   4 +-
 tools/objtool/include/objtool/check.h         |   2 -
 tools/objtool/include/objtool/objtool.h       |   1 +
 tools/objtool/include/objtool/utils.h         |  28 +++
 tools/objtool/mcount.c                        | 138 ++++++++++++
 tools/objtool/objtool.c                       |   1 +
 tools/objtool/orc_gen.c                       |   1 +
 tools/objtool/utils.c                         | 210 ++++++++++++++++++
 tools/objtool/weak.c                          |   5 +
 21 files changed, 590 insertions(+), 182 deletions(-)
 create mode 100644 tools/objtool/arch/powerpc/Build
 create mode 100644 tools/objtool/arch/powerpc/decode.c
 create mode 100644 tools/objtool/arch/powerpc/include/arch/cfi_regs.h
 create mode 100644 tools/objtool/arch/powerpc/include/arch/elf.h
 create mode 100644 tools/objtool/builtin-mcount.c
 create mode 100644 tools/objtool/include/objtool/utils.h
 create mode 100644 tools/objtool/mcount.c
 create mode 100644 tools/objtool/utils.c

-- 
2.31.1

[RFC PATCH 1/3] objtool: Move common code to utils.c

[Sathvika Vasireddy]

This patch moves common code to utils.c file. Code from this file will
be reused across check.c and mcount.c (will be introduced in the coming
patches). Also, ensure that this change works well with existing
commands.

Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 tools/objtool/Build                   |   1 +
 tools/objtool/check.c                 | 178 +-----------------------
 tools/objtool/include/objtool/check.h |   2 -
 tools/objtool/include/objtool/utils.h |  28 ++++
 tools/objtool/orc_gen.c               |   1 +
 tools/objtool/utils.c                 | 192 ++++++++++++++++++++++++++
 6 files changed, 223 insertions(+), 179 deletions(-)
 create mode 100644 tools/objtool/include/objtool/utils.h
 create mode 100644 tools/objtool/utils.c

diff --git a/tools/objtool/Build b/tools/objtool/Build
index b7222d5cc7bc..161fd451241a 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -12,6 +12,7 @@ objtool-y += builtin-check.o
 objtool-y += builtin-orc.o
 objtool-y += elf.o
 objtool-y += objtool.o
+objtool-y += utils.o
 
 objtool-y += libstring.o
 objtool-y += libctype.o
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 7c33ec67c4a9..161dd181d9d4 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -12,6 +12,7 @@
 #include <objtool/cfi.h>
 #include <objtool/arch.h>
 #include <objtool/check.h>
+#include <objtool/utils.h>
 #include <objtool/special.h>
 #include <objtool/warn.h>
 #include <objtool/endianness.h>
@@ -33,19 +34,6 @@ static struct cfi_init_state initial_func_cfi;
 static struct cfi_state init_cfi;
 static struct cfi_state func_cfi;
 
-struct instruction *find_insn(struct objtool_file *file,
-			      struct section *sec, unsigned long offset)
-{
-	struct instruction *insn;
-
-	hash_for_each_possible(file->insn_hash, insn, hash, sec_offset_hash(sec, offset)) {
-		if (insn->sec == sec && insn->offset == offset)
-			return insn;
-	}
-
-	return NULL;
-}
-
 static struct instruction *next_insn_same_sec(struct objtool_file *file,
 					      struct instruction *insn)
 {
@@ -342,86 +330,8 @@ static void *cfi_hash_alloc(unsigned long size)
 	return cfi_hash;
 }
 
-static unsigned long nr_insns;
 static unsigned long nr_insns_visited;
 
-/*
- * Call the arch-specific instruction decoder for all the instructions and add
- * them to the global instruction list.
- */
-static int decode_instructions(struct objtool_file *file)
-{
-	struct section *sec;
-	struct symbol *func;
-	unsigned long offset;
-	struct instruction *insn;
-	int ret;
-
-	for_each_sec(file, sec) {
-
-		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
-			continue;
-
-		if (strcmp(sec->name, ".altinstr_replacement") &&
-		    strcmp(sec->name, ".altinstr_aux") &&
-		    strncmp(sec->name, ".discard.", 9))
-			sec->text = true;
-
-		if (!strcmp(sec->name, ".noinstr.text") ||
-		    !strcmp(sec->name, ".entry.text"))
-			sec->noinstr = true;
-
-		for (offset = 0; offset < sec->sh.sh_size; offset += insn->len) {
-			insn = malloc(sizeof(*insn));
-			if (!insn) {
-				WARN("malloc failed");
-				return -1;
-			}
-			memset(insn, 0, sizeof(*insn));
-			INIT_LIST_HEAD(&insn->alts);
-			INIT_LIST_HEAD(&insn->stack_ops);
-
-			insn->sec = sec;
-			insn->offset = offset;
-
-			ret = arch_decode_instruction(file, sec, offset,
-						      sec->sh.sh_size - offset,
-						      &insn->len, &insn->type,
-						      &insn->immediate,
-						      &insn->stack_ops);
-			if (ret)
-				goto err;
-
-			hash_add(file->insn_hash, &insn->hash, sec_offset_hash(sec, insn->offset));
-			list_add_tail(&insn->list, &file->insn_list);
-			nr_insns++;
-		}
-
-		list_for_each_entry(func, &sec->symbol_list, list) {
-			if (func->type != STT_FUNC || func->alias != func)
-				continue;
-
-			if (!find_insn(file, sec, func->offset)) {
-				WARN("%s(): can't find starting instruction",
-				     func->name);
-				return -1;
-			}
-
-			sym_for_each_insn(file, func, insn)
-				insn->func = func;
-		}
-	}
-
-	if (stats)
-		printf("nr_insns: %lu\n", nr_insns);
-
-	return 0;
-
-err:
-	free(insn);
-	return ret;
-}
-
 /*
  * Read the pv_ops[] .data table to find the static initialized values.
  */
@@ -731,49 +641,6 @@ static int create_retpoline_sites_sections(struct objtool_file *file)
 	return 0;
 }
 
-static int create_mcount_loc_sections(struct objtool_file *file)
-{
-	struct section *sec;
-	unsigned long *loc;
-	struct instruction *insn;
-	int idx;
-
-	sec = find_section_by_name(file->elf, "__mcount_loc");
-	if (sec) {
-		INIT_LIST_HEAD(&file->mcount_loc_list);
-		WARN("file already has __mcount_loc section, skipping");
-		return 0;
-	}
-
-	if (list_empty(&file->mcount_loc_list))
-		return 0;
-
-	idx = 0;
-	list_for_each_entry(insn, &file->mcount_loc_list, call_node)
-		idx++;
-
-	sec = elf_create_section(file->elf, "__mcount_loc", 0, sizeof(unsigned long), idx);
-	if (!sec)
-		return -1;
-
-	idx = 0;
-	list_for_each_entry(insn, &file->mcount_loc_list, call_node) {
-
-		loc = (unsigned long *)sec->data->d_buf + idx;
-		memset(loc, 0, sizeof(unsigned long));
-
-		if (elf_add_reloc_to_insn(file->elf, sec,
-					  idx * sizeof(unsigned long),
-					  R_X86_64_64,
-					  insn->sec, insn->offset))
-			return -1;
-
-		idx++;
-	}
-
-	return 0;
-}
-
 /*
  * Warnings shouldn't be reported for ignored functions.
  */
@@ -1013,38 +880,6 @@ __weak bool arch_is_retpoline(struct symbol *sym)
 	return false;
 }
 
-#define NEGATIVE_RELOC	((void *)-1L)
-
-static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn)
-{
-	if (insn->reloc == NEGATIVE_RELOC)
-		return NULL;
-
-	if (!insn->reloc) {
-		if (!file)
-			return NULL;
-
-		insn->reloc = find_reloc_by_dest_range(file->elf, insn->sec,
-						       insn->offset, insn->len);
-		if (!insn->reloc) {
-			insn->reloc = NEGATIVE_RELOC;
-			return NULL;
-		}
-	}
-
-	return insn->reloc;
-}
-
-static void remove_insn_ops(struct instruction *insn)
-{
-	struct stack_op *op, *tmp;
-
-	list_for_each_entry_safe(op, tmp, &insn->stack_ops, list) {
-		list_del(&op->list);
-		free(op);
-	}
-}
-
 static void annotate_call_site(struct objtool_file *file,
 			       struct instruction *insn, bool sibling)
 {
@@ -1256,17 +1091,6 @@ static int add_jump_destinations(struct objtool_file *file)
 	return 0;
 }
 
-static struct symbol *find_call_destination(struct section *sec, unsigned long offset)
-{
-	struct symbol *call_dest;
-
-	call_dest = find_func_by_offset(sec, offset);
-	if (!call_dest)
-		call_dest = find_symbol_by_offset(sec, offset);
-
-	return call_dest;
-}
-
 /*
  * Find the destination instructions for all calls.
  */
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index 6cfff078897f..d4e378c7aa30 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -79,8 +79,6 @@ static inline bool is_jump(struct instruction *insn)
 	return is_static_jump(insn) || is_dynamic_jump(insn);
 }
 
-struct instruction *find_insn(struct objtool_file *file,
-			      struct section *sec, unsigned long offset);
 
 #define for_each_insn(file, insn)					\
 	list_for_each_entry(insn, &file->insn_list, list)
diff --git a/tools/objtool/include/objtool/utils.h b/tools/objtool/include/objtool/utils.h
new file mode 100644
index 000000000000..f808a66dd0a8
--- /dev/null
+++ b/tools/objtool/include/objtool/utils.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2017 Josh Poimboeuf <jpoimboe@redhat.com>
+ */
+
+#ifndef UTILS_H
+#define UTILS_H
+
+#include <stdbool.h>
+#include <objtool/cfi.h>
+#include <objtool/arch.h>
+
+int decode_instructions(struct objtool_file *file);
+struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn);
+void remove_insn_ops(struct instruction *insn);
+struct symbol *find_call_destination(struct section *sec, unsigned long offset);
+int create_mcount_loc_sections(struct objtool_file *file);
+struct instruction *find_insn(struct objtool_file *file,
+			      struct section *sec, unsigned long offset);
+
+#define sym_for_each_insn(file, sym, insn)                              \
+	for (insn = find_insn(file, sym->sec, sym->offset);             \
+	     insn && &insn->list != &file->insn_list &&                 \
+		insn->sec == sym->sec &&                                \
+		insn->offset < sym->offset + sym->len;                  \
+	     insn = list_next_entry(insn, list))
+
+#endif /* UTILS_H */
diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index dd3c64af9db2..383c5f2f0658 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -10,6 +10,7 @@
 #include <asm/orc_types.h>
 
 #include <objtool/check.h>
+#include <objtool/utils.h>
 #include <objtool/warn.h>
 #include <objtool/endianness.h>
 
diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
new file mode 100644
index 000000000000..d1fc6a123a6e
--- /dev/null
+++ b/tools/objtool/utils.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2017 Josh Poimboeuf <jpoimboe@redhat.com>
+ */
+
+#include <string.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+
+#include <objtool/builtin.h>
+#include <objtool/check.h>
+#include <objtool/utils.h>
+#include <objtool/warn.h>
+
+#include <linux/objtool.h>
+#include <linux/hashtable.h>
+#include <linux/kernel.h>
+#include <linux/static_call_types.h>
+
+struct instruction *find_insn(struct objtool_file *file,
+			      struct section *sec, unsigned long offset)
+{
+	struct instruction *insn;
+
+	hash_for_each_possible(file->insn_hash, insn, hash, sec_offset_hash(sec, offset)) {
+		if (insn->sec == sec && insn->offset == offset)
+			return insn;
+	}
+
+	return NULL;
+}
+
+#define NEGATIVE_RELOC  ((void *)-1L)
+
+struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn)
+{
+	if (insn->reloc == NEGATIVE_RELOC)
+		return NULL;
+
+	if (!insn->reloc) {
+		if (!file)
+			return NULL;
+
+		insn->reloc = find_reloc_by_dest_range(file->elf, insn->sec,
+						       insn->offset, insn->len);
+		if (!insn->reloc) {
+			insn->reloc = NEGATIVE_RELOC;
+			return NULL;
+		}
+	}
+
+	return insn->reloc;
+}
+void remove_insn_ops(struct instruction *insn)
+{
+	struct stack_op *op, *tmp;
+
+	list_for_each_entry_safe(op, tmp, &insn->stack_ops, list) {
+		list_del(&op->list);
+		free(op);
+	}
+}
+
+
+struct symbol *find_call_destination(struct section *sec, unsigned long offset)
+{
+	struct symbol *call_dest;
+
+	call_dest = find_func_by_offset(sec, offset);
+	if (!call_dest)
+		call_dest = find_symbol_by_offset(sec, offset);
+
+	return call_dest;
+}
+
+static unsigned long nr_insns;
+
+int decode_instructions(struct objtool_file *file)
+{
+	struct section *sec;
+	struct symbol *func;
+	unsigned long offset;
+	struct instruction *insn;
+	int ret;
+
+	for_each_sec(file, sec) {
+
+		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
+			continue;
+
+		if (strcmp(sec->name, ".altinstr_replacement") &&
+		    strcmp(sec->name, ".altinstr_aux") &&
+		    strncmp(sec->name, ".discard.", 9))
+			sec->text = true;
+
+		if (!strcmp(sec->name, ".noinstr.text") ||
+		    !strcmp(sec->name, ".entry.text"))
+			sec->noinstr = true;
+
+		for (offset = 0; offset < sec->sh.sh_size; offset += insn->len) {
+			insn = malloc(sizeof(*insn));
+			if (!insn) {
+				WARN("malloc failed");
+				return -1;
+			}
+			memset(insn, 0, sizeof(*insn));
+			INIT_LIST_HEAD(&insn->alts);
+			INIT_LIST_HEAD(&insn->stack_ops);
+
+			insn->sec = sec;
+			insn->offset = offset;
+
+			ret = arch_decode_instruction(file, sec, offset,
+						      sec->sh.sh_size - offset,
+						      &insn->len, &insn->type,
+						      &insn->immediate,
+						      &insn->stack_ops);
+			if (ret)
+				goto err;
+
+			hash_add(file->insn_hash, &insn->hash, sec_offset_hash(sec, insn->offset));
+			list_add_tail(&insn->list, &file->insn_list);
+			nr_insns++;
+		}
+
+		list_for_each_entry(func, &sec->symbol_list, list) {
+			if (func->type != STT_FUNC || func->alias != func)
+				continue;
+
+			if (!find_insn(file, sec, func->offset)) {
+				WARN("%s(): can't find starting instruction",
+				     func->name);
+				return -1;
+			}
+
+			sym_for_each_insn(file, func, insn)
+				insn->func = func;
+		}
+	}
+
+	if (stats)
+		printf("nr_insns: %lu\n", nr_insns);
+
+	return 0;
+
+err:
+	free(insn);
+	return ret;
+}
+
+int create_mcount_loc_sections(struct objtool_file *file)
+{
+	struct section *sec;
+	unsigned long *loc;
+	struct instruction *insn;
+	int idx;
+
+	sec = find_section_by_name(file->elf, "__mcount_loc");
+	if (sec) {
+		INIT_LIST_HEAD(&file->mcount_loc_list);
+		WARN("file already has __mcount_loc section, skipping");
+		return 0;
+	}
+
+	if (list_empty(&file->mcount_loc_list))
+		return 0;
+
+	idx = 0;
+	list_for_each_entry(insn, &file->mcount_loc_list, call_node)
+		idx++;
+
+	sec = elf_create_section(file->elf, "__mcount_loc", 0, sizeof(unsigned long), idx);
+	if (!sec)
+		return -1;
+
+	idx = 0;
+	list_for_each_entry(insn, &file->mcount_loc_list, call_node) {
+
+		loc = (unsigned long *)sec->data->d_buf + idx;
+		memset(loc, 0, sizeof(unsigned long));
+
+		if (elf_add_reloc_to_insn(file->elf, sec,
+					  idx * sizeof(unsigned long),
+					  R_X86_64_64,
+					  insn->sec, insn->offset))
+			return -1;
+
+		idx++;
+	}
+
+	return 0;
+}
-- 
2.31.1

Re: [RFC PATCH 1/3] objtool: Move common code to utils.c

[Miroslav Benes]

> +#define sym_for_each_insn(file, sym, insn)                              \
> +	for (insn = find_insn(file, sym->sec, sym->offset);             \
> +	     insn && &insn->list != &file->insn_list &&                 \
> +		insn->sec == sym->sec &&                                \
> +		insn->offset < sym->offset + sym->len;                  \
> +	     insn = list_next_entry(insn, list))
> +
> +#endif /* UTILS_H */

Since you include <objtool/utils.h> in check.c, you can remove the 
definition of sym_for_each_insn() macro from check.c as well.

I wonder if it would make sense to move all these helper functions to 
utils.c and utils.h. Might be connected to what Josh wrote about his work 
on objtool interface.

Regards
Miroslav

[RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand

[Sathvika Vasireddy]

This patch adds 'mcount' as a subcommand to objtool, and enables
the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
is selected. Additionally, architectures can select HAVE_NOP_MCOUNT
if they choose to nop out mcount call sites. If that config option is
selected, then --mnop is passed as an option to 'objtool mcount'

Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 Makefile                                |   6 ++
 arch/x86/Kconfig                        |   3 +-
 scripts/Makefile.build                  |  12 +++
 tools/objtool/Build                     |   2 +
 tools/objtool/Makefile                  |   4 +-
 tools/objtool/builtin-mcount.c          |  74 +++++++++++++
 tools/objtool/include/objtool/builtin.h |   4 +-
 tools/objtool/include/objtool/objtool.h |   1 +
 tools/objtool/mcount.c                  | 138 ++++++++++++++++++++++++
 tools/objtool/objtool.c                 |   1 +
 tools/objtool/weak.c                    |   5 +
 11 files changed, 247 insertions(+), 3 deletions(-)
 create mode 100644 tools/objtool/builtin-mcount.c
 create mode 100644 tools/objtool/mcount.c

diff --git a/Makefile b/Makefile
index 55a30ca69350..316f7d08b30a 100644
--- a/Makefile
+++ b/Makefile
@@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC
   endif
 endif
 ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
+ ifdef CONFIG_HAVE_NOP_MCOUNT
   CC_FLAGS_USING	+= -DCC_USING_NOP_MCOUNT
+ endif
 endif
 ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
   ifdef CONFIG_HAVE_C_RECORDMCOUNT
@@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION
 prepare: tools/objtool
 endif
 
+ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
+prepare: tools/objtool
+endif
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
 prepare: tools/bpf/resolve_btfids
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9f5bd41bf660..eecfe588cd9f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -185,7 +185,8 @@ config X86
 	select HAVE_CONTEXT_TRACKING		if X86_64
 	select HAVE_CONTEXT_TRACKING_OFFSTACK	if HAVE_CONTEXT_TRACKING
 	select HAVE_C_RECORDMCOUNT
-	select HAVE_OBJTOOL_MCOUNT		if STACK_VALIDATION
+	select HAVE_OBJTOOL_MCOUNT
+	select HAVE_NOP_MCOUNT			if HAVE_OBJTOOL_MCOUNT
 	select HAVE_BUILDTIME_MCOUNT_SORT
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DMA_CONTIGUOUS
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a4b89b757287..6a09b4ba14a1 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -242,6 +242,18 @@ cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(o
 
 endif # CONFIG_STACK_VALIDATION
 
+ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
+
+objtool := $(objtree)/tools/objtool/objtool
+
+objtool_args := mcount
+objtool_args += $(if $(CONFIG_HAVE_NOP_MCOUNT), --mnop)
+
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
+
+endif # CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
+
 ifdef CONFIG_LTO_CLANG
 
 # Skip objtool for LLVM bitcode
diff --git a/tools/objtool/Build b/tools/objtool/Build
index 161fd451241a..0b9b8bd0ee92 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -2,12 +2,14 @@ objtool-y += arch/$(SRCARCH)/
 
 objtool-y += weak.o
 
+objtool-$(SUBCMD_MCOUNT) += mcount.o
 objtool-$(SUBCMD_CHECK) += check.o
 objtool-$(SUBCMD_CHECK) += special.o
 objtool-$(SUBCMD_ORC) += check.o
 objtool-$(SUBCMD_ORC) += orc_gen.o
 objtool-$(SUBCMD_ORC) += orc_dump.o
 
+objtool-y += builtin-mcount.o
 objtool-y += builtin-check.o
 objtool-y += builtin-orc.o
 objtool-y += elf.o
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 92ce4fce7bc7..8404cf696954 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -41,13 +41,15 @@ AWK = awk
 
 SUBCMD_CHECK := n
 SUBCMD_ORC := n
+SUBCMD_MCOUNT := n
 
 ifeq ($(SRCARCH),x86)
 	SUBCMD_CHECK := y
 	SUBCMD_ORC := y
+	SUBCMD_MCOUNT := y
 endif
 
-export SUBCMD_CHECK SUBCMD_ORC
+export SUBCMD_CHECK SUBCMD_ORC SUBCMD_MCOUNT
 export srctree OUTPUT CFLAGS SRCARCH AWK
 include $(srctree)/tools/build/Makefile.include
 
diff --git a/tools/objtool/builtin-mcount.c b/tools/objtool/builtin-mcount.c
new file mode 100644
index 000000000000..f203d118cffa
--- /dev/null
+++ b/tools/objtool/builtin-mcount.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <subcmd/parse-options.h>
+#include <string.h>
+#include <stdlib.h>
+#include <objtool/builtin.h>
+#include <objtool/objtool.h>
+
+bool mnop;
+
+static const char * const mcount_usage[] = {
+	"objtool mcount [<options>] file.o",
+	NULL,
+};
+
+static const char * const env_usage[] = {
+	"OBJTOOL_ARGS=\"<options>\"",
+	NULL,
+};
+
+const struct option mcount_options[] = {
+	OPT_BOOLEAN('N', "mnop", &mnop, "nop mcount call sites"),
+	OPT_END(),
+};
+
+int cmd_parse_options_mcount(int argc, const char **argv, const char * const usage[])
+{
+	const char *envv[16] = { };
+	char *env;
+	int envc;
+
+	env = getenv("OBJTOOL_ARGS");
+	if (env) {
+		envv[0] = "OBJTOOL_ARGS";
+		for (envc = 1; envc < ARRAY_SIZE(envv); ) {
+			envv[envc++] = env;
+			env = strchr(env, ' ');
+			if (!env)
+				break;
+			*env = '\0';
+			env++;
+		}
+
+		parse_options(envc, envv, mcount_options, env_usage, 0);
+	}
+
+	argc = parse_options(argc, argv, mcount_options, usage, 0);
+	if (argc != 1)
+		usage_with_options(usage, mcount_options);
+	return argc;
+}
+
+int cmd_mcount(int argc, const char **argv)
+{
+	const char *objname;
+	struct objtool_file *file;
+	int ret;
+
+	argc = cmd_parse_options_mcount(argc, argv, mcount_usage);
+	objname = argv[0];
+
+	file = objtool_open_read(objname);
+	if (!file)
+		return 1;
+
+	ret = objtool_mcount(file);
+	if (ret)
+		return ret;
+
+	if (file->elf->changed)
+		return elf_write(file->elf);
+
+	return 0;
+}
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 89ba869ed08f..d7ed6ff65729 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -9,11 +9,13 @@
 
 extern const struct option check_options[];
 extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats,
-            validate_dup, vmlinux, mcount, noinstr, backup, sls;
+	    validate_dup, vmlinux, mcount, noinstr, backup, sls, mnop;
 
 extern int cmd_parse_options(int argc, const char **argv, const char * const usage[]);
+extern int cmd_parse_options_mcount(int argc, const char **argv, const char * const usage[]);
 
 extern int cmd_check(int argc, const char **argv);
 extern int cmd_orc(int argc, const char **argv);
+extern int cmd_mcount(int argc, const char **argv);
 
 #endif /* _BUILTIN_H */
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index f99fbc6078d5..edcf28d4407c 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -41,5 +41,6 @@ void objtool_pv_add(struct objtool_file *file, int idx, struct symbol *func);
 int check(struct objtool_file *file);
 int orc_dump(const char *objname);
 int orc_create(struct objtool_file *file);
+int objtool_mcount(struct objtool_file *file);
 
 #endif /* _OBJTOOL_H */
diff --git a/tools/objtool/mcount.c b/tools/objtool/mcount.c
new file mode 100644
index 000000000000..fc3d215671bf
--- /dev/null
+++ b/tools/objtool/mcount.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <string.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+
+#include <arch/elf.h>
+#include <objtool/builtin.h>
+#include <objtool/cfi.h>
+#include <objtool/arch.h>
+#include <objtool/check.h>
+#include <objtool/utils.h>
+#include <objtool/special.h>
+#include <objtool/warn.h>
+
+#include <linux/objtool.h>
+#include <linux/hashtable.h>
+#include <linux/kernel.h>
+#include <linux/static_call_types.h>
+
+static int classify_symbols(struct objtool_file *file)
+{
+	struct section *sec;
+	struct symbol *func;
+
+	for_each_sec(file, sec) {
+		list_for_each_entry(func, &sec->symbol_list, list) {
+			if (func->bind != STB_GLOBAL)
+				continue;
+			if ((!strcmp(func->name, "__fentry__")) || (!strcmp(func->name, "_mcount")))
+				func->fentry = true;
+		}
+	}
+
+	return 0;
+}
+
+static void annotate_call_site(struct objtool_file *file,
+							   struct instruction *insn, bool sibling)
+{
+	struct reloc *reloc = insn_reloc(file, insn);
+	struct symbol *sym = insn->call_dest;
+
+	if (!sym)
+		sym = reloc->sym;
+
+	if (sym->fentry) {
+		if (sibling)
+			WARN_FUNC("Tail call to _mcount !?!?", insn->sec, insn->offset);
+		if (mnop) {
+			if (reloc) {
+				reloc->type = R_NONE;
+				elf_write_reloc(file->elf, reloc);
+			}
+			elf_write_insn(file->elf, insn->sec,
+				       insn->offset, insn->len,
+				       arch_nop_insn(insn->len));
+
+			insn->type = INSN_NOP;
+		}
+
+		list_add_tail(&insn->call_node, &file->mcount_loc_list);
+		return;
+	}
+}
+
+static void add_call_dest(struct objtool_file *file, struct instruction *insn,
+						  struct symbol *dest, bool sibling)
+{
+	insn->call_dest = dest;
+	if (!dest)
+		return;
+
+	remove_insn_ops(insn);
+
+	annotate_call_site(file, insn, sibling);
+}
+static int add_call_destinations(struct objtool_file *file)
+{
+	struct instruction *insn;
+	unsigned long dest_off;
+	struct symbol *dest;
+	struct reloc *reloc;
+
+	for_each_insn(file, insn) {
+		if (insn->type != INSN_CALL)
+			continue;
+
+		reloc = insn_reloc(file, insn);
+		if (!reloc) {
+			dest_off = arch_jump_destination(insn);
+			dest = find_call_destination(insn->sec, dest_off);
+
+			add_call_dest(file, insn, dest, false);
+
+
+		} else {
+			add_call_dest(file, insn, reloc->sym, false);
+		}
+	}
+
+	return 0;
+}
+
+static int decode_sections(struct objtool_file *file)
+{
+	int ret;
+
+	ret = decode_instructions(file);
+	if (ret)
+		return ret;
+
+	ret = classify_symbols(file);
+	if (ret)
+		return ret;
+
+	ret = add_call_destinations(file);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+
+int objtool_mcount(struct objtool_file *file)
+{
+	int ret, warnings = 0;
+
+	ret = decode_sections(file);
+	if (ret < 0)
+		return 0;
+
+	ret = create_mcount_loc_sections(file);
+	if (ret < 0)
+		return 0;
+	warnings += ret;
+	return 0;
+}
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index bdf699f6552b..e19851813d5b 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -37,6 +37,7 @@ static const char objtool_usage_string[] =
 
 static struct cmd_struct objtool_cmds[] = {
 	{"check",	cmd_check,	"Perform stack metadata validation on an object file" },
+	{"mcount",      cmd_mcount,    "Generate __mcount_loc section" },
 	{"orc",		cmd_orc,	"Generate in-place ORC unwind tables for an object file" },
 };
 
diff --git a/tools/objtool/weak.c b/tools/objtool/weak.c
index 8314e824db4a..19b2dfcacf2e 100644
--- a/tools/objtool/weak.c
+++ b/tools/objtool/weak.c
@@ -29,3 +29,8 @@ int __weak orc_create(struct objtool_file *file)
 {
 	UNSUPPORTED("orc");
 }
+
+int __weak objtool_mcount(struct objtool_file *file)
+{
+	UNSUPPORTED("mcount subcommand");
+}
-- 
2.31.1

Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand

[Christophe Leroy]

Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit :
> This patch adds 'mcount' as a subcommand to objtool, and enables
> the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
> is selected. Additionally, architectures can select HAVE_NOP_MCOUNT
> if they choose to nop out mcount call sites. If that config option is
> selected, then --mnop is passed as an option to 'objtool mcount'
> 
> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
> ---
>   Makefile                                |   6 ++
>   arch/x86/Kconfig                        |   3 +-
>   scripts/Makefile.build                  |  12 +++
>   tools/objtool/Build                     |   2 +
>   tools/objtool/Makefile                  |   4 +-
>   tools/objtool/builtin-mcount.c          |  74 +++++++++++++
>   tools/objtool/include/objtool/builtin.h |   4 +-
>   tools/objtool/include/objtool/objtool.h |   1 +
>   tools/objtool/mcount.c                  | 138 ++++++++++++++++++++++++
>   tools/objtool/objtool.c                 |   1 +
>   tools/objtool/weak.c                    |   5 +
>   11 files changed, 247 insertions(+), 3 deletions(-)
>   create mode 100644 tools/objtool/builtin-mcount.c
>   create mode 100644 tools/objtool/mcount.c
> 
> diff --git a/Makefile b/Makefile
> index 55a30ca69350..316f7d08b30a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC
>     endif
>   endif
>   ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
> + ifdef CONFIG_HAVE_NOP_MCOUNT
>     CC_FLAGS_USING	+= -DCC_USING_NOP_MCOUNT
> + endif
>   endif
>   ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>     ifdef CONFIG_HAVE_C_RECORDMCOUNT
> @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION
>   prepare: tools/objtool
>   endif
>   
> +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
> +prepare: tools/objtool
> +endif

I don't think that will work for powerpc.

See arch/powerpc/Makefile

powerpc builds the VDSO under prepare: , and powerpc has 
CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that 
seem to use objtool, allthough that looks odd to me. Must be something 
to fix somewhere.

   powerpc64-linux-gcc 
-Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc 
-I./arch/powerpc/include -I./arch/powerpc/include/generated  -I./include 
-I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi 
-I./include/uapi -I./include/generated/uapi -include 
./include/linux/compiler-version.h -include ./include/linux/kconfig.h 
-include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc 
-DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef 
-Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing 
-fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration 
-Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89 
-mbig-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv1 
-mcall-aixdesc -mcmodel=medium -mno-pointers-to-nested-functions 
-mtune=power7 -mcpu=power5 -mno-altivec -mno-vsx 
-fno-asynchronous-unwind-tables -mno-string -Wa,-maltivec -Wa,-mpower4 
-Wa,-many -mabi=elfv1 -mcall-aixdesc -mbig-endian 
-mstack-protector-guard=tls -mstack-protector-guard-reg=r13 
-fno-delete-null-pointer-checks -Wno-frame-address 
-Wno-format-truncation -Wno-format-overflow 
-Wno-address-of-packed-member -O2 -fno-allow-store-data-races 
-Wframe-larger-than=2048 -fstack-protector-strong 
-Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable 
-Wno-unused-const-variable -fno-stack-clash-protection 
-Wdeclaration-after-statement -Wvla -Wno-pointer-sign 
-Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds 
-Wno-array-bounds -Wno-stringop-overflow -Wno-restrict 
-Wno-maybe-uninitialized -Wno-alloc-size-larger-than 
-fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time 
-Werror=incompatible-pointer-types -Werror=designated-init 
-Wno-packed-not-aligned -mstack-protector-guard-offset=3200 -shared 
-fno-common -fno-builtin -nostdlib -Wl,--hash-style=both -include 
/home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c -fno-stack-protector 
-DDISABLE_BRANCH_PROFILING -ffreestanding -fasynchronous-unwind-tables 
-ffixed-r30 
-DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"' 
-DKBUILD_BASENAME='"vgettimeofday_64"' 
-DKBUILD_MODNAME='"vgettimeofday_64"' 
-D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o 
arch/powerpc/kernel/vdso/vgettimeofday-64.o 
arch/powerpc/kernel/vdso/vgettimeofday.c  ; ./tools/objtool/objtool 
mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o


> +
>   ifdef CONFIG_BPF
>   ifdef CONFIG_DEBUG_INFO_BTF
>   prepare: tools/bpf/resolve_btfids

Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand

[Naveen N. Rao]

Christophe Leroy wrote:
> 
> 
> Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit :
>> This patch adds 'mcount' as a subcommand to objtool, and enables
>> the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>> is selected. Additionally, architectures can select HAVE_NOP_MCOUNT
>> if they choose to nop out mcount call sites. If that config option is
>> selected, then --mnop is passed as an option to 'objtool mcount'
>> 
>> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
>> ---
>>   Makefile                                |   6 ++
>>   arch/x86/Kconfig                        |   3 +-
>>   scripts/Makefile.build                  |  12 +++
>>   tools/objtool/Build                     |   2 +
>>   tools/objtool/Makefile                  |   4 +-
>>   tools/objtool/builtin-mcount.c          |  74 +++++++++++++
>>   tools/objtool/include/objtool/builtin.h |   4 +-
>>   tools/objtool/include/objtool/objtool.h |   1 +
>>   tools/objtool/mcount.c                  | 138 ++++++++++++++++++++++++
>>   tools/objtool/objtool.c                 |   1 +
>>   tools/objtool/weak.c                    |   5 +
>>   11 files changed, 247 insertions(+), 3 deletions(-)
>>   create mode 100644 tools/objtool/builtin-mcount.c
>>   create mode 100644 tools/objtool/mcount.c
>> 
>> diff --git a/Makefile b/Makefile
>> index 55a30ca69350..316f7d08b30a 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC
>>     endif
>>   endif
>>   ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>> + ifdef CONFIG_HAVE_NOP_MCOUNT
>>     CC_FLAGS_USING	+= -DCC_USING_NOP_MCOUNT
>> + endif
>>   endif
>>   ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>>     ifdef CONFIG_HAVE_C_RECORDMCOUNT
>> @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION
>>   prepare: tools/objtool
>>   endif
>>   
>> +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>> +prepare: tools/objtool
>> +endif
> 
> I don't think that will work for powerpc.
> 
> See arch/powerpc/Makefile
> 
> powerpc builds the VDSO under prepare: , and powerpc has 
> CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that 
> seem to use objtool, allthough that looks odd to me. Must be something 
> to fix somewhere.
> 
>    powerpc64-linux-gcc 
> -Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc 
> -I./arch/powerpc/include -I./arch/powerpc/include/generated  -I./include 
> -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi 
> -I./include/uapi -I./include/generated/uapi -include 
> ./include/linux/compiler-version.h -include ./include/linux/kconfig.h 
> -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc 
> -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef 
> -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing 
> -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration 
> -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89 
> -mbig-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv1 
> -mcall-aixdesc -mcmodel=medium -mno-pointers-to-nested-functions 
> -mtune=power7 -mcpu=power5 -mno-altivec -mno-vsx 
> -fno-asynchronous-unwind-tables -mno-string -Wa,-maltivec -Wa,-mpower4 
> -Wa,-many -mabi=elfv1 -mcall-aixdesc -mbig-endian 
> -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 
> -fno-delete-null-pointer-checks -Wno-frame-address 
> -Wno-format-truncation -Wno-format-overflow 
> -Wno-address-of-packed-member -O2 -fno-allow-store-data-races 
> -Wframe-larger-than=2048 -fstack-protector-strong 
> -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable 
> -Wno-unused-const-variable -fno-stack-clash-protection 
> -Wdeclaration-after-statement -Wvla -Wno-pointer-sign 
> -Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds 
> -Wno-array-bounds -Wno-stringop-overflow -Wno-restrict 
> -Wno-maybe-uninitialized -Wno-alloc-size-larger-than 
> -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time 
> -Werror=incompatible-pointer-types -Werror=designated-init 
> -Wno-packed-not-aligned -mstack-protector-guard-offset=3200 -shared 
> -fno-common -fno-builtin -nostdlib -Wl,--hash-style=both -include 
> /home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c -fno-stack-protector 
> -DDISABLE_BRANCH_PROFILING -ffreestanding -fasynchronous-unwind-tables 
> -ffixed-r30 
> -DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"' 
> -DKBUILD_BASENAME='"vgettimeofday_64"' 
> -DKBUILD_MODNAME='"vgettimeofday_64"' 
> -D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o 
> arch/powerpc/kernel/vdso/vgettimeofday-64.o 
> arch/powerpc/kernel/vdso/vgettimeofday.c  ; ./tools/objtool/objtool 
> mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o

We don't enable ftrace for vdso, so I suspect objtool run above will be 
a no-op. This needs to be confirmed, of course.


- Naveen

Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand

[Christophe Leroy]

Le 21/03/2022 à 09:19, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>>
>>
>> Le 18/03/2022 à 11:51, Sathvika Vasireddy a écrit :
>>> This patch adds 'mcount' as a subcommand to objtool, and enables
>>> the same for x86. objtool is built if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>>> is selected. Additionally, architectures can select HAVE_NOP_MCOUNT
>>> if they choose to nop out mcount call sites. If that config option is
>>> selected, then --mnop is passed as an option to 'objtool mcount'
>>>
>>> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
>>> ---
>>>   Makefile                                |   6 ++
>>>   arch/x86/Kconfig                        |   3 +-
>>>   scripts/Makefile.build                  |  12 +++
>>>   tools/objtool/Build                     |   2 +
>>>   tools/objtool/Makefile                  |   4 +-
>>>   tools/objtool/builtin-mcount.c          |  74 +++++++++++++
>>>   tools/objtool/include/objtool/builtin.h |   4 +-
>>>   tools/objtool/include/objtool/objtool.h |   1 +
>>>   tools/objtool/mcount.c                  | 138 ++++++++++++++++++++++++
>>>   tools/objtool/objtool.c                 |   1 +
>>>   tools/objtool/weak.c                    |   5 +
>>>   11 files changed, 247 insertions(+), 3 deletions(-)
>>>   create mode 100644 tools/objtool/builtin-mcount.c
>>>   create mode 100644 tools/objtool/mcount.c
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 55a30ca69350..316f7d08b30a 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -846,7 +846,9 @@ ifdef CONFIG_FTRACE_MCOUNT_USE_CC
>>>     endif
>>>   endif
>>>   ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>>> + ifdef CONFIG_HAVE_NOP_MCOUNT
>>>     CC_FLAGS_USING    += -DCC_USING_NOP_MCOUNT
>>> + endif
>>>   endif
>>>   ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>>>     ifdef CONFIG_HAVE_C_RECORDMCOUNT
>>> @@ -1303,6 +1305,10 @@ ifdef CONFIG_STACK_VALIDATION
>>>   prepare: tools/objtool
>>>   endif
>>> +ifdef CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
>>> +prepare: tools/objtool
>>> +endif
>>
>> I don't think that will work for powerpc.
>>
>> See arch/powerpc/Makefile
>>
>> powerpc builds the VDSO under prepare: , and powerpc has 
>> CONFIG_HAVE_GENERIC_VDSO so there are some C files in that step that 
>> seem to use objtool, allthough that looks odd to me. Must be something 
>> to fix somewhere.
>>
>>    powerpc64-linux-gcc 
>> -Wp,-MMD,arch/powerpc/kernel/vdso/.vgettimeofday-64.o.d -nostdinc 
>> -I./arch/powerpc/include -I./arch/powerpc/include/generated  
>> -I./include -I./arch/powerpc/include/uapi 
>> -I./arch/powerpc/include/generated/uapi -I./include/uapi 
>> -I./include/generated/uapi -include ./include/linux/compiler-version.h 
>> -include ./include/linux/kconfig.h -include 
>> ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc 
>> -DHAVE_AS_ATHIGH=1 -fmacro-prefix-map=./= -Wall -Wundef 
>> -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing 
>> -fno-common -fshort-wchar -fno-PIE 
>> -Werror=implicit-function-declaration -Werror=implicit-int 
>> -Werror=return-type -Wno-format-security -std=gnu89 -mbig-endian -m64 
>> -msoft-float -pipe -mtraceback=no -mabi=elfv1 -mcall-aixdesc 
>> -mcmodel=medium -mno-pointers-to-nested-functions -mtune=power7 
>> -mcpu=power5 -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables 
>> -mno-string -Wa,-maltivec -Wa,-mpower4 -Wa,-many -mabi=elfv1 
>> -mcall-aixdesc -mbig-endian -mstack-protector-guard=tls 
>> -mstack-protector-guard-reg=r13 -fno-delete-null-pointer-checks 
>> -Wno-frame-address -Wno-format-truncation -Wno-format-overflow 
>> -Wno-address-of-packed-member -O2 -fno-allow-store-data-races 
>> -Wframe-larger-than=2048 -fstack-protector-strong 
>> -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable 
>> -Wno-unused-const-variable -fno-stack-clash-protection 
>> -Wdeclaration-after-statement -Wvla -Wno-pointer-sign 
>> -Wcast-function-type -Wno-stringop-truncation -Wno-zero-length-bounds 
>> -Wno-array-bounds -Wno-stringop-overflow -Wno-restrict 
>> -Wno-maybe-uninitialized -Wno-alloc-size-larger-than 
>> -fno-strict-overflow -fno-stack-check -fconserve-stack 
>> -Werror=date-time -Werror=incompatible-pointer-types 
>> -Werror=designated-init -Wno-packed-not-aligned 
>> -mstack-protector-guard-offset=3200 -shared -fno-common -fno-builtin 
>> -nostdlib -Wl,--hash-style=both -include 
>> /home/chleroy/linux-powerpc/lib/vdso/gettimeofday.c 
>> -fno-stack-protector -DDISABLE_BRANCH_PROFILING -ffreestanding 
>> -fasynchronous-unwind-tables -ffixed-r30 
>> -DKBUILD_MODFILE='"arch/powerpc/kernel/vdso/vgettimeofday-64"' 
>> -DKBUILD_BASENAME='"vgettimeofday_64"' 
>> -DKBUILD_MODNAME='"vgettimeofday_64"' 
>> -D__KBUILD_MODNAME=kmod_vgettimeofday_64 -c -o 
>> arch/powerpc/kernel/vdso/vgettimeofday-64.o 
>> arch/powerpc/kernel/vdso/vgettimeofday.c  ; ./tools/objtool/objtool 
>> mcount arch/powerpc/kernel/vdso/vgettimeofday-64.o
> 
> We don't enable ftrace for vdso, so I suspect objtool run above will be 
> a no-op. This needs to be confirmed, of course.
> 

I just checked without the series: recordmcount isn't run for VDSO, so 
objtool shouldn't be run either when the series is applied.

Christophe

Re: [RFC PATCH 2/3] objtool: Enable and implement 'mcount' subcommand

[Naveen N. Rao]

Christophe Leroy wrote:
> 
> 
> Le 21/03/2022 à 09:19, Naveen N. Rao a écrit :
>> Christophe Leroy wrote:
>> 
>> We don't enable ftrace for vdso, so I suspect objtool run above will be 
>> a no-op. This needs to be confirmed, of course.
>> 
> 
> I just checked without the series: recordmcount isn't run for VDSO, so 
> objtool shouldn't be run either when the series is applied.

Agree. I was only pointing out that it would be harmless, but it is good 
to ensure objtool is skipped for files/directories where ftrace isn't 
enabled. recordmcount keys off the presence of CC_FLAGS_FTRACE, and we 
should do the same for 'objtool --mcount'.


- Naveen

[RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Sathvika Vasireddy]

This patch adds powerpc specific functions required for
'objtool mcount' to work, and enables mcount for ppc.

Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 arch/powerpc/Kconfig                          |  1 +
 tools/objtool/Makefile                        |  4 ++
 tools/objtool/arch/powerpc/Build              |  1 +
 tools/objtool/arch/powerpc/decode.c           | 51 +++++++++++++++++++
 .../arch/powerpc/include/arch/cfi_regs.h      | 37 ++++++++++++++
 tools/objtool/arch/powerpc/include/arch/elf.h |  8 +++
 tools/objtool/utils.c                         | 28 ++++++++--
 7 files changed, 125 insertions(+), 5 deletions(-)
 create mode 100644 tools/objtool/arch/powerpc/Build
 create mode 100644 tools/objtool/arch/powerpc/decode.c
 create mode 100644 tools/objtool/arch/powerpc/include/arch/cfi_regs.h
 create mode 100644 tools/objtool/arch/powerpc/include/arch/elf.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b779603978e1..5ddafd3ce210 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -225,6 +225,7 @@ config PPC
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI				if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
 	select HAVE_OPTPROBES
+	select HAVE_OBJTOOL_MCOUNT
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_EVENTS_NMI		if PPC64
 	select HAVE_PERF_REGS
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 8404cf696954..06a9fcb4a0a3 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -49,6 +49,10 @@ ifeq ($(SRCARCH),x86)
 	SUBCMD_MCOUNT := y
 endif
 
+ifeq ($(SRCARCH),powerpc)
+        SUBCMD_MCOUNT := y
+endif
+
 export SUBCMD_CHECK SUBCMD_ORC SUBCMD_MCOUNT
 export srctree OUTPUT CFLAGS SRCARCH AWK
 include $(srctree)/tools/build/Makefile.include
diff --git a/tools/objtool/arch/powerpc/Build b/tools/objtool/arch/powerpc/Build
new file mode 100644
index 000000000000..3ff1f00c6a47
--- /dev/null
+++ b/tools/objtool/arch/powerpc/Build
@@ -0,0 +1 @@
+objtool-y += decode.o
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
new file mode 100644
index 000000000000..58988f88b315
--- /dev/null
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <objtool/check.h>
+#include <objtool/elf.h>
+#include <objtool/arch.h>
+#include <objtool/warn.h>
+#include <objtool/builtin.h>
+
+int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
+			    unsigned long offset, unsigned int maxlen,
+			    unsigned int *len, enum insn_type *type,
+			    unsigned long *immediate,
+			    struct list_head *ops_list)
+{
+	u32 insn;
+	unsigned int opcode;
+	u64 imm;
+
+	*immediate = imm = 0;
+	memcpy(&insn, sec->data->d_buf+offset, 4);
+	*len = 4;
+	*type = INSN_OTHER;
+
+	opcode = (insn >> 26);
+
+	switch (opcode) {
+	case 18: /* bl */
+		*type = INSN_CALL;
+		break;
+	}
+	*immediate = imm;
+	return 0;
+}
+
+unsigned long arch_dest_reloc_offset(int addend)
+{
+	return addend;
+}
+
+unsigned long arch_jump_destination(struct instruction *insn)
+{
+	return insn->offset +  insn->immediate;
+}
+
+const char *arch_nop_insn(int len)
+{
+	return NULL;
+}
diff --git a/tools/objtool/arch/powerpc/include/arch/cfi_regs.h b/tools/objtool/arch/powerpc/include/arch/cfi_regs.h
new file mode 100644
index 000000000000..1369176c8a94
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/cfi_regs.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _OBJTOOL_CFI_REGS_H
+#define _OBJTOOL_CFI_REGS_H
+
+#define CFI_SP                  1
+#define CFI_TOC                 2
+#define CFI_R3                  3
+#define CFI_R4                  4
+#define CFI_R5                  5
+#define CFI_R6                  6
+#define CFI_R7                  7
+#define CFI_R8                  8
+#define CFI_R9                  9
+#define CFI_R10                 10
+#define CFI_R14                 14
+#define CFI_R15                 15
+#define CFI_R16                 16
+#define CFI_R17                 17
+#define CFI_R18                 18
+#define CFI_R19                 19
+#define CFI_R20                 20
+#define CFI_R21                 21
+#define CFI_R22                 22
+#define CFI_R23                 23
+#define CFI_R24                 24
+#define CFI_R25                 25
+#define CFI_R26                 26
+#define CFI_R27                 27
+#define CFI_R28                 28
+#define CFI_R29                 29
+#define CFI_R30                 30
+#define CFI_R31                 31
+#define CFI_LR                  32
+#define CFI_NUM_REGS            33
+
+#endif
diff --git a/tools/objtool/arch/powerpc/include/arch/elf.h b/tools/objtool/arch/powerpc/include/arch/elf.h
new file mode 100644
index 000000000000..3c8ebb7d2a6b
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/elf.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _OBJTOOL_ARCH_ELF
+#define _OBJTOOL_ARCH_ELF
+
+#define R_NONE R_PPC_NONE
+
+#endif /* _OBJTOOL_ARCH_ELF */
diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
index d1fc6a123a6e..c9c14fa0dfd7 100644
--- a/tools/objtool/utils.c
+++ b/tools/objtool/utils.c
@@ -179,11 +179,29 @@ int create_mcount_loc_sections(struct objtool_file *file)
 		loc = (unsigned long *)sec->data->d_buf + idx;
 		memset(loc, 0, sizeof(unsigned long));
 
-		if (elf_add_reloc_to_insn(file->elf, sec,
-					  idx * sizeof(unsigned long),
-					  R_X86_64_64,
-					  insn->sec, insn->offset))
-			return -1;
+		if (file->elf->ehdr.e_machine == EM_X86_64) {
+			if (elf_add_reloc_to_insn(file->elf, sec,
+						  idx * sizeof(unsigned long),
+						  R_X86_64_64,
+						  insn->sec, insn->offset))
+				return -1;
+		}
+
+		if (file->elf->ehdr.e_machine == EM_PPC64) {
+			if (elf_add_reloc_to_insn(file->elf, sec,
+						  idx * sizeof(unsigned long),
+						  R_PPC64_ADDR64,
+						  insn->sec, insn->offset))
+				return -1;
+		}
+
+		if (file->elf->ehdr.e_machine == EM_PPC) {
+			if (elf_add_reloc_to_insn(file->elf, sec,
+						  idx * sizeof(unsigned long),
+						  R_PPC_ADDR32,
+						  insn->sec, insn->offset))
+				return -1;
+		}
 
 		idx++;
 	}
-- 
2.31.1

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Peter Zijlstra]

On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
> This patch adds powerpc specific functions required for
> 'objtool mcount' to work, and enables mcount for ppc.

I would love to see more objtool enablement for Power :-)


> diff --git a/tools/objtool/arch/powerpc/include/arch/elf.h b/tools/objtool/arch/powerpc/include/arch/elf.h
> new file mode 100644
> index 000000000000..3c8ebb7d2a6b
> --- /dev/null
> +++ b/tools/objtool/arch/powerpc/include/arch/elf.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef _OBJTOOL_ARCH_ELF
> +#define _OBJTOOL_ARCH_ELF
> +
> +#define R_NONE R_PPC_NONE
> +
> +#endif /* _OBJTOOL_ARCH_ELF */
> diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
> index d1fc6a123a6e..c9c14fa0dfd7 100644
> --- a/tools/objtool/utils.c
> +++ b/tools/objtool/utils.c
> @@ -179,11 +179,29 @@ int create_mcount_loc_sections(struct objtool_file *file)
>  		loc = (unsigned long *)sec->data->d_buf + idx;
>  		memset(loc, 0, sizeof(unsigned long));
>  
> -		if (elf_add_reloc_to_insn(file->elf, sec,
> -					  idx * sizeof(unsigned long),
> -					  R_X86_64_64,
> -					  insn->sec, insn->offset))
> -			return -1;
> +		if (file->elf->ehdr.e_machine == EM_X86_64) {
> +			if (elf_add_reloc_to_insn(file->elf, sec,
> +						  idx * sizeof(unsigned long),
> +						  R_X86_64_64,
> +						  insn->sec, insn->offset))
> +				return -1;
> +		}
> +
> +		if (file->elf->ehdr.e_machine == EM_PPC64) {
> +			if (elf_add_reloc_to_insn(file->elf, sec,
> +						  idx * sizeof(unsigned long),
> +						  R_PPC64_ADDR64,
> +						  insn->sec, insn->offset))
> +				return -1;
> +		}
> +
> +		if (file->elf->ehdr.e_machine == EM_PPC) {
> +			if (elf_add_reloc_to_insn(file->elf, sec,
> +						  idx * sizeof(unsigned long),
> +						  R_PPC_ADDR32,
> +						  insn->sec, insn->offset))
> +				return -1;
> +		}

It appears to me that repeating this code 3 times doesn't really scale
well, how about we introduce a helper like:


int elf_reloc_type_long(struct elf *elf)
{
	switch (elf->ehdr.e_machine) {
	case EM_X86_64:
		return R_X86_64_64;
	case EM_PPC64:
		return R_PPC64_ADDR64;
	case EM_PPC:
		return R_PPC_ADDR32;
	default:
		WARN("unknown machine...")
		exit(-1);
	}
}

		if (elf_add_reloc_to_insn(file->elf, sec,
					  idx * sizeof(unsigned long),
					  elf_reloc_type_long(file->elf),
					  insn->sec, insn->offset))
			return -1;

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Hi,

Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>> This patch adds powerpc specific functions required for
>> 'objtool mcount' to work, and enables mcount for ppc.
> 
> I would love to see more objtool enablement for Power :-)

I have not received this series and I can't see it on powerpc patchwork 
either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)

Did you send it to linuxppc-dev list ? If not can you resend it there ?


Is your series a replacement of scripts/recordmcount.c ?

If so, is there anything common with the following RFC:

https://lore.kernel.org/lkml/cover.1591125127.git.mhelsley@vmware.com/

Thanks
Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Michael Ellerman]

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>> This patch adds powerpc specific functions required for
>>> 'objtool mcount' to work, and enables mcount for ppc.
>> 
>> I would love to see more objtool enablement for Power :-)
>
> I have not received this series and I can't see it on powerpc patchwork 
> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>
> Did you send it to linuxppc-dev list ? If not can you resend it there ?

It is there, might have been delayed?

http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129

There are some CI failures.

cheers

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>> This patch adds powerpc specific functions required for
>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>
>>> I would love to see more objtool enablement for Power :-)
>>
>> I have not received this series and I can't see it on powerpc patchwork
>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>
>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
> 
> It is there, might have been delayed?

Yes something must have gone wrong.

Peter's and myself's responses are not at 
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220318105140.43914-4-sv@linux.ibm.com/ 
while yours and Naveen's ones are there.

Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>> This patch adds powerpc specific functions required for
>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>
>>> I would love to see more objtool enablement for Power :-)
>>
>> I have not received this series and I can't see it on powerpc patchwork
>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>
>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
> 
> It is there, might have been delayed?
> 
> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
> 
> There are some CI failures.
> 

On PPC64 we randomly get:

/bin/sh: 1: ./tools/objtool/objtool: not found
make[2]: *** [arch/powerpc/kernel/vdso/vgettimeofday-64.o] Error 127
/linux/arch/powerpc/kernel/vdso/Makefile:77: recipe for target 
'arch/powerpc/kernel/vdso/vgettimeofday-64.o' failed
make[2]: *** Deleting file 'arch/powerpc/kernel/vdso/vgettimeofday-64.o'
make[1]: *** [vdso_prepare] Error 2
make[1]: *** Waiting for unfinished jobs....
/linux/arch/powerpc/Makefile:423: recipe for target 'vdso_prepare' failed
make: *** [__sub-make] Error 2
Makefile:219: recipe for target '__sub-make' failed

This is linkely because prepare: target depends on vdso_prepare and 
prepare: target depends on objtool, but vdso_prepare: doesn't depend on 
objtool.

I'm not sure it is correct to run objtool mcount on VDSO as it is kind 
of useless. Only VDSO64 seems to call objtool, not VDSO32.

Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>> This patch adds powerpc specific functions required for
>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>
>>> I would love to see more objtool enablement for Power :-)
>>
>> I have not received this series and I can't see it on powerpc patchwork
>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>
>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
> 
> It is there, might have been delayed?
> 
> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
> 
> There are some CI failures.
> 

On PPC32 I get :

[    0.000000] ftrace: No functions to be traced?

Without this series I get:

[    0.000000] ftrace: allocating 22508 entries in 17 pages
[    0.000000] ftrace: allocated 17 pages with 2 groups


Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Le 21/03/2022 à 08:56, Christophe Leroy a écrit :
> 
> 
> Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>>> This patch adds powerpc specific functions required for
>>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>>
>>>> I would love to see more objtool enablement for Power :-)
>>>
>>> I have not received this series and I can't see it on powerpc patchwork
>>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>>
>>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
>>
>> It is there, might have been delayed?
>>
>> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
>>
>> There are some CI failures.
>>
> 
> On PPC32 I get :
> 
> [    0.000000] ftrace: No functions to be traced?
> 
> Without this series I get:
> 
> [    0.000000] ftrace: allocating 22508 entries in 17 pages
> [    0.000000] ftrace: allocated 17 pages with 2 groups
> 
> 


ppc64_defconfig on QEMU:

[    0.000000][    T0] ftrace: No functions to be traced?

Without your series:

[    0.000000][    T0] ftrace: allocating 38750 entries in 15 pages
[    0.000000][    T0] ftrace: allocated 15 pages with 4 groups
[    0.000000][    T0] trace event string verifier disabled

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Le 21/03/2022 à 09:30, Christophe Leroy a écrit :
> 
> 
> Le 21/03/2022 à 08:56, Christophe Leroy a écrit :
>>
>>
>> Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>>>> This patch adds powerpc specific functions required for
>>>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>>>
>>>>> I would love to see more objtool enablement for Power :-)
>>>>
>>>> I have not received this series and I can't see it on powerpc patchwork
>>>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>>>
>>>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
>>>
>>> It is there, might have been delayed?
>>>
>>> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
>>>
>>> There are some CI failures.
>>>
>>
>> On PPC32 I get :
>>
>> [    0.000000] ftrace: No functions to be traced?
>>
>> Without this series I get:
>>
>> [    0.000000] ftrace: allocating 22508 entries in 17 pages
>> [    0.000000] ftrace: allocated 17 pages with 2 groups
>>
>>
> 
> 
> ppc64_defconfig on QEMU:
> 
> [    0.000000][    T0] ftrace: No functions to be traced?
> 
> Without your series:
> 
> [    0.000000][    T0] ftrace: allocating 38750 entries in 15 pages
> [    0.000000][    T0] ftrace: allocated 15 pages with 4 groups
> [    0.000000][    T0] trace event string verifier disabled
> 

ppc64le_defconfig on QEMU:

[    0.000000][    T0] ftrace: allocating 37236 entries in 14 pages
[    0.000000][    T0] ftrace: allocated 14 pages with 3 groups

Without your series:

[    0.000000][    T0] ftrace: allocating 37236 entries in 14 pages
[    0.000000][    T0] ftrace: allocated 14 pages with 3 groups

So it seems it works only on Little Endian.
Works neither on PPC32 nor on PPC64 Big Endian

Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Le 21/03/2022 a 08:56, Christophe Leroy a ecrit :
> 
> 
> Le 21/03/2022 a 03:27, Michael Ellerman a ecrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> Le 18/03/2022 a 13:26, Peter Zijlstra a ecrit :
>>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>>> This patch adds powerpc specific functions required for
>>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>>
>>>> I would love to see more objtool enablement for Power :-)
>>>
>>> I have not received this series and I can't see it on powerpc patchwork
>>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>>
>>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
>>
>> It is there, might have been delayed?
>>
>> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
>>
>> There are some CI failures.
>>
> 
> On PPC32 I get :
> 
> [    0.000000] ftrace: No functions to be traced?
> 
> Without this series I get:
> 
> [    0.000000] ftrace: allocating 22508 entries in 17 pages
> [    0.000000] ftrace: allocated 17 pages with 2 groups
> 


With the changes below I managed to get a working ftrace on a PPC32 target.

Christophe

---------
From: Christophe Leroy <christophe.leroy@csgroup.eu>
Subject: [PATCH] powerpc/objtool: Set to big endian and 32 bits

Small ack to crossbuild a PPC32 kernel with a x86_64 host.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 tools/objtool/arch/powerpc/decode.c                  |  3 ++-
 tools/objtool/arch/powerpc/include/arch/endianness.h |  9 +++++++++
 tools/objtool/elf.c                                  |  4 ++--
 tools/objtool/utils.c                                | 12 +++++++-----
 4 files changed, 20 insertions(+), 8 deletions(-)
 create mode 100644 tools/objtool/arch/powerpc/include/arch/endianness.h

diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 58988f88b315..330af382e39f 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -8,6 +8,7 @@
 #include <objtool/arch.h>
 #include <objtool/warn.h>
 #include <objtool/builtin.h>
+#include <objtool/endianness.h>
 
 int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
 			    unsigned long offset, unsigned int maxlen,
@@ -20,7 +21,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 	u64 imm;
 
 	*immediate = imm = 0;
-	memcpy(&insn, sec->data->d_buf+offset, 4);
+	insn = bswap_if_needed(*(u32 *)(sec->data->d_buf + offset));
 	*len = 4;
 	*type = INSN_OTHER;
 
diff --git a/tools/objtool/arch/powerpc/include/arch/endianness.h b/tools/objtool/arch/powerpc/include/arch/endianness.h
new file mode 100644
index 000000000000..275087bfcc16
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/endianness.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _ARCH_ENDIANNESS_H
+#define _ARCH_ENDIANNESS_H
+
+#include <endian.h>
+
+#define __TARGET_BYTE_ORDER __BIG_ENDIAN
+
+#endif /* _ARCH_ENDIANNESS_H */
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 4b384c907027..433f0e327b68 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -867,7 +867,7 @@ static struct section *elf_create_rela_reloc_section(struct elf *elf, struct sec
 	strcpy(relocname, ".rela");
 	strcat(relocname, base->name);
 
-	sec = elf_create_section(elf, relocname, 0, sizeof(GElf_Rela), 0);
+	sec = elf_create_section(elf, relocname, 0, sizeof(Elf32_Rela), 0);
 	free(relocname);
 	if (!sec)
 		return NULL;
@@ -876,7 +876,7 @@ static struct section *elf_create_rela_reloc_section(struct elf *elf, struct sec
 	sec->base = base;
 
 	sec->sh.sh_type = SHT_RELA;
-	sec->sh.sh_addralign = 8;
+	sec->sh.sh_addralign = 4;
 	sec->sh.sh_link = find_section_by_name(elf, ".symtab")->idx;
 	sec->sh.sh_info = base->idx;
 	sec->sh.sh_flags = SHF_INFO_LINK;
diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
index c9c14fa0dfd7..f77695c81386 100644
--- a/tools/objtool/utils.c
+++ b/tools/objtool/utils.c
@@ -151,7 +151,7 @@ int decode_instructions(struct objtool_file *file)
 int create_mcount_loc_sections(struct objtool_file *file)
 {
 	struct section *sec;
-	unsigned long *loc;
+	unsigned int *loc;
 	struct instruction *insn;
 	int idx;
 
@@ -169,15 +169,17 @@ int create_mcount_loc_sections(struct objtool_file *file)
 	list_for_each_entry(insn, &file->mcount_loc_list, call_node)
 		idx++;
 
-	sec = elf_create_section(file->elf, "__mcount_loc", 0, sizeof(unsigned long), idx);
+	sec = elf_create_section(file->elf, "__mcount_loc", 0, sizeof(unsigned int), idx);
 	if (!sec)
 		return -1;
 
+	sec->sh.sh_addralign = 4;
+
 	idx = 0;
 	list_for_each_entry(insn, &file->mcount_loc_list, call_node) {
 
-		loc = (unsigned long *)sec->data->d_buf + idx;
-		memset(loc, 0, sizeof(unsigned long));
+		loc = (unsigned int *)sec->data->d_buf + idx;
+		memset(loc, 0, sizeof(unsigned int));
 
 		if (file->elf->ehdr.e_machine == EM_X86_64) {
 			if (elf_add_reloc_to_insn(file->elf, sec,
@@ -197,7 +199,7 @@ int create_mcount_loc_sections(struct objtool_file *file)
 
 		if (file->elf->ehdr.e_machine == EM_PPC) {
 			if (elf_add_reloc_to_insn(file->elf, sec,
-						  idx * sizeof(unsigned long),
+						  idx * sizeof(unsigned int),
 						  R_PPC_ADDR32,
 						  insn->sec, insn->offset))
 				return -1;
-- 
2.35.1

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Naveen N. Rao]

Peter Zijlstra wrote:
> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>> This patch adds powerpc specific functions required for
>> 'objtool mcount' to work, and enables mcount for ppc.
> 
> I would love to see more objtool enablement for Power :-)
> 
> 
>> diff --git a/tools/objtool/arch/powerpc/include/arch/elf.h b/tools/objtool/arch/powerpc/include/arch/elf.h
>> new file mode 100644
>> index 000000000000..3c8ebb7d2a6b
>> --- /dev/null
>> +++ b/tools/objtool/arch/powerpc/include/arch/elf.h
>> @@ -0,0 +1,8 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +
>> +#ifndef _OBJTOOL_ARCH_ELF
>> +#define _OBJTOOL_ARCH_ELF
>> +
>> +#define R_NONE R_PPC_NONE
>> +
>> +#endif /* _OBJTOOL_ARCH_ELF */
>> diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
>> index d1fc6a123a6e..c9c14fa0dfd7 100644
>> --- a/tools/objtool/utils.c
>> +++ b/tools/objtool/utils.c
>> @@ -179,11 +179,29 @@ int create_mcount_loc_sections(struct objtool_file *file)
>>  		loc = (unsigned long *)sec->data->d_buf + idx;
>>  		memset(loc, 0, sizeof(unsigned long));
>>  
>> -		if (elf_add_reloc_to_insn(file->elf, sec,
>> -					  idx * sizeof(unsigned long),
>> -					  R_X86_64_64,
>> -					  insn->sec, insn->offset))
>> -			return -1;
>> +		if (file->elf->ehdr.e_machine == EM_X86_64) {
>> +			if (elf_add_reloc_to_insn(file->elf, sec,
>> +						  idx * sizeof(unsigned long),
>> +						  R_X86_64_64,
>> +						  insn->sec, insn->offset))
>> +				return -1;
>> +		}
>> +
>> +		if (file->elf->ehdr.e_machine == EM_PPC64) {
>> +			if (elf_add_reloc_to_insn(file->elf, sec,
>> +						  idx * sizeof(unsigned long),
>> +						  R_PPC64_ADDR64,
>> +						  insn->sec, insn->offset))
>> +				return -1;
>> +		}
>> +
>> +		if (file->elf->ehdr.e_machine == EM_PPC) {
>> +			if (elf_add_reloc_to_insn(file->elf, sec,
>> +						  idx * sizeof(unsigned long),
>> +						  R_PPC_ADDR32,
>> +						  insn->sec, insn->offset))
>> +				return -1;
>> +		}
> 
> It appears to me that repeating this code 3 times doesn't really scale
> well, how about we introduce a helper like:
> 
> 
> int elf_reloc_type_long(struct elf *elf)
> {
> 	switch (elf->ehdr.e_machine) {
> 	case EM_X86_64:
> 		return R_X86_64_64;
> 	case EM_PPC64:
> 		return R_PPC64_ADDR64;
> 	case EM_PPC:
> 		return R_PPC_ADDR32;
> 	default:
> 		WARN("unknown machine...")
> 		exit(-1);
> 	}
> }
> 
> 		if (elf_add_reloc_to_insn(file->elf, sec,
> 					  idx * sizeof(unsigned long),
> 					  elf_reloc_type_long(file->elf),
> 					  insn->sec, insn->offset))
> 			return -1;

Good point. I suppose we'll need a similar helper, say, 
elf_reloc_type_none(), to replace R_NONE usage if we want to have 
objtool work for cross-arch builds.


- Naveen

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Hi Peter, Hi Josh

Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>> This patch adds powerpc specific functions required for
>> 'objtool mcount' to work, and enables mcount for ppc.
> 
> I would love to see more objtool enablement for Power :-)
> 
> 

I'm also very happy someone started to look at it.

I thought it would be more difficult to get it work on powerpc.

Iml looking forward to being able to use it and implement INLINE STATIC 
CALLs on PPC32 to start with.

I'm wondering what are the plans on your side and what we should wait 
for and what we could start with.

I could do the same as done by Sathvika for static calls, in extenso get 
it out of check.c into a standalone feature. On the other hand I 
understood that Josh is also working at making the different features of 
objtool independant, so should I wait for that ? Any idea of when it 
comes out ?

Second point is the endianess and 32/64 selection, especially when 
crossbuilding. There is already some stuff regarding endianess based on 
bswap_if_needed() but that's based on constant selection at build time 
and I couldn't find an easy way to set it conditionaly based on the 
target being built.

Regarding 32/64 selection, there is almost nothing, it's based on using 
type 'long' which means that at the time being the target and the build 
platform must both be 32 bits or 64 bits.

For both cases (endianess and 32/64) I think the solution should 
probably be to start with the fileformat of the object file being 
reworked by objtool.

What are current works in progress on objtool ? Should I wait Josh's 
changes before starting looking at all this ? Should I wait for anything 
else ?

Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Josh Poimboeuf]

On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
> Second point is the endianess and 32/64 selection, especially when 
> crossbuilding. There is already some stuff regarding endianess based on 
> bswap_if_needed() but that's based on constant selection at build time 
> and I couldn't find an easy way to set it conditionaly based on the 
> target being built.
>
> Regarding 32/64 selection, there is almost nothing, it's based on using 
> type 'long' which means that at the time being the target and the build 
> platform must both be 32 bits or 64 bits.
> 
> For both cases (endianess and 32/64) I think the solution should 
> probably be to start with the fileformat of the object file being 
> reworked by objtool.

Do we really need to detect the endianness/bitness at runtime?  Objtool
is built with the kernel, why not just build-in the same target
assumptions as the kernel itself?

> What are current works in progress on objtool ? Should I wait Josh's 
> changes before starting looking at all this ? Should I wait for anything 
> else ?

I'm not making any major changes to the code, just shuffling things
around to make the interface more modular.  I hope to have something
soon (this week).  Peter recently added a big feature (Intel IBT) which
is already in -next.

Contributions are welcome, with the understanding that you'll help
maintain it ;-)

Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
which did the full stack validation.  I'm not sure what ever became of
that.

FWIW, there have been some objtool patches for arm64 stack validation,
but the arm64 maintainers have been hesitant to get on board with
objtool, as it brings a certain maintenance burden.  Especially for the
full stack validation and ORC unwinder.  But if you only want inline
static calls and/or mcount then it'd probably be much easier to
maintain.

-- 
Josh

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Peter Zijlstra]

On Mon, Mar 28, 2022 at 12:59:20PM -0700, Josh Poimboeuf wrote:

> I'm not making any major changes to the code, just shuffling things
> around to make the interface more modular.  I hope to have something
> soon (this week).  Peter recently added a big feature (Intel IBT) which
> is already in -next.

Hit Linus' tree yesterday :-)

> Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
> which did the full stack validation.  I'm not sure what ever became of
> that.

I've also heard chatter about s390.

> FWIW, there have been some objtool patches for arm64 stack validation,
> but the arm64 maintainers have been hesitant to get on board with
> objtool, as it brings a certain maintenance burden.  Especially for the
> full stack validation and ORC unwinder.  But if you only want inline
> static calls and/or mcount then it'd probably be much easier to
> maintain.

IIRC the major stumbling block for arm64 is the whole jump-table thing.
Either they need to rely on compiler plugins to provide objtool that
data (yuck, since we support at least 2 different compilers), disable
jump-tables (yuck, for that limits code-gen just to please a tool) or
use DWARF (yuck, because build times).

There was a little talk about an impromptu 'abi' to communicate
jump-table details to objtool without going full on DWARF, but that
seems to have hit a dead end again.

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Peter Zijlstra]

+arm64 people...

On Mon, Mar 28, 2022 at 10:14:38PM +0200, Peter Zijlstra wrote:
> On Mon, Mar 28, 2022 at 12:59:20PM -0700, Josh Poimboeuf wrote:
> 
> > I'm not making any major changes to the code, just shuffling things
> > around to make the interface more modular.  I hope to have something
> > soon (this week).  Peter recently added a big feature (Intel IBT) which
> > is already in -next.
> 
> Hit Linus' tree yesterday :-)
> 
> > Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
> > which did the full stack validation.  I'm not sure what ever became of
> > that.
> 
> I've also heard chatter about s390.
> 
> > FWIW, there have been some objtool patches for arm64 stack validation,
> > but the arm64 maintainers have been hesitant to get on board with
> > objtool, as it brings a certain maintenance burden.  Especially for the
> > full stack validation and ORC unwinder.  But if you only want inline
> > static calls and/or mcount then it'd probably be much easier to
> > maintain.
> 
> IIRC the major stumbling block for arm64 is the whole jump-table thing.
> Either they need to rely on compiler plugins to provide objtool that
> data (yuck, since we support at least 2 different compilers), disable
> jump-tables (yuck, for that limits code-gen just to please a tool) or
> use DWARF (yuck, because build times).
> 
> There was a little talk about an impromptu 'abi' to communicate
> jump-table details to objtool without going full on DWARF, but that
> seems to have hit a dead end again.

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Josh Poimboeuf]

On Mon, Mar 28, 2022 at 10:14:38PM +0200, Peter Zijlstra wrote:
> > FWIW, there have been some objtool patches for arm64 stack validation,
> > but the arm64 maintainers have been hesitant to get on board with
> > objtool, as it brings a certain maintenance burden.  Especially for the
> > full stack validation and ORC unwinder.  But if you only want inline
> > static calls and/or mcount then it'd probably be much easier to
> > maintain.
> 
> IIRC the major stumbling block for arm64 is the whole jump-table thing.
> Either they need to rely on compiler plugins to provide objtool that
> data (yuck, since we support at least 2 different compilers), disable
> jump-tables (yuck, for that limits code-gen just to please a tool) or
> use DWARF (yuck, because build times).

Well yeah, that was indeed the main technical issue but I seem to
remember some arm64 maintainers not really being sold on the value of
objtool regardless.

> There was a little talk about an impromptu 'abi' to communicate
> jump-table details to objtool without going full on DWARF, but that
> seems to have hit a dead end again.

Probably my fault, not enough hours in the day...

-- 
Josh

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Michael Ellerman]

Josh Poimboeuf <jpoimboe@redhat.com> writes:
> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>> Second point is the endianess and 32/64 selection, especially when 
>> crossbuilding. There is already some stuff regarding endianess based on 
>> bswap_if_needed() but that's based on constant selection at build time 
>> and I couldn't find an easy way to set it conditionaly based on the 
>> target being built.
>>
>> Regarding 32/64 selection, there is almost nothing, it's based on using 
>> type 'long' which means that at the time being the target and the build 
>> platform must both be 32 bits or 64 bits.
>> 
>> For both cases (endianess and 32/64) I think the solution should 
>> probably be to start with the fileformat of the object file being 
>> reworked by objtool.
>
> Do we really need to detect the endianness/bitness at runtime?  Objtool
> is built with the kernel, why not just build-in the same target
> assumptions as the kernel itself?

I don't think we need runtime detection. But it will need to support
basically most combinations of objtool running as 32-bit/64-bit LE/BE
while the kernel it's analysing is 32-bit/64-bit LE/BE.

>> What are current works in progress on objtool ? Should I wait Josh's 
>> changes before starting looking at all this ? Should I wait for anything 
>> else ?
>
> I'm not making any major changes to the code, just shuffling things
> around to make the interface more modular.  I hope to have something
> soon (this week).  Peter recently added a big feature (Intel IBT) which
> is already in -next.
>
> Contributions are welcome, with the understanding that you'll help
> maintain it ;-)
>
> Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
> which did the full stack validation.  I'm not sure what ever became of
> that.

From memory he was starting to clean the patches up in late 2019, but I
guess that probably got derailed by COVID. AFAIK he never posted
anything. Maybe someone at IBM has a copy internally (Naveen?).

> FWIW, there have been some objtool patches for arm64 stack validation,
> but the arm64 maintainers have been hesitant to get on board with
> objtool, as it brings a certain maintenance burden.  Especially for the
> full stack validation and ORC unwinder.  But if you only want inline
> static calls and/or mcount then it'd probably be much easier to
> maintain.

I would like to have the stack validation, but I am also worried about
the maintenance burden.

I guess we start with mcount, which looks pretty minimal judging by this
series, and see how we go from there.

cheers

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Le 29/03/2022 à 14:01, Michael Ellerman a écrit :
> Josh Poimboeuf <jpoimboe@redhat.com> writes:
>> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>>> Second point is the endianess and 32/64 selection, especially when
>>> crossbuilding. There is already some stuff regarding endianess based on
>>> bswap_if_needed() but that's based on constant selection at build time
>>> and I couldn't find an easy way to set it conditionaly based on the
>>> target being built.
>>>
>>> Regarding 32/64 selection, there is almost nothing, it's based on using
>>> type 'long' which means that at the time being the target and the build
>>> platform must both be 32 bits or 64 bits.
>>>
>>> For both cases (endianess and 32/64) I think the solution should
>>> probably be to start with the fileformat of the object file being
>>> reworked by objtool.
>>
>> Do we really need to detect the endianness/bitness at runtime?  Objtool
>> is built with the kernel, why not just build-in the same target
>> assumptions as the kernel itself?
> 
> I don't think we need runtime detection. But it will need to support
> basically most combinations of objtool running as 32-bit/64-bit LE/BE
> while the kernel it's analysing is 32-bit/64-bit LE/BE.

Exactly, the way it is done today with a constant in 
objtool/endianness.h is too simple, we need to be able to select it 
based on kernel's config. Is there a way to get the CONFIG_ macros from 
the kernel ? If yes then we could use CONFIG_64BIT and 
CONFIG_CPU_LITTLE_ENDIAN to select the correct options in objtool.


> 
>>> What are current works in progress on objtool ? Should I wait Josh's
>>> changes before starting looking at all this ? Should I wait for anything
>>> else ?
>>
>> I'm not making any major changes to the code, just shuffling things
>> around to make the interface more modular.  I hope to have something
>> soon (this week).  Peter recently added a big feature (Intel IBT) which
>> is already in -next.
>>
>> Contributions are welcome, with the understanding that you'll help
>> maintain it ;-)
>>
>> Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
>> which did the full stack validation.  I'm not sure what ever became of
>> that.
> 
>  From memory he was starting to clean the patches up in late 2019, but I
> guess that probably got derailed by COVID. AFAIK he never posted
> anything. Maybe someone at IBM has a copy internally (Naveen?).
> 
>> FWIW, there have been some objtool patches for arm64 stack validation,
>> but the arm64 maintainers have been hesitant to get on board with
>> objtool, as it brings a certain maintenance burden.  Especially for the
>> full stack validation and ORC unwinder.  But if you only want inline
>> static calls and/or mcount then it'd probably be much easier to
>> maintain.
> 
> I would like to have the stack validation, but I am also worried about
> the maintenance burden.
> 
> I guess we start with mcount, which looks pretty minimal judging by this
> series, and see how we go from there.
> 

I'm not sure mcount is really needed as we have recordmcount, but at 
least it is an easy one to start with and as we have recordmount we can 
easily compare the results and check it works as expected.

Then it should be straight forward to provide static calls.

Then I'd like to go with uaccess blocks checks as suggested by Christoph 
at 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/a94be61f008ab29c231b805e1a97e9dab35cb0cc.1629732940.git.christophe.leroy@csgroup.eu/, 
thought it might be less easy.


Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Josh Poimboeuf]

On Tue, Mar 29, 2022 at 05:32:18PM +0000, Christophe Leroy wrote:
> 
> 
> Le 29/03/2022 à 14:01, Michael Ellerman a écrit :
> > Josh Poimboeuf <jpoimboe@redhat.com> writes:
> >> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
> >>> Second point is the endianess and 32/64 selection, especially when
> >>> crossbuilding. There is already some stuff regarding endianess based on
> >>> bswap_if_needed() but that's based on constant selection at build time
> >>> and I couldn't find an easy way to set it conditionaly based on the
> >>> target being built.
> >>>
> >>> Regarding 32/64 selection, there is almost nothing, it's based on using
> >>> type 'long' which means that at the time being the target and the build
> >>> platform must both be 32 bits or 64 bits.
> >>>
> >>> For both cases (endianess and 32/64) I think the solution should
> >>> probably be to start with the fileformat of the object file being
> >>> reworked by objtool.
> >>
> >> Do we really need to detect the endianness/bitness at runtime?  Objtool
> >> is built with the kernel, why not just build-in the same target
> >> assumptions as the kernel itself?
> > 
> > I don't think we need runtime detection. But it will need to support
> > basically most combinations of objtool running as 32-bit/64-bit LE/BE
> > while the kernel it's analysing is 32-bit/64-bit LE/BE.
> 
> Exactly, the way it is done today with a constant in 
> objtool/endianness.h is too simple, we need to be able to select it 
> based on kernel's config. Is there a way to get the CONFIG_ macros from 
> the kernel ? If yes then we could use CONFIG_64BIT and 
> CONFIG_CPU_LITTLE_ENDIAN to select the correct options in objtool.

As of now, there's no good way to get CONFIG options from the kernel.
That's pretty much by design, since objtool is meant to be a standalone
tool.  In fact there are people who've used objtool for other projects.

The objtool Makefile does at least have access to HOSTARCH/SRCARCH, but
I guess that doesn't help here.  We could maybe export the endian/bit
details in env variables to the objtool build somehow.

But, I managed to forget that objtool can already be cross-compiled for
a x86-64 target, from a 32-bit x86 LE host or a 64-bit powerpc BE host.
There are some people out there doing x86 kernel builds on such systems
who reported bugs, which were since fixed.  And the fixes were pretty
trivial, IIRC.

Libelf actually does a decent job of abstracting those details from
objtool.  So, forget what I said, it might be ok to just detect
endian/bit (and possibly even arch) at runtime like you originally
suggested.

For example bswap_if_needed() could be reworked to be a runtime check.

-- 
Josh

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Naveen N. Rao]

Christophe Leroy wrote:
> 
> 
> Le 29/03/2022 à 14:01, Michael Ellerman a écrit :
>> Josh Poimboeuf <jpoimboe@redhat.com> writes:
>>> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>>>> What are current works in progress on objtool ? Should I wait Josh's
>>>> changes before starting looking at all this ? Should I wait for anything
>>>> else ?
>>>
>>> I'm not making any major changes to the code, just shuffling things
>>> around to make the interface more modular.  I hope to have something
>>> soon (this week).  Peter recently added a big feature (Intel IBT) which
>>> is already in -next.
>>>
>>> Contributions are welcome, with the understanding that you'll help
>>> maintain it ;-)
>>>
>>> Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
>>> which did the full stack validation.  I'm not sure what ever became of
>>> that.
>> 
>>  From memory he was starting to clean the patches up in late 2019, but I
>> guess that probably got derailed by COVID. AFAIK he never posted
>> anything. Maybe someone at IBM has a copy internally (Naveen?).

Kamalesh had a WIP series to enable stack validation on powerpc. From 
what I recall, he was waiting on and/or working with the arm64 folks 
around some of the common changes needed in objtool.

>> 
>>> FWIW, there have been some objtool patches for arm64 stack validation,
>>> but the arm64 maintainers have been hesitant to get on board with
>>> objtool, as it brings a certain maintenance burden.  Especially for the
>>> full stack validation and ORC unwinder.  But if you only want inline
>>> static calls and/or mcount then it'd probably be much easier to
>>> maintain.
>> 
>> I would like to have the stack validation, but I am also worried about
>> the maintenance burden.
>> 
>> I guess we start with mcount, which looks pretty minimal judging by this
>> series, and see how we go from there.
>> 
> 
> I'm not sure mcount is really needed as we have recordmcount, but at 
> least it is an easy one to start with and as we have recordmount we can 
> easily compare the results and check it works as expected.

On the contrary, I think support for mcount in objtool is something we 
want to get going soon (hopefully, in time for v5.19) given the issues 
we are seeing with recordmcount:
- https://github.com/linuxppc/issues/issues/388
- https://lore.kernel.org/all/20220211014313.1790140-1-aik@ozlabs.ru/


- Naveen

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Hi Josh,

Le 28/03/2022 à 21:59, Josh Poimboeuf a écrit :
> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>> What are current works in progress on objtool ? Should I wait Josh's
>> changes before starting looking at all this ? Should I wait for anything
>> else ?
> 
> I'm not making any major changes to the code, just shuffling things
> around to make the interface more modular.  I hope to have something
> soon (this week).  Peter recently added a big feature (Intel IBT) which
> is already in -next.
> 

Were you able to send out something ?

Thanks
Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Josh Poimboeuf]

On Thu, May 12, 2022 at 02:52:40PM +0000, Christophe Leroy wrote:
> Hi Josh,
> 
> Le 28/03/2022 à 21:59, Josh Poimboeuf a écrit :
> > On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
> >> What are current works in progress on objtool ? Should I wait Josh's
> >> changes before starting looking at all this ? Should I wait for anything
> >> else ?
> > 
> > I'm not making any major changes to the code, just shuffling things
> > around to make the interface more modular.  I hope to have something
> > soon (this week).  Peter recently added a big feature (Intel IBT) which
> > is already in -next.
> > 
> 
> Were you able to send out something ?

Yes, the objtool rewrite is now in tip/objtool/core and linux-next.

-- 
Josh

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Christophe Leroy]

Le 12/05/2022 à 17:12, Josh Poimboeuf a écrit :
> On Thu, May 12, 2022 at 02:52:40PM +0000, Christophe Leroy wrote:
>> Hi Josh,
>>
>> Le 28/03/2022 à 21:59, Josh Poimboeuf a écrit :
>>> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>>>> What are current works in progress on objtool ? Should I wait Josh's
>>>> changes before starting looking at all this ? Should I wait for anything
>>>> else ?
>>>
>>> I'm not making any major changes to the code, just shuffling things
>>> around to make the interface more modular.  I hope to have something
>>> soon (this week).  Peter recently added a big feature (Intel IBT) which
>>> is already in -next.
>>>
>>
>> Were you able to send out something ?
> 
> Yes, the objtool rewrite is now in tip/objtool/core and linux-next.

Nice.

I gave it a try this morning, I selected HAVE_OBJTOOL and 
HAVE_OBJTOOL_MCOUNT from arch/powerpc/Kconfig


Seems like there are still some x86 arch specific stuff in common common 
code and I get the following errors.

Also, is it normal to get those functions built allthough I have not 
selected HAVE_STACK_VALIDATION ?

   CC      /home/chleroy/linux-powerpc/tools/objtool/check.o
check.c: In function 'has_valid_stack_frame':
check.c:2369:30: error: 'CFI_BP' undeclared (first use in this 
function); did you mean 'CFI_SP'?
  2369 |         if (cfi->cfa.base == CFI_BP &&
       |                              ^~~~~~
       |                              CFI_SP
check.c:2369:30: note: each undeclared identifier is reported only once 
for each function it appears in
check.c:2371:44: error: 'CFI_RA' undeclared (first use in this 
function); did you mean 'CFI_R3'?
  2371 |             check_reg_frame_pos(&cfi->regs[CFI_RA], 
-cfi->cfa.offset + 8))
       |                                            ^~~~~~
       |                                            CFI_R3
check.c: In function 'update_cfi_state':
check.c:2499:70: error: 'CFI_BP' undeclared (first use in this 
function); did you mean 'CFI_SP'?
  2499 |                         if (op->src.reg == CFI_SP && 
op->dest.reg == CFI_BP &&
       | 
       ^~~~~~
       | 
       CFI_SP
make[3]: *** [/home/chleroy/linux-powerpc/tools/build/Makefile.build:97: 
/home/chleroy/linux-powerpc/tools/objtool/check.o] Error 1
make[2]: *** [Makefile:54: 
/home/chleroy/linux-powerpc/tools/objtool/objtool-in.o] Error 2
make[1]: *** [Makefile:69: objtool] Error 2
make: *** [Makefile:1337: tools/objtool] Error 2


What would be the best approach to fix that ?

Thanks
Christophe

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Peter Zijlstra]

On Sat, May 21, 2022 at 09:38:35AM +0000, Christophe Leroy wrote:

> I gave it a try this morning, I selected HAVE_OBJTOOL and 
> HAVE_OBJTOOL_MCOUNT from arch/powerpc/Kconfig
> 
> 
> Seems like there are still some x86 arch specific stuff in common common 
> code and I get the following errors.

I'm assuming there's a metric ton of x86 specific stuff in there.
That'll take a while to clean out.

Mostly Josh's rewrite was centered around splitting out the runtime
options, but objtool is still always build with all options included,
even the ones you're not using for your arch, which is what's triggering
the problems you see here, I suppose...

> Also, is it normal to get those functions built allthough I have not 
> selected HAVE_STACK_VALIDATION ?
> 
>    CC      /home/chleroy/linux-powerpc/tools/objtool/check.o
> check.c: In function 'has_valid_stack_frame':
> check.c:2369:30: error: 'CFI_BP' undeclared (first use in this 
> function); did you mean 'CFI_SP'?
>   2369 |         if (cfi->cfa.base == CFI_BP &&
>        |                              ^~~~~~
>        |                              CFI_SP
> check.c:2369:30: note: each undeclared identifier is reported only once 
> for each function it appears in
> check.c:2371:44: error: 'CFI_RA' undeclared (first use in this 
> function); did you mean 'CFI_R3'?
>   2371 |             check_reg_frame_pos(&cfi->regs[CFI_RA], 
> -cfi->cfa.offset + 8))
>        |                                            ^~~~~~
>        |                                            CFI_R3
> check.c: In function 'update_cfi_state':
> check.c:2499:70: error: 'CFI_BP' undeclared (first use in this 
> function); did you mean 'CFI_SP'?
>   2499 |                         if (op->src.reg == CFI_SP && 
> op->dest.reg == CFI_BP &&
>        | 
>        ^~~~~~
>        | 
>        CFI_SP
> make[3]: *** [/home/chleroy/linux-powerpc/tools/build/Makefile.build:97: 
> /home/chleroy/linux-powerpc/tools/objtool/check.o] Error 1
> make[2]: *** [Makefile:54: 
> /home/chleroy/linux-powerpc/tools/objtool/objtool-in.o] Error 2
> make[1]: *** [Makefile:69: objtool] Error 2
> make: *** [Makefile:1337: tools/objtool] Error 2
> 
> 
> What would be the best approach to fix that ?

Define CFI_BP to your frame register (r2, afaict) I suppose. If you're
only using OBJTOOL_MCOUNT this code will never run, so all you have to
ensure is that it compiles, not that it makes sense (-:

The very long and complicated way would be to propagate the various
CONFIG_HAVE_* build time things to the objtool build and librally
sprinkle a lot of #ifdef around.

Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

[Naveen N. Rao]

Peter Zijlstra wrote:
> On Sat, May 21, 2022 at 09:38:35AM +0000, Christophe Leroy wrote:
> 
>> I gave it a try this morning, I selected HAVE_OBJTOOL and 
>> HAVE_OBJTOOL_MCOUNT from arch/powerpc/Kconfig
>> 
>> 
>> Seems like there are still some x86 arch specific stuff in common common 
>> code and I get the following errors.
> 
> I'm assuming there's a metric ton of x86 specific stuff in there.
> That'll take a while to clean out.
> 
> Mostly Josh's rewrite was centered around splitting out the runtime
> options, but objtool is still always build with all options included,
> even the ones you're not using for your arch, which is what's triggering
> the problems you see here, I suppose...
> 
>> Also, is it normal to get those functions built allthough I have not 
>> selected HAVE_STACK_VALIDATION ?
>> 
>>    CC      /home/chleroy/linux-powerpc/tools/objtool/check.o
>> check.c: In function 'has_valid_stack_frame':
>> check.c:2369:30: error: 'CFI_BP' undeclared (first use in this 
>> function); did you mean 'CFI_SP'?
>>   2369 |         if (cfi->cfa.base == CFI_BP &&
>>        |                              ^~~~~~
>>        |                              CFI_SP
>> check.c:2369:30: note: each undeclared identifier is reported only once 
>> for each function it appears in
>> check.c:2371:44: error: 'CFI_RA' undeclared (first use in this 
>> function); did you mean 'CFI_R3'?
>>   2371 |             check_reg_frame_pos(&cfi->regs[CFI_RA], 
>> -cfi->cfa.offset + 8))
>>        |                                            ^~~~~~
>>        |                                            CFI_R3
>> check.c: In function 'update_cfi_state':
>> check.c:2499:70: error: 'CFI_BP' undeclared (first use in this 
>> function); did you mean 'CFI_SP'?
>>   2499 |                         if (op->src.reg == CFI_SP && 
>> op->dest.reg == CFI_BP &&
>>        | 
>>        ^~~~~~
>>        | 
>>        CFI_SP
>> make[3]: *** [/home/chleroy/linux-powerpc/tools/build/Makefile.build:97: 
>> /home/chleroy/linux-powerpc/tools/objtool/check.o] Error 1
>> make[2]: *** [Makefile:54: 
>> /home/chleroy/linux-powerpc/tools/objtool/objtool-in.o] Error 2
>> make[1]: *** [Makefile:69: objtool] Error 2
>> make: *** [Makefile:1337: tools/objtool] Error 2
>> 
>> 
>> What would be the best approach to fix that ?
> 
> Define CFI_BP to your frame register (r2, afaict) I suppose. If you're
> only using OBJTOOL_MCOUNT this code will never run, so all you have to
> ensure is that it compiles, not that it makes sense (-:

Sathvika has been looking into this.

> 
> The very long and complicated way would be to propagate the various
> CONFIG_HAVE_* build time things to the objtool build and librally
> sprinkle a lot of #ifdef around.

I think there were just a couple of unrelated checks/warnings that were 
causing problems on powerpc. So, we likely won't need too many #ifdefs, 
at least for mcount purposes.

Sathvika,
Can you post what you have?


- Naveen

Re: [RFC PATCH 0/3] objtool: Add mcount sub-command

[Josh Poimboeuf]

On Fri, Mar 18, 2022 at 04:21:37PM +0530, Sathvika Vasireddy wrote:
> This patchset adds support to implement 'objtool mcount' command.
> 
> Right now, objtool is built if CONFIG_STACK_VALIDATION is enabled.
> And, '__mcount_loc' section is generated by objtool when --mcount 
> option is passed to check sub-command. 
> 
> For architectures to be able to generate '__mcount_loc' section
> without having to do stack validation, introduce 'mcount' as
> a sub-command to objtool. This way, objtool is built for mcount 
> if CONFIG_FTRACE_MCOUNT_USE_OBJTOOL is enabled. Additionally, 
> architectures can select HAVE_NOP_MCOUNT to be able to nop out
> mcount call sites.  
> 
> TODO: Enable "objtool mcount" for clang LTO builds.
> 
> Sathvika Vasireddy (3):
>   objtool: Move common code to utils.c
>   objtool: Enable and implement 'mcount' subcommand
>   objtool/mcount: Add powerpc specific functions

Hi Sathvika,

Thanks for the patches!

I have some other patches in progress which will rework the objtool
interface by modularizing the cmdline options, so that each option can
be specified either individually or in combination.  Even stack
validation itself will be its own separate option.

I think it will help your situation as well: "objtool run --mcount" will
only do '__mcount_loc' generation and nothing else.

Something like so:

$ ./objtool run --help

 Usage: objtool run [<options>] file.o

Commands (at least one required):
    -a, --uaccess         validate uaccess
    -c, --static-call     annotate static calls
    -i, --ibt             validate and annotate IBT
    -m, --mcount          generate '__mcount_loc' section
    -n, --noinstr         validate noinstr
    -o, --orc             generate ORC metadata
    -r, --retpoline       validate retpoline usage
    -S, --sls             validate straight-line-speculation mitigation
    -s, --stack-val       validate stack metadata

Options:
        --backtrace       unwind on error
        --backup          create .orig files before modification
        --dry-run         don't write the modifications
        --fp              object uses frame pointers
        --module          object will be part of a kernel module
        --no-unreachable  skip 'unreachable instruction' warnings
        --stats           print statistics
        --vmlinux         object is vmlinux.o


Hopefully I'll have the patches ready soon.

-- 
Josh