linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/3] objtool: Fixup alternate feature relative addresses
@ 2025-09-29  8:04 Sathvika Vasireddy
  2025-09-29  8:04 ` [RFC PATCH v2 1/3] objtool/powerpc: Enhance objtool to fixup " Sathvika Vasireddy
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sathvika Vasireddy @ 2025-09-29  8:04 UTC (permalink / raw)
  To: linux-kbuild, linuxppc-dev
  Cc: nathan, masahiroy, kees, naveen, jpoimboe, peterz, npiggin, maddy,
	segher, christophe.leroy, mingo, mpe, mahesh, sv

This patch series implements build-time fixup of alternate feature
relative addresses for powerpc.

Previously, Nicholas Piggin proposed a build-time solution using a
custom PowerPC tool [1], which provided the foundation for this approach.
The current implementation leverages objtool's existing ELF parsing
infrastructure to do the same.

This patchset applies atop powerpc/merge branch.

Links and History:
[1] Original PowerPC tool approach:
    http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20170521010130.13552-1-npiggin@gmail.com/
[2] Initial RFC v1:
    https://lore.kernel.org/linuxppc-dev/20240422092206.147078-1-sv@linux.ibm.com/T/#t
[3] PowerPC instruction decoder borrowed from:
    https://lore.kernel.org/linuxppc-dev/bfa8364da047d8610a09458a1cd924a0566aedbb.1736955567.git.christophe.leroy@csgroup.eu/#Z31tools:objtool:arch:powerpc:decode.c

Testing:
Build and Boot tested on ppc64le, ppc64be, and ppc32be configs.

Changes in V2:
- Added implementation support for ppc64 LE, BE, ppc32 BE.

Sathvika Vasireddy (3):
  objtool/powerpc: Enhance objtool to fixup alternate feature relative
    addresses
  kbuild: Add objtool integration for PowerPC feature fixups
  powerpc: Enable build-time feature fixup processing by default

 Makefile                                  |   7 +
 arch/Kconfig                              |   3 +
 arch/powerpc/Kconfig                      |   3 +
 arch/powerpc/Makefile                     |   5 +
 arch/powerpc/include/asm/feature-fixups.h |   2 +-
 arch/powerpc/kernel/vmlinux.lds.S         |   8 +-
 arch/powerpc/lib/feature-fixups.c         |  12 -
 scripts/Makefile.lib                      |   5 +-
 scripts/Makefile.vmlinux                  |  13 +-
 tools/objtool/arch/powerpc/decode.c       |  15 +-
 tools/objtool/arch/powerpc/special.c      | 436 ++++++++++++++++++++++
 tools/objtool/builtin-check.c             |   2 +
 tools/objtool/check.c                     |  48 ++-
 tools/objtool/elf.c                       |   4 +
 tools/objtool/include/objtool/builtin.h   |   1 +
 tools/objtool/include/objtool/special.h   |  56 +++
 16 files changed, 593 insertions(+), 27 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH v2 1/3] objtool/powerpc: Enhance objtool to fixup alternate feature relative addresses
  2025-09-29  8:04 [RFC PATCH v2 0/3] objtool: Fixup alternate feature relative addresses Sathvika Vasireddy
@ 2025-09-29  8:04 ` Sathvika Vasireddy
  2025-09-29 10:49   ` Peter Zijlstra
  2025-09-29  8:04 ` [RFC PATCH v2 2/3] kbuild: Add objtool integration for PowerPC feature fixups Sathvika Vasireddy
  2025-09-29  8:04 ` [RFC PATCH v2 3/3] powerpc: Enable build-time feature fixup processing by default Sathvika Vasireddy
  2 siblings, 1 reply; 8+ messages in thread
From: Sathvika Vasireddy @ 2025-09-29  8:04 UTC (permalink / raw)
  To: linux-kbuild, linuxppc-dev
  Cc: nathan, masahiroy, kees, naveen, jpoimboe, peterz, npiggin, maddy,
	segher, christophe.leroy, mingo, mpe, mahesh, sv

Implement build-time fixup of alternate feature relative addresses for
the out-of-line (else) patch code. Initial posting to achieve the same
using another tool can be found at [1]. Idea is to implement this using
objtool instead of introducing another tool since it already has elf
parsing and processing covered.

Introduce --ftr-fixup as an option to objtool to do feature fixup at
build-time.

Couple of issues and warnings encountered while implementing feature
fixup using objtool are as follows:

1. libelf is creating corrupted vmlinux file after writing necessary
changes to the file. Due to this, kexec is not able to load new
kernel.

It gives the following error:
        ELF Note corrupted !
        Cannot determine the file type of vmlinux

To fix this issue, after opening vmlinux file, make a call to
elf_flagelf (e, ELF_C_SET, ELF_F_LAYOUT). This instructs libelf not
to touch the segment and section layout. It informs the library
that the application will take responsibility for the layout of the
file and that the library should not insert any padding between
sections.

2. Fix can't find starting instruction warnings when run on vmlinux

Objtool throws a lot of can't find starting instruction warnings
when run on vmlinux with --ftr-fixup option.

These warnings are seen because find_insn() function looks for
instructions at offsets that are relative to the start of the section.
In case of individual object files (.o), there are no can't find
starting instruction warnings seen because the actual offset
associated with an instruction is itself a relative offset since the
sections start at offset 0x0.

However, in case of vmlinux, find_insn() function fails to find
instructions at the actual offset associated with an instruction
since the sections in vmlinux do not start at offset 0x0. Due to
this, find_insn() will look for absolute offset and not the relative
offset. This is resulting in a lot of can't find starting instruction
warnings when objtool is run on vmlinux.

To fix this, pass offset that is relative to the start of the section
to find_insn().

find_insn() is also looking for symbols of size 0. But, objtool does
not store empty STT_NOTYPE symbols in the rbtree. Due to this,
for empty symbols, objtool is throwing can't find starting
instruction warnings. Fix this by ignoring symbols that are of
size 0 since objtool does not add them to the rbtree.

3. Objtool is throwing unannotated intra-function call warnings
when run on vmlinux with --ftr-fixup option.

One such example:

vmlinux: warning: objtool: .text+0x3d94:
                        unannotated intra-function call

.text + 0x3d94 = c000000000008000 + 3d94 = c0000000000081d4

c0000000000081d4: 45 24 02 48  bl c00000000002a618
<system_reset_exception+0x8>

c00000000002a610 <system_reset_exception>:
c00000000002a610:       0e 01 4c 3c     addis   r2,r12,270
                        c00000000002a610: R_PPC64_REL16_HA    .TOC.
c00000000002a614:       f0 6c 42 38     addi    r2,r2,27888
                        c00000000002a614: R_PPC64_REL16_LO    .TOC.+0x4
c00000000002a618:       a6 02 08 7c     mflr    r0

This is happening because we should be looking for destination
symbols that are at absolute offsets instead of relative offsets.
After fixing dest_off to point to absolute offset, there are still
a lot of these warnings shown.

In the above example, objtool is computing the destination
offset to be c00000000002a618, which points to a completely
different instruction. find_call_destination() is looking for this
offset and failing. Instead, we should be looking for destination
offset c00000000002a610 which points to system_reset_exception
function.

Even after fixing the way destination offset is computed, and
after looking for dest_off - 0x8 in cases where the original offset
is not found, there are still a lot of unannotated intra-function
call warnings generated. This is due to symbols that are not
properly annotated.

So, for now, as a hack to curb these warnings, do not emit
unannotated intra-function call warnings when objtool is run
with --ftr-fixup option.

[1]
https://lore.kernel.org/linuxppc-dev/20170521010130.13552-1-npiggin@gmail.com/

Co-developed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 tools/objtool/arch/powerpc/decode.c     |  15 +-
 tools/objtool/arch/powerpc/special.c    | 436 ++++++++++++++++++++++++
 tools/objtool/builtin-check.c           |   2 +
 tools/objtool/check.c                   |  48 ++-
 tools/objtool/elf.c                     |   4 +
 tools/objtool/include/objtool/builtin.h |   1 +
 tools/objtool/include/objtool/special.h |  56 +++
 7 files changed, 553 insertions(+), 9 deletions(-)

diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index c851c51d4bd3..a508d50a54d6 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -47,13 +47,26 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 	enum insn_type typ;
 	unsigned long imm;
 	u32 ins;
+	unsigned int aa;
 
 	ins = bswap_if_needed(file->elf, *(u32 *)(sec->data->d_buf + offset));
 	opcode = ins >> 26;
 	typ = INSN_OTHER;
 	imm = 0;
+	aa = ins & 2;
 
 	switch (opcode) {
+	case 16:
+		if (ins & 1)
+			typ = INSN_OTHER;
+		else
+			typ = INSN_JUMP_CONDITIONAL;
+		imm = ins & 0xfffc;
+		if (imm & 0x8000)
+			imm -= 0x10000;
+		insn->immediate = imm | aa;
+		break;
+
 	case 18: /* b[l][a] */
 		if (ins == 0x48000005)	/* bl .+4 */
 			typ = INSN_OTHER;
@@ -65,7 +78,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 		imm = ins & 0x3fffffc;
 		if (imm & 0x2000000)
 			imm -= 0x4000000;
-		imm |= ins & 2;	/* AA flag */
+		insn->immediate = imm | aa;
 		break;
 	}
 
diff --git a/tools/objtool/arch/powerpc/special.c b/tools/objtool/arch/powerpc/special.c
index 51610689abf7..028ffe39e2a8 100644
--- a/tools/objtool/arch/powerpc/special.c
+++ b/tools/objtool/arch/powerpc/special.c
@@ -3,7 +3,17 @@
 #include <stdlib.h>
 #include <objtool/special.h>
 #include <objtool/builtin.h>
+#include <objtool/warn.h>
+#include <asm/byteorder.h>
+#include <errno.h>
 
+struct section *ftr_alt;
+
+struct fixup_entry *fes;
+unsigned int nr_fes;
+
+uint64_t fe_alt_start = -1;
+uint64_t fe_alt_end;
 
 bool arch_support_alt_relocation(struct special_alt *special_alt,
 				 struct instruction *insn,
@@ -18,3 +28,429 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
 {
 	exit(-1);
 }
+
+int process_alt_data(struct objtool_file *file)
+{
+	struct section *section;
+
+	section = find_section_by_name(file->elf, ".__ftr_alternates.text");
+	ftr_alt = section;
+
+	if (!ftr_alt) {
+		WARN(".__ftr_alternates.text section not found\n");
+		return -1;
+	}
+
+	fe_alt_start = ftr_alt->sh.sh_addr;
+	fe_alt_end = ftr_alt->sh.sh_addr + ftr_alt->sh.sh_size;
+
+	return 0;
+
+}
+
+static int is_le(struct objtool_file *file)
+{
+	return file->elf->ehdr.e_ident[EI_DATA] == ELFDATA2LSB;
+}
+
+static int is_64bit(struct objtool_file *file)
+{
+	return file->elf->ehdr.e_ident[EI_CLASS] == ELFCLASS64;
+}
+
+static uint32_t f32_to_cpu(struct objtool_file *file, uint32_t val)
+{
+	if (is_le(file))
+		return __le32_to_cpu(val);
+	else
+		return __be32_to_cpu(val);
+}
+
+static uint64_t f64_to_cpu(struct objtool_file *file, uint64_t val)
+{
+	if (is_le(file))
+		return __le64_to_cpu(val);
+	else
+		return __be64_to_cpu(val);
+}
+
+static uint32_t cpu_to_f32(struct objtool_file *file, uint32_t val)
+{
+	if (is_le(file))
+		return __cpu_to_le32(val);
+	else
+		return __cpu_to_be32(val);
+}
+
+int process_fixup_entries(struct objtool_file *file)
+{
+	struct section *sec;
+	int i;
+
+	for_each_sec(file, sec) {
+		Elf_Data *data;
+		unsigned int nr;
+
+		if (strstr(sec->name, "_ftr_fixup") == NULL)
+			continue;
+
+		if (strstr(sec->name, ".rela") != NULL)
+			continue;
+
+		data = sec->data;
+		if (!data || data->d_size == 0)
+			continue;
+
+		if (is_64bit(file))
+			nr = data->d_size / sizeof(struct fixup_entry_64);
+		else
+			nr = data->d_size / sizeof(struct fixup_entry_32);
+
+		for (i = 0; i < nr; i++) {
+			unsigned long idx;
+			unsigned long long off;
+			struct fixup_entry *dst;
+
+			if (is_64bit(file)) {
+				struct fixup_entry_64 *src;
+
+				idx = i * sizeof(struct fixup_entry_64);
+				off = sec->sh.sh_addr + data->d_off + idx;
+				src = data->d_buf + idx;
+
+				if (src->alt_start_off == src->alt_end_off)
+					continue;
+
+				fes = realloc(fes, (nr_fes + 1) * sizeof(struct fixup_entry));
+				dst = &fes[nr_fes];
+				nr_fes++;
+
+				dst->mask = f64_to_cpu(file, src->mask);
+				dst->value = f64_to_cpu(file, src->value);
+				dst->start_off = f64_to_cpu(file, src->start_off) + off;
+				dst->end_off = f64_to_cpu(file, src->end_off) + off;
+				dst->alt_start_off = f64_to_cpu(file, src->alt_start_off) + off;
+				dst->alt_end_off = f64_to_cpu(file, src->alt_end_off) + off;
+			} else {
+				struct fixup_entry_32 *src;
+
+				idx = i * sizeof(struct fixup_entry_32);
+				off = sec->sh.sh_addr + data->d_off + idx;
+				src = data->d_buf + idx;
+
+				if (src->alt_start_off == src->alt_end_off)
+					continue;
+
+				fes = realloc(fes, (nr_fes + 1) * sizeof(struct fixup_entry));
+				dst = &fes[nr_fes];
+				nr_fes++;
+
+				dst->mask = f32_to_cpu(file, src->mask);
+				dst->value = f32_to_cpu(file, src->value);
+				dst->start_off = (int32_t)f32_to_cpu(file, src->start_off) + off;
+				dst->end_off = (int32_t)f32_to_cpu(file, src->end_off) + off;
+				dst->alt_start_off = (int32_t)f32_to_cpu(file,
+								src->alt_start_off) + off;
+				dst->alt_end_off = (int32_t)f32_to_cpu(file,
+								src->alt_end_off) + off;
+			}
+		}
+	}
+	return 0;
+}
+
+struct fixup_entry *find_fe_altaddr(uint64_t addr)
+{
+	unsigned int i;
+
+	if (addr < fe_alt_start)
+		return NULL;
+	if (addr >= fe_alt_end)
+		return NULL;
+
+	for (i = 0; i < nr_fes; i++) {
+		if (addr >= fes[i].alt_start_off && addr < fes[i].alt_end_off)
+			return &fes[i];
+	}
+	return NULL;
+}
+
+int set_uncond_branch_target(uint32_t *insn,
+		const uint64_t addr, uint64_t target)
+{
+	uint32_t i = *insn;
+	int64_t offset;
+
+	offset = target;
+	if (!(i & BRANCH_ABSOLUTE))
+		offset = offset - addr;
+
+	/* Check we can represent the target in the instruction format */
+	if (offset < -0x2000000 || offset > 0x1fffffc || offset & 0x3)
+		return -EOVERFLOW;
+
+	/* Mask out the flags and target, so they don't step on each other. */
+	*insn = 0x48000000 | (i & 0x3) | (offset & 0x03FFFFFC);
+
+	return 0;
+}
+
+int set_cond_branch_target(uint32_t *insn,
+		const uint64_t addr, uint64_t target)
+{
+	uint32_t i = *insn;
+	int64_t offset;
+
+	offset = target;
+
+	if (!(i & BRANCH_ABSOLUTE))
+		offset = offset - addr;
+
+	/* Check we can represent the target in the instruction format */
+	if (offset < -0x8000 || offset > 0x7FFF || offset & 0x3)
+		return -EOVERFLOW;
+
+	/* Mask out the flags and target, so they don't step on each other. */
+	*insn = 0x40000000 | (i & 0x3FF0003) | (offset & 0xFFFC);
+
+	return 0;
+}
+
+void check_and_flatten_fixup_entries(void)
+{
+	static struct fixup_entry *fe;
+	unsigned int i;
+
+	i = nr_fes;
+	while (i) {
+		static struct fixup_entry *parent;
+		uint64_t nested_off; /* offset from start of parent */
+		uint64_t size;
+
+		i--;
+		fe = &fes[i];
+
+		parent = find_fe_altaddr(fe->start_off);
+		if (!parent) {
+			parent = find_fe_altaddr(fe->end_off);
+			continue;
+		}
+
+		size = fe->end_off - fe->start_off;
+		nested_off = fe->start_off - parent->alt_start_off;
+
+		fe->start_off = parent->start_off + nested_off;
+		fe->end_off = fe->start_off + size;
+	}
+}
+
+
+static struct symbol *find_symbol_at_address_within_section(struct section *sec,
+				unsigned long address)
+{
+	struct symbol *sym;
+
+	sec_for_each_sym(sec, sym) {
+		if (sym->sym.st_value <= address && address < sym->sym.st_value + sym->len)
+			return sym;
+	}
+
+	return NULL;
+}
+
+static int is_local_symbol(uint8_t st_other)
+{
+	return (st_other & 0x3) != 0;
+}
+
+static struct symbol *find_symbol_at_address(struct objtool_file *file,
+					unsigned long address)
+{
+	struct section *sec;
+	struct symbol *sym;
+
+	list_for_each_entry(sec, &file->elf->sections, list) {
+		sym = find_symbol_at_address_within_section(sec, address);
+		if (sym)
+			return sym;
+		}
+	return NULL;
+}
+
+int process_alt_relocations(struct objtool_file *file)
+{
+	struct section *section;
+	size_t n = 0;
+	struct reloc *relocation;
+	struct symbol *sym;
+	struct fixup_entry *fe;
+	uint64_t addr;
+	uint64_t scn_delta;
+	uint64_t dst_addr;
+	const char *insn_ptr;
+	unsigned long target;
+	struct symbol *symbol;
+	int is_local;
+	int j;
+	uint32_t new_insn;
+	uint32_t file_insn;
+	struct instruction decoded_insn = {0};
+	uint32_t *insn_ptr_raw;
+	uint32_t insn;
+
+	section = find_section_by_name(file->elf, ".rela.__ftr_alternates.text");
+	if (!section) {
+		printf(".rela.__ftr_alternates.text section not found.\n");
+		return 0;
+	}
+
+	for (j = 0; j < sec_num_entries(section); j++) {
+
+		relocation = &section->relocs[j];
+		sym = relocation->sym;
+		addr = reloc_offset(relocation);
+		target = sym->sym.st_value + reloc_addend(relocation);
+		symbol = find_symbol_at_address(file, target);
+
+		if (symbol) {
+			is_local = is_local_symbol(symbol->sym.st_other);
+			if (!is_local)
+				target = target + 0x8;
+		}
+
+		n++;
+		fe = find_fe_altaddr(addr);
+		if (!fe)
+			continue;
+
+		if (target >= fe->alt_start_off && target < fe->alt_end_off)
+			continue;
+
+		if (target >= ftr_alt->sh.sh_addr &&
+			target < ftr_alt->sh.sh_addr + ftr_alt->sh.sh_size)
+			return -1;
+
+		scn_delta = addr - ftr_alt->sh.sh_addr;
+		dst_addr = addr - fe->alt_start_off + fe->start_off;
+
+		if (arch_decode_instruction(file, ftr_alt, scn_delta, 4, &decoded_insn) < 0)
+			continue;
+
+		insn_ptr_raw = (uint32_t *)(ftr_alt->data->d_buf + scn_delta);
+		if (!insn_ptr_raw || !ftr_alt->data->d_buf)
+			continue;
+
+		insn = f32_to_cpu(file, *insn_ptr_raw);
+		new_insn = insn;
+
+		switch (decoded_insn.type) {
+		case INSN_JUMP_CONDITIONAL:
+			if (set_cond_branch_target(&new_insn, dst_addr, target) != 0)
+				continue;
+			break;
+
+		case INSN_JUMP_UNCONDITIONAL:
+			if (set_uncond_branch_target(&new_insn, dst_addr, target) != 0)
+				continue;
+			break;
+
+		case INSN_CALL:
+			if (set_uncond_branch_target(&new_insn, dst_addr, target) != 0)
+				continue;
+			break;
+		default:
+			continue;
+		}
+
+		if (new_insn == insn)
+			continue;
+
+		file_insn = cpu_to_f32(file, new_insn);
+		insn_ptr = (const char *)&file_insn;
+		elf_write_insn(file->elf, ftr_alt, scn_delta, sizeof(file_insn), insn_ptr);
+	}
+	return 0;
+}
+
+int process_exception_entries(struct objtool_file *file)
+{
+	struct section *section;
+	Elf_Data *data;
+	unsigned int nr, i;
+
+	section = find_section_by_name(file->elf, "__ex_table");
+	if (!section) {
+		printf("__ex_table section not found\n");
+		return 0;
+	}
+
+	data = section->data;
+	if (!data || data->d_size == 0)
+		return 0;
+
+	nr = data->d_size / sizeof(struct exception_entry);
+
+	for (i = 0; i < nr; i++) {
+		struct exception_entry *ex;
+		unsigned long idx;
+		uint64_t exaddr;
+		unsigned long long off;
+
+		idx = i * sizeof(struct exception_entry);
+		off = section->sh.sh_addr + data->d_off + idx;
+		ex = data->d_buf + idx;
+
+		exaddr = off + (int32_t)f32_to_cpu(file, ex->insn);
+
+		if (exaddr < fe_alt_start)
+			continue;
+		if (exaddr >= fe_alt_end)
+			continue;
+
+		return -1;
+	}
+	return 0;
+}
+
+int process_bug_entries(struct objtool_file *file)
+{
+	struct section *section;
+	Elf_Data *data;
+	unsigned int nr, i;
+
+	section = find_section_by_name(file->elf, "__bug_table");
+	if (!section) {
+		printf("__bug_table section not found\n");
+		return 0;
+	}
+
+	data = section->data;
+	if (!data || data->d_size == 0)
+		return 0;
+
+	if (data->d_size % sizeof(struct bug_entry) != 0)
+		return -1;
+
+	nr = data->d_size / sizeof(struct bug_entry);
+	for (i = 0; i < nr; i++) {
+		struct bug_entry *bug;
+		unsigned long idx;
+		uint64_t entry_addr;
+		uint64_t bugaddr;
+		int32_t bug_disp;
+
+		idx = i * sizeof(struct bug_entry);
+		entry_addr = section->sh.sh_addr + data->d_off + idx;
+		bug = (struct bug_entry *)(data->d_buf + idx);
+		bug_disp = f32_to_cpu(file, bug->bug_addr_disp);
+		bugaddr = entry_addr + bug_disp;
+
+		if (bugaddr < fe_alt_start)
+			continue;
+		if (bugaddr >= fe_alt_end)
+			continue;
+
+		return -1;
+	}
+	return 0;
+}
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 80239843e9f0..d081e07b0e93 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -75,6 +75,7 @@ static const struct option check_options[] = {
 	OPT_GROUP("Actions:"),
 	OPT_CALLBACK_OPTARG('h', "hacks", NULL, NULL, "jump_label,noinstr,skylake", "patch toolchain bugs/limitations", parse_hacks),
 	OPT_BOOLEAN('i', "ibt", &opts.ibt, "validate and annotate IBT"),
+	OPT_BOOLEAN('f', "ftr-fixup", &opts.ftr_fixup, "feature fixup"),
 	OPT_BOOLEAN('m', "mcount", &opts.mcount, "annotate mcount/fentry calls for ftrace"),
 	OPT_BOOLEAN('n', "noinstr", &opts.noinstr, "validate noinstr rules"),
 	OPT_BOOLEAN(0,   "orc", &opts.orc, "generate ORC metadata"),
@@ -160,6 +161,7 @@ static bool opts_valid(void)
 
 	if (opts.hack_jump_label	||
 	    opts.hack_noinstr		||
+	    opts.ftr_fixup		||
 	    opts.ibt			||
 	    opts.mcount			||
 	    opts.noinstr		||
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d14f20ef1db1..38f8f21051ad 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -514,7 +514,11 @@ static int decode_instructions(struct objtool_file *file)
 			if (func->embedded_insn || func->alias != func)
 				continue;
 
-			if (!find_insn(file, sec, func->offset)) {
+			if (func->len == 0)
+				continue;
+
+			if (!find_insn(file, sec, opts.ftr_fixup ?
+						func->offset - sec->sym->offset : func->offset)) {
 				ERROR("%s(): can't find starting instruction", func->name);
 				return -1;
 			}
@@ -1558,9 +1562,11 @@ static int add_jump_destinations(struct objtool_file *file)
 			    dest_off == func->offset + func->len)
 				continue;
 
-			ERROR_INSN(insn, "can't find jump dest instruction at %s+0x%lx",
-				   dest_sec->name, dest_off);
-			return -1;
+			if (!opts.ftr_fixup) {
+				ERROR_INSN(insn, "can't find jump dest instruction at %s+0x%lx",
+					   dest_sec->name, dest_off);
+				return -1;
+			}
 		}
 
 		/*
@@ -1664,7 +1670,7 @@ static int add_call_destinations(struct objtool_file *file)
 			if (func && func->ignore)
 				continue;
 
-			if (!insn_call_dest(insn)) {
+			if (!insn_call_dest(insn) && !opts.ftr_fixup) {
 				ERROR_INSN(insn, "unannotated intra-function call");
 				return -1;
 			}
@@ -2568,9 +2574,11 @@ static int decode_sections(struct objtool_file *file)
 			return ret;
 	}
 
-	ret = add_jump_destinations(file);
-	if (ret)
-		return ret;
+	if (!opts.ftr_fixup) {
+		ret = add_jump_destinations(file);
+		if (ret)
+			return ret;
+	}
 
 	/*
 	 * Must be before add_call_destination(); it changes INSN_CALL to
@@ -4698,6 +4706,30 @@ int check(struct objtool_file *file)
 	if (!nr_insns)
 		goto out;
 
+	if (opts.ftr_fixup) {
+		ret = process_alt_data(file);
+		if (ret < 0)
+			return ret;
+
+		ret = process_fixup_entries(file);
+		if (ret < 0)
+			return ret;
+
+		check_and_flatten_fixup_entries();
+
+		ret = process_exception_entries(file);
+		if (ret < 0)
+			return ret;
+
+		ret = process_bug_entries(file);
+		if (ret < 0)
+			return ret;
+
+		ret = process_alt_relocations(file);
+		if (ret < 0)
+			return ret;
+	}
+
 	if (opts.retpoline)
 		warnings += validate_retpoline(file);
 
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index ca5d77db692a..860c0cde1b11 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -1052,6 +1052,10 @@ struct elf *elf_open_read(const char *name, int flags)
 		cmd = ELF_C_WRITE;
 
 	elf->elf = elf_begin(elf->fd, cmd, NULL);
+
+	if (opts.ftr_fixup)
+		elf_flagelf(elf->elf, ELF_C_SET, ELF_F_LAYOUT);
+
 	if (!elf->elf) {
 		ERROR_ELF("elf_begin");
 		goto err;
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 6b08666fa69d..629af3c22f56 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -11,6 +11,7 @@ struct opts {
 	/* actions: */
 	bool dump_orc;
 	bool hack_jump_label;
+	bool ftr_fixup;
 	bool hack_noinstr;
 	bool hack_skylake;
 	bool ibt;
diff --git a/tools/objtool/include/objtool/special.h b/tools/objtool/include/objtool/special.h
index 72d09c0adf1a..407bb87a26ad 100644
--- a/tools/objtool/include/objtool/special.h
+++ b/tools/objtool/include/objtool/special.h
@@ -12,6 +12,41 @@
 
 #define C_JUMP_TABLE_SECTION ".data.rel.ro.c_jump_table"
 
+#define BRANCH_SET_LINK 0x1
+#define BRANCH_ABSOLUTE 0x2
+
+struct bug_entry {
+	int32_t  bug_addr_disp;
+	int32_t  file_disp;
+	uint16_t line;
+	uint16_t flags;
+};
+
+struct exception_entry {
+	int32_t insn;
+	int32_t fixup;
+};
+
+struct fixup_entry_64 {
+	uint64_t mask;
+	uint64_t value;
+	uint64_t start_off;
+	uint64_t end_off;
+	uint64_t alt_start_off;
+	uint64_t alt_end_off;
+};
+
+#define fixup_entry fixup_entry_64
+
+struct fixup_entry_32 {
+	uint32_t mask;
+	uint32_t value;
+	uint32_t start_off;
+	uint32_t end_off;
+	uint32_t alt_start_off;
+	uint32_t alt_end_off;
+};
+
 struct special_alt {
 	struct list_head list;
 
@@ -28,6 +63,8 @@ struct special_alt {
 	unsigned int orig_len, new_len; /* group only */
 };
 
+int process_alt_data(struct objtool_file *file);
+
 int special_get_alts(struct elf *elf, struct list_head *alts);
 
 void arch_handle_alternative(struct special_alt *alt);
@@ -38,4 +75,23 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
 struct reloc *arch_find_switch_table(struct objtool_file *file,
 				     struct instruction *insn,
 				     unsigned long *table_size);
+
+int process_fixup_entries(struct objtool_file *file);
+
+void check_and_flatten_fixup_entries(void);
+
+int process_exception_entries(struct objtool_file *file);
+
+int process_bug_entries(struct objtool_file *file);
+
+int process_alt_relocations(struct objtool_file *file);
+
+struct fixup_entry *find_fe_altaddr(uint64_t addr);
+
+int set_uncond_branch_target(uint32_t *insn,
+		const uint64_t addr, uint64_t target);
+
+int set_cond_branch_target(uint32_t *insn,
+		const uint64_t addr, uint64_t target);
+
 #endif /* _SPECIAL_H */
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH v2 2/3] kbuild: Add objtool integration for PowerPC feature fixups
  2025-09-29  8:04 [RFC PATCH v2 0/3] objtool: Fixup alternate feature relative addresses Sathvika Vasireddy
  2025-09-29  8:04 ` [RFC PATCH v2 1/3] objtool/powerpc: Enhance objtool to fixup " Sathvika Vasireddy
