Boot hang with certain PowerPC configurations after da0e5b885b25cf4ded0fa89b965dc6979ac02ca9 #1581
Comments
nathanchance
added
[BUG] Untriaged
|
I encounter a pahole (my The change refactored how I can find some |
cc @mpe |
|
I can make the value correct but.. well, it's just more code without many benefits. It's also a bit tricky to implement (I really want to get rid of glibc logic is the following and a smaller DT_LACOUNT value will still work. Assert the first DT_RELACOUNT relocations are R_PPC64_RELATIVE. Perform R_PPC64_RELATIVE.
For the rest relocations, check the type and perform one of R_PPC64_RELATIVE/R_PPC64_ADDR64/...The arch/powerpc code just seems to assume that there is no I'd hope that the arch/powerpc code just be benign and allow the DT_RELACOUNT value to be any of The powerpc maintainers may search for RELR on https://sourceware.org/pipermail/binutils/2022-January/ and refactor the code to support DT_RELR. |
|
There's probably some important context as why the number of that type of relocation is hard coded in the source... |
|
It appears that cc @aik since I noticed you opened llvm/llvm-project#53786 above. |
…4le-llvm-ias" This reverts commit fdee477, reversing changes made to 3aa7dcd. An ld.lld change in main breaks booting CONFIG_RELOCATABLE kernels, which includes Fedora and OpenSUSE's configurations. Revert this change for now, until a solution or appropriate workaround can be created. Link: ClangBuiltLinux/linux#1581 Signed-off-by: Nathan Chancellor <[email protected]>
|
This patch can fix the issue From 9822ba2edb1808a2cf3a4690178d20a0ea8dfd36 Mon Sep 17 00:00:00 2001
From: Fangrui Song <hidden>
Date: Tue, 8 Mar 2022 21:31:51 -0800
Subject: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ
DT_RELACOUNT is an ELF dynamic tag inherited from SunOS indicating the number
of R_*_RELATIVE relocations. It is optional but {ld.lld,ld.lld} -z combreloc
always creates it (if non-zero) to slightly speed up glibc ld.so relocation
resolving by avoiding R_*R_PPC64_RELATIVE type comparison. The tag is otherwise
nearly unused in the wild.
lld>=15 (since commit da0e5b885b25cf4ded0fa89b965dc6979ac02ca9) underestimates
DT_RELACOUNT for ppc64 when position-independent long branch thunks are used.
Since our code always compares the relocation type with R_PPC64_RELATIVE,
replacing every occurrence of DT_RELACOUNT with
DT_RELASZ/sizeof(Elf64_Rela)=DT_RELASZ/24 is correct. Make such changes to
avoid using the obscure DT_RELACOUNT.
For an unsigned 32-bit integer (the bound of DT_RELASZ), x divided by 24
can be implemented as x*0xaaaaaaab >> 4.
Link: https://github.com/ClangBuiltLinux/linux/issues/1581
Reported-by: Nathan Chancellor <hidden>
Signed-off-by: Fangrui Song <hidden>
---
arch/powerpc/boot/crt0.S | 28 +++++++++++++++++-----------
arch/powerpc/kernel/reloc_64.S | 15 +++++++++------
2 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
index feadee18e271..1c96ebe7ef1a 100644
--- a/arch/powerpc/boot/crt0.S
+++ b/arch/powerpc/boot/crt0.S
@@ -8,7 +8,7 @@
#include "ppc_asm.h"
RELA = 7
-RELACOUNT = 0x6ffffff9
+RELASZ = 8
.data
/* A procedure descriptor used when booting this as a COFF file.
@@ -65,7 +65,7 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */
subf r11,r11,r12 /* runtime - linktime offset */
/* The dynamic section contains a series of tagged entries.
- * We need the RELA and RELACOUNT entries. */
+ * We need the RELA and RELASZ entries. */
li r9,0
li r0,0
9: lwz r8,0(r12) /* get tag */
@@ -75,18 +75,21 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */
bne 11f
lwz r9,4(r12) /* get RELA pointer in r9 */
b 12f
-11: addis r8,r8,(-RELACOUNT)@ha
- cmpwi r8,RELACOUNT@l
+11: cmpwi r8,RELASZ
bne 12f
- lwz r0,4(r12) /* get RELACOUNT value in r0 */
+ lwz r0,4(r12) /* get RELASZ / 24 in r0 */
+ lis r8,0xaaaa
+ ori r8,r8,0xaaab
+ mulhwu r0,r0,r8
+ srwi r0,r0,4
12: addi r12,r12,8
b 9b
/* The relocation section contains a list of relocations.
* We now do the R_PPC_RELATIVE ones, which point to words
* which need to be initialized with addend + offset.
- * The R_PPC_RELATIVE ones come first and there are RELACOUNT
- * of them. */
+ * The R_PPC_RELATIVE ones come first and there are at most
+ * RELASZ/24 of them. */
10: /* skip relocation if we don't have both */
cmpwi r0,0
beq 3f
@@ -160,14 +163,17 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */
bne 10f
ld r13,8(r11) /* get RELA pointer in r13 */
b 11f
-10: addis r12,r12,(-RELACOUNT)@ha
- cmpdi r12,RELACOUNT@l
+10: cmpdi r12,RELASZ
bne 11f
- ld r8,8(r11) /* get RELACOUNT value in r8 */
+ ld r8,8(r11) /* get RELASZ / 24 in r8 */
+ lis r0,0xaaaa
+ ori r0,r0,0xaaab
+ mulhwu r8,r8,r0
+ srwi r8,r8,4
11: addi r11,r11,16
b 9b
12:
- cmpdi r13,0 /* check we have both RELA and RELACOUNT */
+ cmpdi r13,0 /* check we have both RELA and RELASZ */
cmpdi cr1,r8,0
beq 3f
beq cr1,3f
diff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S
index 02d4719bf43a..362be759609f 100644
--- a/arch/powerpc/kernel/reloc_64.S
+++ b/arch/powerpc/kernel/reloc_64.S
@@ -8,7 +8,7 @@
#include <asm/ppc_asm.h>
RELA = 7
-RELACOUNT = 0x6ffffff9
+RELASZ = 8
R_PPC64_RELATIVE = 22
/*
@@ -27,7 +27,7 @@ _GLOBAL(relocate)
add r10,r10,r12 /* r10 has runtime addr of _stext */
/*
- * Scan the dynamic section for the RELA and RELACOUNT entries.
+ * Scan the dynamic section for the RELA and RELASZ entries.
*/
li r7,0
li r8,0
@@ -38,13 +38,16 @@ _GLOBAL(relocate)
bne 2f
ld r7,8(r11) /* get RELA pointer in r7 */
b 3f
-2: addis r6,r6,(-RELACOUNT)@ha
- cmpdi r6,RELACOUNT@l
+2: cmpdi r6,RELASZ
bne 3f
- ld r8,8(r11) /* get RELACOUNT value in r8 */
+ ld r8,8(r11) /* get RELA / 24 in r8 */
+ lis r0,0xaaaa
+ ori r0,r0,0xaaab
+ mulhwu r8,r8,r0
+ srwi r8,r8,4
3: addi r11,r11,16
b 1b
-4: cmpdi r7,0 /* check we have both RELA and RELACOUNT */
+4: cmpdi r7,0 /* check we have both RELA and RELASZ */
cmpdi cr1,r8,0
beq 6f
beq cr1,6f
--
2.35.1 |
nathanchance
added
[BUG] linux
|
Thanks a lot Fangrui! That patch works for me, I have provided a tag upstream. |
So this seems implicitly fixed in -next.
|
Do note that both of those runs did not use |
|
Merged into mainline: https://git.kernel.org/linus/d799769188529abc6cbf035a10087a51f7832b6b I am not sure this would be an acceptable backport for stable, so I am just going to close this up for now. |
nathanchance
added
[FIXED][LINUX] 5.18
|
As d799769188529abc6cbf035a10087a51f7832b6b also resolves #1594, I have started a thread about whether or not applying that change to stable is appropriate. |
Uh oh!
There was an error while loading. Please reload this page.
After llvm/llvm-project@da0e5b8, both Fedora and OpenSUSE's powerpc64le configurations fail to boot for me in QEMU. On v5.17-rc3:
At the commit prior to that one, the kernel fires right up:
I have done no further triage, as I am currently testing a few different things. cc @MaskRay in case something obvious sticks out to you. I am happy to provide any information that might make figuring out what is going wrong any easier.
The text was updated successfully, but these errors were encountered: