[RFC PATCH 2/2] objtool/powerpc: Enhance objtool to fixup alternate feature relative addresses
Sathvika Vasireddy
sv at linux.vnet.ibm.com
Mon Jun 3 18:29:28 AEST 2024
Hi Nathan,
On 4/23/24 5:58 AM, Nathan Chancellor wrote:
> Hi Sathvika,
>
> On Mon, Apr 22, 2024 at 02:52:06PM +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.
>>
>> TODO:
>> This patch enables build time feature fixup only for powerpc little
>> endian configs. There are boot failures with big endian configs.
>> Posting this as an initial RFC to get some review comments while I work
>> on big endian issues.
>>
>> [1]
>> https://lore.kernel.org/linuxppc-dev/20170521010130.13552-1-npiggin@gmail.com/
>>
>> Co-developed-by: Nicholas Piggin <npiggin at gmail.com>
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> Signed-off-by: Sathvika Vasireddy <sv at linux.ibm.com>
> When I build this series with LLVM 14 [1] (due to an issue I report
> below), I am getting a crash when CONFIG_FTR_FIXUP_SELFTEST is disabled.
>
> diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
> index 544a65fda77b..95d2906ec814 100644
> --- a/arch/powerpc/configs/ppc64_defconfig
> +++ b/arch/powerpc/configs/ppc64_defconfig
> @@ -427,7 +427,6 @@ CONFIG_BLK_DEV_IO_TRACE=y
> CONFIG_IO_STRICT_DEVMEM=y
> CONFIG_PPC_EMULATED_STATS=y
> CONFIG_CODE_PATCHING_SELFTEST=y
> -CONFIG_FTR_FIXUP_SELFTEST=y
> CONFIG_MSI_BITMAP_SELFTEST=y
> CONFIG_XMON=y
> CONFIG_BOOTX_TEXT=y
>
> $ make -kj"$(nproc)" ARCH=powerpc LLVM=$PWD/llvm-14.0.6-x86_64/bin/ ppc64le_defconfig vmlinux
> ...
> LD vmlinux
> NM System.map
> SORTTAB vmlinux
> CHKHEAD vmlinux
> CHKREL vmlinux
> make[4]: *** [scripts/Makefile.vmlinux:38: vmlinux] Error 139
> make[4]: *** Deleting file 'vmlinux
> ...
>
> I do not see the objtool command in V=1 output but I do see the end of
> scripts/link-vmlinux.sh, so it does seem like it is crashing in objtool.
>
> [1]: from https://mirrors.edge.kernel.org/pub/tools/llvm/
Thanks for reporting this, and apologies for the delay in response.
This issue is happening while processing __fw_ftr_fixup section, which
has no data and dereferencing this null pointer is causing segmentation
fault. I was able to recreate the issue, and the below diff fixes it for me.
diff --git a/tools/objtool/arch/powerpc/special.c
b/tools/objtool/arch/powerpc/special.c
index 5ec3eed34eb0..67329d44db24 100644
--- a/tools/objtool/arch/powerpc/special.c
+++ b/tools/objtool/arch/powerpc/special.c
@@ -53,38 +53,39 @@ int process_fixup_entries(struct objtool_file *file)
if (strstr(sec->name, "_ftr_fixup") != NULL) {
Elf_Data *data = sec->data;
- if (data && data->d_size > 0)
+ if (data && data->d_size > 0) {
nr = data->d_size / sizeof(struct
fixup_entry);
- for (i = 0; i < nr; i++) {
- struct fixup_entry *dst;
- unsigned long idx;
- unsigned long long off;
- struct fixup_entry *src;
+ for (i = 0; i < nr; i++) {
+ struct fixup_entry *dst;
+ unsigned long idx;
+ unsigned long long off;
+ struct fixup_entry *src;
- idx = i * sizeof(struct fixup_entry);
- off = sec->sh.sh_addr + data->d_off + idx;
- src = data->d_buf + idx;
+ idx = i * sizeof(struct
fixup_entry);
+ off = sec->sh.sh_addr +
data->d_off + idx;
+ src = data->d_buf + idx;
- if (src->alt_start_off == src->alt_end_off)
- continue;
+ 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++;
+ fes = realloc(fes, (nr_fes + 1)
* sizeof(struct fixup_entry));
+ dst = &fes[nr_fes];
+ nr_fes++;
- dst->mask = __le64_to_cpu(src->mask);
- dst->value = __le64_to_cpu(src->value);
- dst->start_off =
__le64_to_cpu(src->start_off) + off;
- dst->end_off =
__le64_to_cpu(src->end_off) + off;
- dst->alt_start_off =
__le64_to_cpu(src->alt_start_off) + off;
- dst->alt_end_off =
__le64_to_cpu(src->alt_end_off) + off;
+ dst->mask =
__le64_to_cpu(src->mask);
+ dst->value =
__le64_to_cpu(src->value);
+ dst->start_off =
__le64_to_cpu(src->start_off) + off;
+ dst->end_off =
__le64_to_cpu(src->end_off) + off;
+ dst->alt_start_off =
__le64_to_cpu(src->alt_start_off) + off;
+ dst->alt_end_off =
__le64_to_cpu(src->alt_end_off) + off;
- if (dst->alt_start_off < fe_alt_start)
- fe_alt_start = dst->alt_start_off;
+ if (dst->alt_start_off <
fe_alt_start)
+ fe_alt_start =
dst->alt_start_off;
- if (dst->alt_end_off > fe_alt_end)
- fe_alt_end = dst->alt_end_off;
+ if (dst->alt_end_off > fe_alt_end)
+ fe_alt_end =
dst->alt_end_off;
+ }
}
}
}
@@ -246,7 +247,6 @@ static struct symbol *find_symbol_at_address(struct
objtool_file *file,
int process_alt_relocations(struct objtool_file *file)
{
struct section *section;
- size_t n = 0;
uint32_t insn;
uint32_t *i;
unsigned int opcode;
@@ -276,8 +276,6 @@ int process_alt_relocations(struct objtool_file *file)
target = target + 0x8;
}
- n++;
-
fe = find_fe_altaddr(addr);
if (fe) {
I'll include these changes in the next version.
Thanks,
Sathvika
More information about the Linuxppc-dev
mailing list