@ 2025-09-29  8:04 ` Sathvika Vasireddy
  2025-10-04 19:43   ` Nicolas Schier
  2025-09-29  8:04 ` [RFC PATCH v2 3/3] powerpc: Enable build-time feature fixup processing by default Sathvika Vasireddy
  2 siblings, 1 reply; 8+ messages in thread
From: Sathvika Vasireddy @ 2025-09-29  8:04 UTC (permalink / raw)
  To: linux-kbuild, linuxppc-dev
  Cc: nathan, masahiroy, kees, naveen, jpoimboe, peterz, npiggin, maddy,
	segher, christophe.leroy, mingo, mpe, mahesh, sv

Add build system support for PowerPC feature fixup processing:

- Add HAVE_OBJTOOL_FTR_FIXUP config option for architectures that support
  build-time feature fixup processing
- Integrate objtool feature fixup processing into vmlinux build

Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 Makefile                 |  7 +++++++
 arch/Kconfig             |  3 +++
 scripts/Makefile.lib     |  5 +++--
 scripts/Makefile.vmlinux | 13 ++++++++++++-
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index cf37b9407821..aa54e4348c8c 100644
--- a/Makefile
+++ b/Makefile
@@ -1419,6 +1419,13 @@ ifdef CONFIG_OBJTOOL
 prepare: tools/objtool
 endif
 
+# CONFIG_OBJTOOL and CONFIG_HAVE_OBJTOOL_FTR_FIXUP are unrelated, separate
+# options. It was integrated in objtool in order to borrow the elf parser,
+# but this is different from how the other objtool commands are used.
+ifdef CONFIG_HAVE_OBJTOOL_FTR_FIXUP
+prepare: tools/objtool
+endif
+
 ifdef CONFIG_BPF
 ifdef CONFIG_DEBUG_INFO_BTF
 prepare: tools/bpf/resolve_btfids
diff --git a/arch/Kconfig b/arch/Kconfig
index d1b4ffd6e085..d870aab17cba 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1334,6 +1334,9 @@ config HAVE_UACCESS_VALIDATION
 	bool
 	select OBJTOOL
 
+config HAVE_OBJTOOL_FTR_FIXUP
+	bool
+
 config HAVE_STACK_VALIDATION
 	bool
 	help
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1d581ba5df66..c77d4ceff2cc 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -169,10 +169,10 @@ cpp_flags      = -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
 
 ld_flags       = $(KBUILD_LDFLAGS) $(ldflags-y) $(LDFLAGS_$(@F))
 
-ifdef CONFIG_OBJTOOL
-
 objtool := $(objtree)/tools/objtool/objtool
 
+ifdef CONFIG_OBJTOOL
+
 objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HACK)		+= --hacks=jump_label
 objtool-args-$(CONFIG_HAVE_NOINSTR_HACK)		+= --hacks=noinstr
 objtool-args-$(CONFIG_MITIGATION_CALL_DEPTH_TRACKING)	+= --hacks=skylake
@@ -200,6 +200,7 @@ objtool-args = $(objtool-args-y)					\
 delay-objtool := $(or $(CONFIG_LTO_CLANG),$(CONFIG_X86_KERNEL_IBT))
 
 cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool-args) $@)
+
 cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
 
 objtool-enabled := y
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index b64862dc6f08..94cc2bba929a 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -84,7 +84,8 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
 # Final link of vmlinux with optional arch pass after final link
 cmd_link_vmlinux =							\
 	$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)" "$@";	\
-	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
+	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)	\
+	$(cmd_objtool_vmlinux)
 
 targets += $(vmlinux-final)
 $(vmlinux-final): scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
@@ -131,3 +132,13 @@ existing-targets := $(wildcard $(sort $(targets)))
 -include $(foreach f,$(existing-targets),$(dir $(f)).$(notdir $(f)).cmd)
 
 .PHONY: $(PHONY)
+
+# objtool for vmlinux
+# ----------------------------------
+#
+#  For feature fixup, objtool does not run on individual
+#  translation units. Run this on vmlinux instead.
+
+ifdef CONFIG_HAVE_OBJTOOL_FTR_FIXUP
+cmd_objtool_vmlinux = ; $(objtool) --ftr-fixup --link $@
+endif
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH v2 3/3] powerpc: Enable build-time feature fixup processing by default
  2025-09-29  8:04 [RFC PATCH v2 0/3] objtool: Fixup alternate feature relative addresses Sathvika Vasireddy
  2025-09-29  8:04 ` [RFC PATCH v2 1/3] objtool/powerpc: Enhance objtool to fixup " Sathvika Vasireddy
  2025-09-29  8:04 ` [RFC PATCH v2 2/3] kbuild: Add objtool integration for PowerPC feature fixups Sathvika Vasireddy
@ 2025-09-29  8:04 ` Sathvika Vasireddy
  2 siblings, 0 replies; 8+ messages in thread
