Message ID | 20220131041407.435244-1-aik@ozlabs.ru (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [kernel] powerpc/64: Add UADDR64 relocation support | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
Le 31/01/2022 à 05:14, Alexey Kardashevskiy a écrit : > When ld detects unaligned relocations, it emits R_PPC64_UADDR64 > relocations instead of R_PPC64_RELATIVE. Currently R_PPC64_UADDR64 are > detected by arch/powerpc/tools/relocs_check.sh and expected not to work. > Below is a simple chunk to trigger this behaviour: According to relocs_check.sh, this is expected to happen only with binutils < 2.19. Today minimum binutils version is 2.23 Have you observed this problem with newer version of binutils ? > > \#pragma GCC push_options > \#pragma GCC optimize ("O0") AFAIU Linux Kernel is always built with O2 Have you observed the problem with O2 ? > struct entry { > const char *file; > int line; > } __attribute__((packed)); > static const struct entry e1 = { .file = __FILE__, .line = __LINE__ }; > static const struct entry e2 = { .file = __FILE__, .line = __LINE__ }; > ... > prom_printf("e1=%s %lx %lx\n", e1.file, (unsigned long) e1.file, mfmsr()); > prom_printf("e2=%s %lx\n", e2.file, (unsigned long) e2.file); > \#pragma GCC pop_options > > > This adds support for UADDR64 for 64bit. This reuses __dynamic_symtab > from the 32bit which supports more relocation types already. > > This adds a workaround for the number of relocations as the DT_RELACOUNT > ELF Dynamic Array Tag does not include relocations other than > R_PPC64_RELATIVE. This instead iterates over the entire .rela.dyn section. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > Tested via qemu gdb stub (the kernel is loaded at 0x400000). > > Disasm: > > c000000001a804d0 <e1>: > c000000001a804d0: b0 04 a8 01 .long 0x1a804b0 > c000000001a804d0: R_PPC64_RELATIVE *ABS*-0x3ffffffffe57fb50 > c000000001a804d4: 00 00 00 c0 lfs f0,0(0) > c000000001a804d8: fa 08 00 00 .long 0x8fa > > c000000001a804dc <e2>: > ... > c000000001a804dc: R_PPC64_UADDR64 .rodata+0x4b0 > > Before relocation: > >>>> p *(unsigned long *) 0x1e804d0 > $1 = 0xc000000001a804b0 >>>> p *(unsigned long *) 0x1e804dc > $2 = 0x0 > > After: >>>> p *(unsigned long *) 0x1e804d0 > $1 = 0x1e804b0 >>>> p *(unsigned long *) 0x1e804dc > $2 = 0x1e804b0 > --- > arch/powerpc/kernel/reloc_64.S | 47 +++++++++++++++++++++++++----- > arch/powerpc/kernel/vmlinux.lds.S | 3 +- > arch/powerpc/tools/relocs_check.sh | 6 ---- > 3 files changed, 41 insertions(+), 15 deletions(-) > > diff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S > index 02d4719bf43a..a91175723d9d 100644 > --- a/arch/powerpc/kernel/reloc_64.S > +++ b/arch/powerpc/kernel/reloc_64.S > @@ -10,6 +10,7 @@ > RELA = 7 > RELACOUNT = 0x6ffffff9 > R_PPC64_RELATIVE = 22 > +R_PPC64_UADDR64 = 43 > > /* > * r3 = desired final address of kernel > @@ -25,6 +26,8 @@ _GLOBAL(relocate) > add r9,r9,r12 /* r9 has runtime addr of .rela.dyn section */ > ld r10,(p_st - 0b)(r12) > add r10,r10,r12 /* r10 has runtime addr of _stext */ > + ld r13,(p_sym - 0b)(r12) > + add r13,r13,r12 /* r13 has runtime addr of .dynsym */ > > /* > * Scan the dynamic section for the RELA and RELACOUNT entries. > @@ -46,8 +49,8 @@ _GLOBAL(relocate) > b 1b > 4: cmpdi r7,0 /* check we have both RELA and RELACOUNT */ > cmpdi cr1,r8,0 > - beq 6f > - beq cr1,6f > + beq 9f > + beq cr1,9f > > /* > * Work out linktime address of _stext and hence the > @@ -60,25 +63,55 @@ _GLOBAL(relocate) > subf r10,r7,r10 > subf r3,r10,r3 /* final_offset */ > > + /* > + * FIXME > + * Here r8 is a number of relocations in .rela.dyn. > + * When ld issues UADDR64 relocations, they end up at the end > + * of the .rela.dyn section. However RELACOUNT does not include > + * them so the loop below is going to finish after the last > + * R_PPC64_RELATIVE as they normally go first. > + * Work out the size of .rela.dyn at compile time. > + */ > + ld r8,(p_rela_end - 0b)(r12) > + ld r18,(p_rela - 0b)(r12) > + sub r8,r8,r18 > + li r18,24 /* 24 == sizeof(elf64_rela) */ > + divd r8,r8,r18 > + > /* > * Run through the list of relocations and process the > - * R_PPC64_RELATIVE ones. > + * R_PPC64_RELATIVE and R_PPC64_UADDR64 ones. > */ > mtctr r8 > -5: ld r0,8(9) /* ELF64_R_TYPE(reloc->r_info) */ > +5: lwa r0,8(r9) /* ELF64_R_TYPE(reloc->r_info) */ > cmpdi r0,R_PPC64_RELATIVE > bne 6f > ld r6,0(r9) /* reloc->r_offset */ > ld r0,16(r9) /* reloc->r_addend */ > - add r0,r0,r3 > + b 7f > + > +6: cmpdi r0,R_PPC64_UADDR64 > + bne 8f > + ld r6,0(r9) > + ld r0,16(r9) > + lwa r14,12(r9) /* ELF64_R_SYM(reloc->r_info) */ > + mulli r14,r14,24 /* 24 == sizeof(elf64_sym) */ > + add r14,r14,r13 /* elf64_sym[ELF64_R_SYM] */ > + ld r14,8(r14) > + add r0,r0,r14 > + > +7: add r0,r0,r3 > stdx r0,r7,r6 > - addi r9,r9,24 > + > +8: addi r9,r9,24 > bdnz 5b > > -6: blr > +9: blr > > .balign 8 > p_dyn: .8byte __dynamic_start - 0b > p_rela: .8byte __rela_dyn_start - 0b > +p_rela_end: .8byte __rela_dyn_end - 0b > +p_sym: .8byte __dynamic_symtab - 0b > p_st: .8byte _stext - 0b > > diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S > index 2bcca818136a..e9d9bda3ffaf 100644 > --- a/arch/powerpc/kernel/vmlinux.lds.S > +++ b/arch/powerpc/kernel/vmlinux.lds.S > @@ -281,9 +281,7 @@ SECTIONS > . = ALIGN(8); > .dynsym : AT(ADDR(.dynsym) - LOAD_OFFSET) > { > -#ifdef CONFIG_PPC32 > __dynamic_symtab = .; > -#endif > *(.dynsym) > } > .dynstr : AT(ADDR(.dynstr) - LOAD_OFFSET) { *(.dynstr) } > @@ -299,6 +297,7 @@ SECTIONS > { > __rela_dyn_start = .; > *(.rela*) > + __rela_dyn_end = .; > } > #endif > /* .exit.data is discarded at runtime, not link time, > diff --git a/arch/powerpc/tools/relocs_check.sh b/arch/powerpc/tools/relocs_check.sh > index 014e00e74d2b..956b9e236a60 100755 > --- a/arch/powerpc/tools/relocs_check.sh > +++ b/arch/powerpc/tools/relocs_check.sh > @@ -54,9 +54,3 @@ fi > num_bad=$(echo "$bad_relocs" | wc -l) > echo "WARNING: $num_bad bad relocations" > echo "$bad_relocs" > - > -# If we see this type of relocation it's an idication that > -# we /may/ be using an old version of binutils. > -if echo "$bad_relocs" | grep -q -F -w R_PPC64_UADDR64; then > - echo "WARNING: You need at least binutils >= 2.19 to build a CONFIG_RELOCATABLE kernel" > -fi
On 1/31/22 17:38, Christophe Leroy wrote: > > > Le 31/01/2022 à 05:14, Alexey Kardashevskiy a écrit : >> When ld detects unaligned relocations, it emits R_PPC64_UADDR64 >> relocations instead of R_PPC64_RELATIVE. Currently R_PPC64_UADDR64 are >> detected by arch/powerpc/tools/relocs_check.sh and expected not to work. >> Below is a simple chunk to trigger this behaviour: > > According to relocs_check.sh, this is expected to happen only with > binutils < 2.19. Today minimum binutils version is 2.23 > > Have you observed this problem with newer version of binutils ? Oh yeah. 2.36.1. And the toolchain folks explained internally that this is correct behavior and this was a ticking bomb which exploded now and the kernel has to deal with it. >> >> \#pragma GCC push_options >> \#pragma GCC optimize ("O0") > > AFAIU Linux Kernel is always built with O2 Correct. Even O1 hides this. > Have you observed the problem with O2 ? Yes, I see it once I enable CONFIG_PRINTK_INDEX (this is how it was spotted with my particular config, there is still a fair chance that this config option does not cause UADDR64 always) but I did not debug with it enabled as pretty much every single __func__ passed to printk caused unaligned relocation (tens of thousands). Note that this particular case can be fixed by removing __packed from "struct pi_entry" (== re-arm the bomb). Thanks, > > >> struct entry { >> const char *file; >> int line; >> } __attribute__((packed)); >> static const struct entry e1 = { .file = __FILE__, .line = __LINE__ }; >> static const struct entry e2 = { .file = __FILE__, .line = __LINE__ }; >> ... >> prom_printf("e1=%s %lx %lx\n", e1.file, (unsigned long) e1.file, mfmsr()); >> prom_printf("e2=%s %lx\n", e2.file, (unsigned long) e2.file); >> \#pragma GCC pop_options >> >> >> This adds support for UADDR64 for 64bit. This reuses __dynamic_symtab >> from the 32bit which supports more relocation types already. >> >> This adds a workaround for the number of relocations as the DT_RELACOUNT >> ELF Dynamic Array Tag does not include relocations other than >> R_PPC64_RELATIVE. This instead iterates over the entire .rela.dyn section. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> >> Tested via qemu gdb stub (the kernel is loaded at 0x400000). >> >> Disasm: >> >> c000000001a804d0 <e1>: >> c000000001a804d0: b0 04 a8 01 .long 0x1a804b0 >> c000000001a804d0: R_PPC64_RELATIVE *ABS*-0x3ffffffffe57fb50 >> c000000001a804d4: 00 00 00 c0 lfs f0,0(0) >> c000000001a804d8: fa 08 00 00 .long 0x8fa >> >> c000000001a804dc <e2>: >> ... >> c000000001a804dc: R_PPC64_UADDR64 .rodata+0x4b0 >> >> Before relocation: >> >>>>> p *(unsigned long *) 0x1e804d0 >> $1 = 0xc000000001a804b0 >>>>> p *(unsigned long *) 0x1e804dc >> $2 = 0x0 >> >> After: >>>>> p *(unsigned long *) 0x1e804d0 >> $1 = 0x1e804b0 >>>>> p *(unsigned long *) 0x1e804dc >> $2 = 0x1e804b0 >> --- >> arch/powerpc/kernel/reloc_64.S | 47 +++++++++++++++++++++++++----- >> arch/powerpc/kernel/vmlinux.lds.S | 3 +- >> arch/powerpc/tools/relocs_check.sh | 6 ---- >> 3 files changed, 41 insertions(+), 15 deletions(-) >> >> diff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S >> index 02d4719bf43a..a91175723d9d 100644 >> --- a/arch/powerpc/kernel/reloc_64.S >> +++ b/arch/powerpc/kernel/reloc_64.S >> @@ -10,6 +10,7 @@ >> RELA = 7 >> RELACOUNT = 0x6ffffff9 >> R_PPC64_RELATIVE = 22 >> +R_PPC64_UADDR64 = 43 >> >> /* >> * r3 = desired final address of kernel >> @@ -25,6 +26,8 @@ _GLOBAL(relocate) >> add r9,r9,r12 /* r9 has runtime addr of .rela.dyn section */ >> ld r10,(p_st - 0b)(r12) >> add r10,r10,r12 /* r10 has runtime addr of _stext */ >> + ld r13,(p_sym - 0b)(r12) >> + add r13,r13,r12 /* r13 has runtime addr of .dynsym */ >> >> /* >> * Scan the dynamic section for the RELA and RELACOUNT entries. >> @@ -46,8 +49,8 @@ _GLOBAL(relocate) >> b 1b >> 4: cmpdi r7,0 /* check we have both RELA and RELACOUNT */ >> cmpdi cr1,r8,0 >> - beq 6f >> - beq cr1,6f >> + beq 9f >> + beq cr1,9f >> >> /* >> * Work out linktime address of _stext and hence the >> @@ -60,25 +63,55 @@ _GLOBAL(relocate) >> subf r10,r7,r10 >> subf r3,r10,r3 /* final_offset */ >> >> + /* >> + * FIXME >> + * Here r8 is a number of relocations in .rela.dyn. >> + * When ld issues UADDR64 relocations, they end up at the end >> + * of the .rela.dyn section. However RELACOUNT does not include >> + * them so the loop below is going to finish after the last >> + * R_PPC64_RELATIVE as they normally go first. >> + * Work out the size of .rela.dyn at compile time. >> + */ >> + ld r8,(p_rela_end - 0b)(r12) >> + ld r18,(p_rela - 0b)(r12) >> + sub r8,r8,r18 >> + li r18,24 /* 24 == sizeof(elf64_rela) */ >> + divd r8,r8,r18 >> + >> /* >> * Run through the list of relocations and process the >> - * R_PPC64_RELATIVE ones. >> + * R_PPC64_RELATIVE and R_PPC64_UADDR64 ones. >> */ >> mtctr r8 >> -5: ld r0,8(9) /* ELF64_R_TYPE(reloc->r_info) */ >> +5: lwa r0,8(r9) /* ELF64_R_TYPE(reloc->r_info) */ >> cmpdi r0,R_PPC64_RELATIVE >> bne 6f >> ld r6,0(r9) /* reloc->r_offset */ >> ld r0,16(r9) /* reloc->r_addend */ >> - add r0,r0,r3 >> + b 7f >> + >> +6: cmpdi r0,R_PPC64_UADDR64 >> + bne 8f >> + ld r6,0(r9) >> + ld r0,16(r9) >> + lwa r14,12(r9) /* ELF64_R_SYM(reloc->r_info) */ >> + mulli r14,r14,24 /* 24 == sizeof(elf64_sym) */ >> + add r14,r14,r13 /* elf64_sym[ELF64_R_SYM] */ >> + ld r14,8(r14) >> + add r0,r0,r14 >> + >> +7: add r0,r0,r3 >> stdx r0,r7,r6 >> - addi r9,r9,24 >> + >> +8: addi r9,r9,24 >> bdnz 5b >> >> -6: blr >> +9: blr >> >> .balign 8 >> p_dyn: .8byte __dynamic_start - 0b >> p_rela: .8byte __rela_dyn_start - 0b >> +p_rela_end: .8byte __rela_dyn_end - 0b >> +p_sym: .8byte __dynamic_symtab - 0b >> p_st: .8byte _stext - 0b >> >> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S >> index 2bcca818136a..e9d9bda3ffaf 100644 >> --- a/arch/powerpc/kernel/vmlinux.lds.S >> +++ b/arch/powerpc/kernel/vmlinux.lds.S >> @@ -281,9 +281,7 @@ SECTIONS >> . = ALIGN(8); >> .dynsym : AT(ADDR(.dynsym) - LOAD_OFFSET) >> { >> -#ifdef CONFIG_PPC32 >> __dynamic_symtab = .; >> -#endif >> *(.dynsym) >> } >> .dynstr : AT(ADDR(.dynstr) - LOAD_OFFSET) { *(.dynstr) } >> @@ -299,6 +297,7 @@ SECTIONS >> { >> __rela_dyn_start = .; >> *(.rela*) >> + __rela_dyn_end = .; >> } >> #endif >> /* .exit.data is discarded at runtime, not link time, >> diff --git a/arch/powerpc/tools/relocs_check.sh b/arch/powerpc/tools/relocs_check.sh >> index 014e00e74d2b..956b9e236a60 100755 >> --- a/arch/powerpc/tools/relocs_check.sh >> +++ b/arch/powerpc/tools/relocs_check.sh >> @@ -54,9 +54,3 @@ fi >> num_bad=$(echo "$bad_relocs" | wc -l) >> echo "WARNING: $num_bad bad relocations" >> echo "$bad_relocs" >> - >> -# If we see this type of relocation it's an idication that >> -# we /may/ be using an old version of binutils. >> -if echo "$bad_relocs" | grep -q -F -w R_PPC64_UADDR64; then >> - echo "WARNING: You need at least binutils >= 2.19 to build a CONFIG_RELOCATABLE kernel" >> -fi
diff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S index 02d4719bf43a..a91175723d9d 100644 --- a/arch/powerpc/kernel/reloc_64.S +++ b/arch/powerpc/kernel/reloc_64.S @@ -10,6 +10,7 @@ RELA = 7 RELACOUNT = 0x6ffffff9 R_PPC64_RELATIVE = 22 +R_PPC64_UADDR64 = 43 /* * r3 = desired final address of kernel @@ -25,6 +26,8 @@ _GLOBAL(relocate) add r9,r9,r12 /* r9 has runtime addr of .rela.dyn section */ ld r10,(p_st - 0b)(r12) add r10,r10,r12 /* r10 has runtime addr of _stext */ + ld r13,(p_sym - 0b)(r12) + add r13,r13,r12 /* r13 has runtime addr of .dynsym */ /* * Scan the dynamic section for the RELA and RELACOUNT entries. @@ -46,8 +49,8 @@ _GLOBAL(relocate) b 1b 4: cmpdi r7,0 /* check we have both RELA and RELACOUNT */ cmpdi cr1,r8,0 - beq 6f - beq cr1,6f + beq 9f + beq cr1,9f /* * Work out linktime address of _stext and hence the @@ -60,25 +63,55 @@ _GLOBAL(relocate) subf r10,r7,r10 subf r3,r10,r3 /* final_offset */ + /* + * FIXME + * Here r8 is a number of relocations in .rela.dyn. + * When ld issues UADDR64 relocations, they end up at the end + * of the .rela.dyn section. However RELACOUNT does not include + * them so the loop below is going to finish after the last + * R_PPC64_RELATIVE as they normally go first. + * Work out the size of .rela.dyn at compile time. + */ + ld r8,(p_rela_end - 0b)(r12) + ld r18,(p_rela - 0b)(r12) + sub r8,r8,r18 + li r18,24 /* 24 == sizeof(elf64_rela) */ + divd r8,r8,r18 + /* * Run through the list of relocations and process the - * R_PPC64_RELATIVE ones. + * R_PPC64_RELATIVE and R_PPC64_UADDR64 ones. */ mtctr r8 -5: ld r0,8(9) /* ELF64_R_TYPE(reloc->r_info) */ +5: lwa r0,8(r9) /* ELF64_R_TYPE(reloc->r_info) */ cmpdi r0,R_PPC64_RELATIVE bne 6f ld r6,0(r9) /* reloc->r_offset */ ld r0,16(r9) /* reloc->r_addend */ - add r0,r0,r3 + b 7f + +6: cmpdi r0,R_PPC64_UADDR64 + bne 8f + ld r6,0(r9) + ld r0,16(r9) + lwa r14,12(r9) /* ELF64_R_SYM(reloc->r_info) */ + mulli r14,r14,24 /* 24 == sizeof(elf64_sym) */ + add r14,r14,r13 /* elf64_sym[ELF64_R_SYM] */ + ld r14,8(r14) + add r0,r0,r14 + +7: add r0,r0,r3 stdx r0,r7,r6 - addi r9,r9,24 + +8: addi r9,r9,24 bdnz 5b -6: blr +9: blr .balign 8 p_dyn: .8byte __dynamic_start - 0b p_rela: .8byte __rela_dyn_start - 0b +p_rela_end: .8byte __rela_dyn_end - 0b +p_sym: .8byte __dynamic_symtab - 0b p_st: .8byte _stext - 0b diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index 2bcca818136a..e9d9bda3ffaf 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -281,9 +281,7 @@ SECTIONS . = ALIGN(8); .dynsym : AT(ADDR(.dynsym) - LOAD_OFFSET) { -#ifdef CONFIG_PPC32 __dynamic_symtab = .; -#endif *(.dynsym) } .dynstr : AT(ADDR(.dynstr) - LOAD_OFFSET) { *(.dynstr) } @@ -299,6 +297,7 @@ SECTIONS { __rela_dyn_start = .; *(.rela*) + __rela_dyn_end = .; } #endif /* .exit.data is discarded at runtime, not link time, diff --git a/arch/powerpc/tools/relocs_check.sh b/arch/powerpc/tools/relocs_check.sh index 014e00e74d2b..956b9e236a60 100755 --- a/arch/powerpc/tools/relocs_check.sh +++ b/arch/powerpc/tools/relocs_check.sh @@ -54,9 +54,3 @@ fi num_bad=$(echo "$bad_relocs" | wc -l) echo "WARNING: $num_bad bad relocations" echo "$bad_relocs" - -# If we see this type of relocation it's an idication that -# we /may/ be using an old version of binutils. -if echo "$bad_relocs" | grep -q -F -w R_PPC64_UADDR64; then - echo "WARNING: You need at least binutils >= 2.19 to build a CONFIG_RELOCATABLE kernel" -fi