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@xxxxxxxxx/
Co-developed-by: Nicholas Piggin <npiggin@xxxxxxxxx>
Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx>
Signed-off-by: Sathvika Vasireddy <sv@xxxxxxxxxxxxx>
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