From: Sathvika Vasireddy @ 2025-09-29  8:04 UTC (permalink / raw)
  To: linux-kbuild, linuxppc-dev
  Cc: nathan, masahiroy, kees, naveen, jpoimboe, peterz, npiggin, maddy,
	segher, christophe.leroy, mingo, mpe, mahesh, sv

Enable HAVE_OBJTOOL_FTR_FIXUP by default on PowerPC architecture.

- Remove runtime branch translation logic from patch_alt_instruction()
- Add --emit-relocs linker flags for post-link fixup processing
- Update ftr_alt section attributes to include executable flag

Co-developed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 arch/powerpc/Kconfig                      |  3 +++
 arch/powerpc/Makefile                     |  5 +++++
 arch/powerpc/include/asm/feature-fixups.h |  2 +-
 arch/powerpc/kernel/vmlinux.lds.S         |  8 ++++++--
 arch/powerpc/lib/feature-fixups.c         | 12 ------------
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 325c1171894d..450b5822786d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -23,6 +23,9 @@ config 64BIT
 	bool
 	default y if PPC64
 
+config HAVE_OBJTOOL_FTR_FIXUP
+        def_bool y
+
 config LIVEPATCH_64
 	def_bool PPC64
 	depends on LIVEPATCH
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a58b1029592c..8e1dab5f3c9a 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -105,6 +105,11 @@ LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie --no-dynamic-linker
 LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
 LDFLAGS_vmlinux	:= $(LDFLAGS_vmlinux-y)
 
+# --emit-relocs required for post-link fixup of alternate feature
+# text section relocations.
+LDFLAGS_vmlinux        += --emit-relocs
+KBUILD_LDFLAGS_MODULE += --emit-relocs
+
 ifdef CONFIG_PPC64
 ifndef CONFIG_PPC_KERNEL_PCREL
 	# -mcmodel=medium breaks modules because it uses 32bit offsets from
diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index 756a6c694018..d6ae92a292ec 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -32,7 +32,7 @@
 
 #define FTR_SECTION_ELSE_NESTED(label)			\
 label##2:						\
-	.pushsection __ftr_alt_##label,"a";		\
+	.pushsection __ftr_alt_##label, "ax";		\
 	.align 2;					\
 label##3:
 
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index de6ee7d35cff..961ef49f8bd3 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -99,8 +99,8 @@ SECTIONS
 	.text : AT(ADDR(.text) - LOAD_OFFSET) {
 		ALIGN_FUNCTION();
 #endif
-		/* careful! __ftr_alt_* sections need to be close to .text */
-		*(.text.hot .text.hot.* TEXT_MAIN .text.fixup .text.unlikely .text.unlikely.* .fixup __ftr_alt_* .ref.text);
+		*(.text.hot .text.hot.* TEXT_MAIN .text.fixup .text.unlikely
+			.text.unlikely.* .fixup .ref.text);
 		*(.tramp.ftrace.text);
 		NOINSTR_TEXT
 		SCHED_TEXT
@@ -267,6 +267,10 @@ SECTIONS
 		_einittext = .;
 	} :text
 
+	.__ftr_alternates.text : AT(ADDR(.__ftr_alternates.text) - LOAD_OFFSET) {
+		*(__ftr_alt*);
+	}
+
 	/* .exit.text is discarded at runtime, not link time,
 	 * to deal with references from __bug_table
 	 */
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 587c8cf1230f..269e992b1631 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -53,22 +53,10 @@ static u32 *calc_addr(struct fixup_entry *fcur, long offset)
 
 static int patch_alt_instruction(u32 *src, u32 *dest, u32 *alt_start, u32 *alt_end)
 {
-	int err;
 	ppc_inst_t instr;
 
 	instr = ppc_inst_read(src);
 
-	if (instr_is_relative_branch(ppc_inst_read(src))) {
-		u32 *target = (u32 *)branch_target(src);
-
-		/* Branch within the section doesn't need translating */
-		if (target < alt_start || target > alt_end) {
-			err = translate_branch(&instr, dest, src);
-			if (err)
-				return 1;
-		}
-	}
-
 	raw_patch_instruction(dest, instr);
 
 	return 0;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 1/3] objtool/powerpc: Enhance objtool to fixup alternate feature relative addresses
  2025-09-29  8:04 ` [RFC PATCH v2 1/3] objtool/powerpc: Enhance objtool to fixup " Sathvika Vasireddy
@ 2025-09-29 10:49   ` Peter Zijlstra
  2025-09-29 16:59     ` Sathvika Vasireddy
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2025-09-29 10:49 UTC (permalink / raw)
  To: Sathvika Vasireddy
  Cc: linux-kbuild, linuxppc-dev, nathan, masahiroy, kees, naveen,
	jpoimboe, npiggin, maddy, segher, christophe.leroy, mingo, mpe,
	mahesh

On Mon, Sep 29, 2025 at 01:34:54PM +0530, Sathvika Vasireddy wrote:
> Implement build-time fixup of alternate feature relative addresses for
> the out-of-line (else) patch code. Initial posting to achieve the same
> using another tool can be found at [1]. Idea is to implement this using
> objtool instead of introducing another tool since it already has elf
> parsing and processing covered.
> 
> Introduce --ftr-fixup as an option to objtool to do feature fixup at
> build-time.
> 
> Couple of issues and warnings encountered while implementing feature
> fixup using objtool are as follows:
> 
> 1. libelf is creating corrupted vmlinux file after writing necessary
> changes to the file. Due to this, kexec is not able to load new
> kernel.
> 
> It gives the following error:
>         ELF Note corrupted !
>         Cannot determine the file type of vmlinux
> 
> To fix this issue, after opening vmlinux file, make a call to
> elf_flagelf (e, ELF_C_SET, ELF_F_LAYOUT). This instructs libelf not
> to touch the segment and section layout. It informs the library
> that the application will take responsibility for the layout of the
> file and that the library should not insert any padding between
> sections.
> 
> 2. Fix can't find starting instruction warnings when run on vmlinux
> 
> Objtool throws a lot of can't find starting instruction warnings
> when run on vmlinux with --ftr-fixup option.
> 
> These warnings are seen because find_insn() function looks for
> instructions at offsets that are relative to the start of the section.
> In case of individual object files (.o), there are no can't find
> starting instruction warnings seen because the actual offset
> associated with an instruction is itself a relative offset since the
> sections start at offset 0x0.
> 
> However, in case of vmlinux, find_insn() function fails to find
> instructions at the actual offset associated with an instruction
> since the sections in vmlinux do not start at offset 0x0. Due to
> this, find_insn() will look for absolute offset and not the relative
> offset. This is resulting in a lot of can't find starting instruction
> warnings when objtool is run on vmlinux.
> 
> To fix this, pass offset that is relative to the start of the section
> to find_insn().
> 
> find_insn() is also looking for symbols of size 0. But, objtool does
> not store empty STT_NOTYPE symbols in the rbtree. Due to this,
> for empty symbols, objtool is throwing can't find starting
> instruction warnings. Fix this by ignoring symbols that are of
> size 0 since objtool does not add them to the rbtree.
> 
> 3. Objtool is throwing unannotated intra-function call warnings
> when run on vmlinux with --ftr-fixup option.
> 
> One such example:
> 
> vmlinux: warning: objtool: .text+0x3d94:
>                         unannotated intra-function call
> 
> .text + 0x3d94 = c000000000008000 + 3d94 = c0000000000081d4
> 
> c0000000000081d4: 45 24 02 48  bl c00000000002a618
> <system_reset_exception+0x8>
> 
> c00000000002a610 <system_reset_exception>:
> c00000000002a610:       0e 01 4c 3c     addis   r2,r12,270
>                         c00000000002a610: R_PPC64_REL16_HA    .TOC.
> c00000000002a614:       f0 6c 42 38     addi    r2,r2,27888
>                         c00000000002a614: R_PPC64_REL16_LO    .TOC.+0x4
> c00000000002a618:       a6 02 08 7c     mflr    r0
> 
> This is happening because we should be looking for destination
> symbols that are at absolute offsets instead of relative offsets.
> After fixing dest_off to point to absolute offset, there are still
> a lot of these warnings shown.
> 
> In the above example, objtool is computing the destination
> offset to be c00000000002a618, which points to a completely
> different instruction. find_call_destination() is looking for this
> offset and failing. Instead, we should be looking for destination
> offset c00000000002a610 which points to system_reset_exception
> function.
> 
> Even after fixing the way destination offset is computed, and
> after looking for dest_off - 0x8 in cases where the original offset
> is not found, there are still a lot of unannotated intra-function
> call warnings generated. This is due to symbols that are not
> properly annotated.
> 
> So, for now, as a hack to curb these warnings, do not emit
> unannotated intra-function call warnings when objtool is run
> with --ftr-fixup option.

Should not all those fixes be split out into separate patches? Also,
Changelog seems to have lost the bit where you explain *why* you need
this. IIRC Nick's original tool had a description of why this is needed.

Also, please see:

  https://lkml.kernel.org/r/9500b90c4182b03da59472e1a27876818610b084.1758067942.git.jpoimboe@kernel.org

  https://lkml.kernel.org/r/457c2e84b81bd6515aaa60ec8e9e0cc892ed7afa.1758067942.git.jpoimboe@kernel.org




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 1/3] objtool/powerpc: Enhance objtool to fixup alternate feature relative addresses
  2025-09-29 10:49   ` Peter Zijlstra
@ 2025-09-29 16:59     ` Sathvika Vasireddy
  0 siblings, 0 replies; 8+ messages in thread
From: Sathvika Vasireddy @ 2025-09-29 16:59 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kbuild, linuxppc-dev, nathan, masahiroy, kees, naveen,
	jpoimboe, npiggin, maddy, segher, christophe.leroy, mingo, mpe,
	mahesh, Sathvika Vasireddy

Hi Peter,

On 9/29/25 4:19 PM, Peter Zijlstra wrote:
> On Mon, Sep 29, 2025 at 01:34:54PM +0530, Sathvika Vasireddy wrote:
>> Implement build-time fixup of alternate feature relative addresses for
>> the out-of-line (else) patch code. Initial posting to achieve the same
>> using another tool can be found at [1]. Idea is to implement this using
>> objtool instead of introducing another tool since it already has elf
>> parsing and processing covered.
>>
>> Introduce --ftr-fixup as an option to objtool to do feature fixup at
>> build-time.
>>
>> Couple of issues and warnings encountered while implementing feature
>> fixup using objtool are as follows:
>>
>> 1. libelf is creating corrupted vmlinux file after writing necessary
>> changes to the file. Due to this, kexec is not able to load new
>> kernel.
>>
>> It gives the following error:
>>          ELF Note corrupted !
>>          Cannot determine the file type of vmlinux
>>
>> To fix this issue, after opening vmlinux file, make a call to
>> elf_flagelf (e, ELF_C_SET, ELF_F_LAYOUT). This instructs libelf not
>> to touch the segment and section layout. It informs the library
>> that the application will take responsibility for the layout of the
>> file and that the library should not insert any padding between
>> sections.
>>
>> 2. Fix can't find starting instruction warnings when run on vmlinux
>>
>> Objtool throws a lot of can't find starting instruction warnings
>> when run on vmlinux with --ftr-fixup option.
>>
>> These warnings are seen because find_insn() function looks for
>> instructions at offsets that are relative to the start of the section.
>> In case of individual object files (.o), there are no can't find
>> starting instruction warnings seen because the actual offset
>> associated with an instruction is itself a relative offset since the
>> sections start at offset 0x0.
>>
>> However, in case of vmlinux, find_insn() function fails to find
>> instructions at the actual offset associated with an instruction
>> since the sections in vmlinux do not start at offset 0x0. Due to
>> this, find_insn() will look for absolute offset and not the relative
>> offset. This is resulting in a lot of can't find starting instruction
>> warnings when objtool is run on vmlinux.
>>
>> To fix this, pass offset that is relative to the start of the section
>> to find_insn().
>>
>> find_insn() is also looking for symbols of size 0. But, objtool does
>> not store empty STT_NOTYPE symbols in the rbtree. Due to this,
>> for empty symbols, objtool is throwing can't find starting
>> instruction warnings. Fix this by ignoring symbols that are of
>> size 0 since objtool does not add them to the rbtree.
>>
>> 3. Objtool is throwing unannotated intra-function call warnings
>> when run on vmlinux with --ftr-fixup option.
>>
>> One such example:
>>
>> vmlinux: warning: objtool: .text+0x3d94:
>>                          unannotated intra-function call
>>
>> .text + 0x3d94 = c000000000008000 + 3d94 = c0000000000081d4
>>
>> c0000000000081d4: 45 24 02 48  bl c00000000002a618
>> <system_reset_exception+0x8>
>>
>> c00000000002a610 <system_reset_exception>:
>> c00000000002a610:       0e 01 4c 3c     addis   r2,r12,270
>>                          c00000000002a610: R_PPC64_REL16_HA    .TOC.
>> c00000000002a614:       f0 6c 42 38     addi    r2,r2,27888
>>                          c00000000002a614: R_PPC64_REL16_LO    .TOC.+0x4
>> c00000000002a618:       a6 02 08 7c     mflr    r0
>>
>> This is happening because we should be looking for destination
>> symbols that are at absolute offsets instead of relative offsets.
>> After fixing dest_off to point to absolute offset, there are still
>> a lot of these warnings shown.
>>
>> In the above example, objtool is computing the destination
>> offset to be c00000000002a618, which points to a completely
>> different instruction. find_call_destination() is looking for this
>> offset and failing. Instead, we should be looking for destination
>> offset c00000000002a610 which points to system_reset_exception
>> function.
>>
>> Even after fixing the way destination offset is computed, and
>> after looking for dest_off - 0x8 in cases where the original offset
>> is not found, there are still a lot of unannotated intra-function
>> call warnings generated. This is due to symbols that are not
>> properly annotated.
>>
>> So, for now, as a hack to curb these warnings, do not emit
>> unannotated intra-function call warnings when objtool is run
>> with --ftr-fixup option.
> Should not all those fixes be split out into separate patches?
Thank you for reviewing. You're right - these fixes should be split into 
separate patches.
>   Also,
> Changelog seems to have lost the bit where you explain *why* you need
> this. IIRC Nick's original tool had a description of why this is needed.

Yes, Nick's original patch has the description on why this is needed: 
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20170521010130.13552-1-npiggin@gmail.com/

Currently __ftr_alt* sections must be near .text because they lack the 
executable attribute (preventing linker stubs), requiring branches to 
reach targets directly. This may cause build failures as the kernel 
grows. To solve this, the approach uses --emit-relocs to preserve 
relocation information, marks sections executable (allowing stubs), and 
fixes branch offsets at build time based on their runtime locations. 
This implementation uses objtool with the --ftr-fixup option instead of 
a new tool, since objtool already has ELF parsing and instruction 
patching infrastructure. I'll include the details of why this is needed 
in the Changelog in the next version I send.

>
> Also, please see:
>
>    https://lkml.kernel.org/r/9500b90c4182b03da59472e1a27876818610b084.1758067942.git.jpoimboe@kernel.org
>
>    https://lkml.kernel.org/r/457c2e84b81bd6515aaa60ec8e9e0cc892ed7afa.1758067942.git.jpoimboe@kernel.org

Sure, will check.

Thanks,
Sathvika



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 2/3] kbuild: Add objtool integration for PowerPC feature fixups
  2025-09-29  8:04 ` [RFC PATCH v2 2/3] kbuild: Add objtool integration for PowerPC feature fixups Sathvika Vasireddy
@ 2025-10-04 19:43   ` Nicolas Schier
  2025-10-13  1:07     ` Sathvika Vasireddy
  0 siblings, 1 reply; 8+ messages in thread
From: Nicolas Schier @ 2025-10-04 19:43 UTC (permalink / raw)
  To: Sathvika Vasireddy
  Cc: linux-kbuild, linuxppc-dev, nathan, masahiroy, kees, naveen,
	jpoimboe, peterz, npiggin, maddy, segher, christophe.leroy, mingo,
	mpe, mahesh

On Mon, Sep 29, 2025 at 01:34:55PM +0530, Sathvika Vasireddy wrote:
> Add build system support for PowerPC feature fixup processing:
> 
> - Add HAVE_OBJTOOL_FTR_FIXUP config option for architectures that support
>   build-time feature fixup processing
> - Integrate objtool feature fixup processing into vmlinux build
> 
> Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
> ---
>  Makefile                 |  7 +++++++
>  arch/Kconfig             |  3 +++
>  scripts/Makefile.lib     |  5 +++--
>  scripts/Makefile.vmlinux | 13 ++++++++++++-
>  4 files changed, 25 insertions(+), 3 deletions(-)
> 
...
> diff --git a/arch/Kconfig b/arch/Kconfig
> index d1b4ffd6e085..d870aab17cba 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1334,6 +1334,9 @@ config HAVE_UACCESS_VALIDATION
>  	bool
>  	select OBJTOOL
>  
> +config HAVE_OBJTOOL_FTR_FIXUP

For me, FTR is not that natural to parse.  Is there a reason for using
FTR instead of FEATURE?

...
> diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
> index b64862dc6f08..94cc2bba929a 100644
> --- a/scripts/Makefile.vmlinux
> +++ b/scripts/Makefile.vmlinux
> @@ -84,7 +84,8 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>  # Final link of vmlinux with optional arch pass after final link
>  cmd_link_vmlinux =							\
>  	$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)" "$@";	\
> -	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
> +	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)	\
> +	$(cmd_objtool_vmlinux)
>  
>  targets += $(vmlinux-final)
>  $(vmlinux-final): scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
> @@ -131,3 +132,13 @@ existing-targets := $(wildcard $(sort $(targets)))
>  -include $(foreach f,$(existing-targets),$(dir $(f)).$(notdir $(f)).cmd)
>  
>  .PHONY: $(PHONY)
> +
> +# objtool for vmlinux
> +# ----------------------------------
> +#
> +#  For feature fixup, objtool does not run on individual
> +#  translation units. Run this on vmlinux instead.
> +
> +ifdef CONFIG_HAVE_OBJTOOL_FTR_FIXUP
> +cmd_objtool_vmlinux = ; $(objtool) --ftr-fixup --link $@

This cmd_* definition is broken as $(call cmd,objtool_vmlinux) would be
evaluated to '... ; ; ...' (shell syntax error).

What about putting the objtool call right into cmd_link_vmlinux?


cmd_link_vmlinux =							\
	$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)" "$@";	\
	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true);   \
	$(if $(CONFIG_HAVE_OBJTOOL_FTR_FIXUP), $(objtool) --ftr-fixup --link $@, true)


Kind regards,
Nicolas


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH v2 2/3] kbuild: Add objtool integration for PowerPC feature fixups
  2025-10-04 19:43   ` Nicolas Schier
@ 2025-10-13  1:07     ` Sathvika Vasireddy
  0 siblings, 0 replies; 8+ messages in thread
From: Sathvika Vasireddy @ 2025-10-13  1:07 UTC (permalink / raw)
  To: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 3140 bytes --]

Hi Nicolas,

Thank you for the review and suggestions!

On 10/5/25 1:13 AM, Nicolas Schier wrote:
> On Mon, Sep 29, 2025 at 01:34:55PM +0530, Sathvika Vasireddy wrote:
>> Add build system support for PowerPC feature fixup processing:
>>
>> - Add HAVE_OBJTOOL_FTR_FIXUP config option for architectures that support
>>    build-time feature fixup processing
>> - Integrate objtool feature fixup processing into vmlinux build
>>
>> Suggested-by: Masahiro Yamada<masahiroy@kernel.org>
>> Signed-off-by: Sathvika Vasireddy<sv@linux.ibm.com>
>> ---
>>   Makefile                 |  7 +++++++
>>   arch/Kconfig             |  3 +++
>>   scripts/Makefile.lib     |  5 +++--
>>   scripts/Makefile.vmlinux | 13 ++++++++++++-
>>   4 files changed, 25 insertions(+), 3 deletions(-)
>>
> ...
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index d1b4ffd6e085..d870aab17cba 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -1334,6 +1334,9 @@ config HAVE_UACCESS_VALIDATION
>>   	bool
>>   	select OBJTOOL
>>   
>> +config HAVE_OBJTOOL_FTR_FIXUP
> For me, FTR is not that natural to parse.  Is there a reason for using
> FTR instead of FEATURE?

The naming comes from the PowerPC-specific section names (__ftr_alt_*) , 
and (*ftr_fixup) in vmlinux. However, I agree that FEATURE is more 
readable and I'll update all references to use FEATURE_FIXUP instead.

Also, on a second thought, since feature fixup is PowerPC-specific and 
no other architecture has this mechanism (please correct me if I am 
wrong), I will move HAVE_OBJTOOL_FEATURE_FIXUP from arch/Kconfig to 
arch/powerpc/Kconfig in the next version.

>
> ...
>> diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
>> index b64862dc6f08..94cc2bba929a 100644
>> --- a/scripts/Makefile.vmlinux
>> +++ b/scripts/Makefile.vmlinux
>> @@ -84,7 +84,8 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>>   # Final link of vmlinux with optional arch pass after final link
>>   cmd_link_vmlinux =							\
>>   	$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)" "$@";	\
>> -	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>> +	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)	\
>> +	$(cmd_objtool_vmlinux)
>>   
>>   targets += $(vmlinux-final)
>>   $(vmlinux-final): scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
>> @@ -131,3 +132,13 @@ existing-targets := $(wildcard $(sort $(targets)))
>>   -include $(foreach f,$(existing-targets),$(dir $(f)).$(notdir $(f)).cmd)
>>   
>>   .PHONY: $(PHONY)
>> +
>> +# objtool for vmlinux
>> +# ----------------------------------
>> +#
>> +#  For feature fixup, objtool does not run on individual
>> +#  translation units. Run this on vmlinux instead.
>> +
>> +ifdef CONFIG_HAVE_OBJTOOL_FTR_FIXUP
>> +cmd_objtool_vmlinux = ; $(objtool) --ftr-fixup --link $@
> This cmd_* definition is broken as $(call cmd,objtool_vmlinux) would be
> evaluated to '... ; ; ...' (shell syntax error).
>
> What about putting the objtool call right into cmd_link_vmlinux?

Sure, I will include the changes to put the objtool call into 
cmd_link_vmlinux as suggested, in the next version.

Thanks, Sathvika

[-- Attachment #2: Type: text/html, Size: 4512 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-10-13  1:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-29  8:04 [RFC PATCH v2 0/3] objtool: Fixup alternate feature relative addresses Sathvika Vasireddy
2025-09-29  8:04 ` [RFC PATCH v2 1/3] objtool/powerpc: Enhance objtool to fixup " Sathvika Vasireddy
2025-09-29 10:49   ` Peter Zijlstra
2025-09-29 16:59     ` Sathvika Vasireddy
2025-09-29  8:04 ` [RFC PATCH v2 2/3] kbuild: Add objtool integration for PowerPC feature fixups Sathvika Vasireddy
2025-10-04 19:43   ` Nicolas Schier
2025-10-13  1:07     ` Sathvika Vasireddy
2025-09-29  8:04 ` [RFC PATCH v2 3/3] powerpc: Enable build-time feature fixup processing by default Sathvika Vasireddy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).