[PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE1662d

6 messages, 5 authors, 2022-03-11 · open the first message on its own page

[PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24

From: Fangrui Song <hidden>
Date: 2022-03-09 05:53:45

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 and I'd
recommend that software avoids using it.

lld>=14.0.0 (since commit da0e5b885b25cf4ded0fa89b965dc6979ac02ca9)
underestimates DT_RELACOUNT for ppc64 when position-independent long
branch thunks are used. Correcting it needs non-trivial arch-specific
complexity which I'd prefer to avoid. 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 a correct
alternative.

DT_RELASZ is in practice bounded by an uint32_t. Dividing x by 24 can be
implemented as (uint32_t)(x*0xaaaaaaab) >> 4.

Link: https://github.com/ClangBuiltLinux/linux/issues/1581
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Fangrui Song <redacted>
---
 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.616.g0bdcbb4464-goog

Re: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24

From: Nathan Chancellor <nathan@kernel.org>
Date: 2022-03-09 19:02:14

On Tue, Mar 08, 2022 at 09:51:18PM -0800, Fangrui Song wrote:
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}
                                                         ^ ld.{bfd,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 and I'd
recommend that software avoids using it.

lld>=14.0.0 (since commit da0e5b885b25cf4ded0fa89b965dc6979ac02ca9)
underestimates DT_RELACOUNT for ppc64 when position-independent long
branch thunks are used. Correcting it needs non-trivial arch-specific
complexity which I'd prefer to avoid. 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 a correct
alternative.

DT_RELASZ is in practice bounded by an uint32_t. Dividing x by 24 can be
implemented as (uint32_t)(x*0xaaaaaaab) >> 4.

Link: https://github.com/ClangBuiltLinux/linux/issues/1581
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Fangrui Song <redacted>
Thank you a lot for the fix! I tested this with several different
configurations with tip of tree LLVM and they all booted in QEMU. I also
tested one configuration with GCC 5.4 (we support GCC 5.x as a minimum)
and it booted as well.

Tested-by: Nathan Chancellor <nathan@kernel.org>
quoted hunk
---
 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.616.g0bdcbb4464-goog

Re: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24

From: Nick Desaulniers <ndesaulniers@google.com>
Date: 2022-03-10 19:48:57

On Tue, Mar 8, 2022 at 9:53 PM Fangrui Song [off-list ref] wrote:
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 and I'd
recommend that software avoids using it.

lld>=14.0.0 (since commit da0e5b885b25cf4ded0fa89b965dc6979ac02ca9)
underestimates DT_RELACOUNT for ppc64 when position-independent long
branch thunks are used. Correcting it needs non-trivial arch-specific
complexity which I'd prefer to avoid. 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 a correct
alternative.
checking that sizeof(Elf64_Rela) == 24, yep: https://godbolt.org/z/bb4aKbo5T
DT_RELASZ is in practice bounded by an uint32_t. Dividing x by 24 can be
implemented as (uint32_t)(x*0xaaaaaaab) >> 4.
Yep: https://godbolt.org/z/x9445ePPv
quoted hunk
Link: https://github.com/ClangBuiltLinux/linux/issues/1581
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Fangrui Song <redacted>
---
 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
Both translation units include arch/powerpc/include/asm/ppc_asm.h,
which happens to define LOAD_REG_IMMEDIATE. Let's reuse that rather
than open code lis+ori?
quoted hunk
 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.616.g0bdcbb4464-goog

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24

From: Fāng-ruì Sòng <hidden>
Date: 2022-03-10 21:08:26

On Thu, Mar 10, 2022 at 11:48 AM Nick Desaulniers
[off-list ref] wrote:
On Tue, Mar 8, 2022 at 9:53 PM Fangrui Song [off-list ref] wrote:
quoted
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 and I'd
recommend that software avoids using it.

lld>=14.0.0 (since commit da0e5b885b25cf4ded0fa89b965dc6979ac02ca9)
underestimates DT_RELACOUNT for ppc64 when position-independent long
branch thunks are used. Correcting it needs non-trivial arch-specific
complexity which I'd prefer to avoid. 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 a correct
alternative.
checking that sizeof(Elf64_Rela) == 24, yep: https://godbolt.org/z/bb4aKbo5T
quoted
DT_RELASZ is in practice bounded by an uint32_t. Dividing x by 24 can be
implemented as (uint32_t)(x*0xaaaaaaab) >> 4.
Yep: https://godbolt.org/z/x9445ePPv
quoted
Link: https://github.com/ClangBuiltLinux/linux/issues/1581
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Fangrui Song <redacted>
---
 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
Both translation units include arch/powerpc/include/asm/ppc_asm.h,
which happens to define LOAD_REG_IMMEDIATE. Let's reuse that rather
than open code lis+ori?
quoted
 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.616.g0bdcbb4464-goog

--
Thanks,
~Nick Desaulniers
I rebased the patch on
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master and got a conflict.
Seems that https://lore.kernel.org/linuxppc-dev/20220309061822.168173-1-aik@ozlabs.ru/T/#u
("[PATCH kernel v4] powerpc/64: Add UADDR64 relocation support") fixed
the issue.
It just doesn't change arch/powerpc/boot/crt0.S

Re: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2022-03-11 04:16:15

Fāng-ruì Sòng [off-list ref] writes:
On Thu, Mar 10, 2022 at 11:48 AM Nick Desaulniers
[off-list ref] wrote:
quoted
On Tue, Mar 8, 2022 at 9:53 PM Fangrui Song [off-list ref] wrote:
quoted
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 and I'd
recommend that software avoids using it.

lld>=14.0.0 (since commit da0e5b885b25cf4ded0fa89b965dc6979ac02ca9)
underestimates DT_RELACOUNT for ppc64 when position-independent long
branch thunks are used. Correcting it needs non-trivial arch-specific
complexity which I'd prefer to avoid. 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 a correct
alternative.
checking that sizeof(Elf64_Rela) == 24, yep: https://godbolt.org/z/bb4aKbo5T
quoted
DT_RELASZ is in practice bounded by an uint32_t. Dividing x by 24 can be
implemented as (uint32_t)(x*0xaaaaaaab) >> 4.
Yep: https://godbolt.org/z/x9445ePPv
quoted
Link: https://github.com/ClangBuiltLinux/linux/issues/1581
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Fangrui Song <redacted>
---
 arch/powerpc/boot/crt0.S       | 28 +++++++++++++++++-----------
 arch/powerpc/kernel/reloc_64.S | 15 +++++++++------
 2 files changed, 26 insertions(+), 17 deletions(-)
...
I rebased the patch on
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master and got a conflict.
Seems that https://lore.kernel.org/linuxppc-dev/20220309061822.168173-1-aik@ozlabs.ru/T/#u
("[PATCH kernel v4] powerpc/64: Add UADDR64 relocation support") fixed
the issue.
It just doesn't change arch/powerpc/boot/crt0.S
Yeah sorry, I applied Alexey's v4 just before I saw your patch arrive on
the list.

If one of you can rework this so it applies on top that would be great :)

cheers

Re: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24

From: Alexey Kardashevskiy <hidden>
Date: 2022-03-11 04:36:10


On 3/11/22 15:15, Michael Ellerman wrote:
Fāng-ruì Sòng [off-list ref] writes:
quoted
On Thu, Mar 10, 2022 at 11:48 AM Nick Desaulniers
[off-list ref] wrote:
quoted
On Tue, Mar 8, 2022 at 9:53 PM Fangrui Song [off-list ref] wrote:
quoted
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 and I'd
recommend that software avoids using it.

lld>=14.0.0 (since commit da0e5b885b25cf4ded0fa89b965dc6979ac02ca9)
underestimates DT_RELACOUNT for ppc64 when position-independent long
branch thunks are used. Correcting it needs non-trivial arch-specific
complexity which I'd prefer to avoid. 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 a correct
alternative.
checking that sizeof(Elf64_Rela) == 24, yep: https://godbolt.org/z/bb4aKbo5T
quoted
DT_RELASZ is in practice bounded by an uint32_t. Dividing x by 24 can be
implemented as (uint32_t)(x*0xaaaaaaab) >> 4.
Yep: https://godbolt.org/z/x9445ePPv
quoted
Link: https://github.com/ClangBuiltLinux/linux/issues/1581
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Fangrui Song <redacted>
---
  arch/powerpc/boot/crt0.S       | 28 +++++++++++++++++-----------
  arch/powerpc/kernel/reloc_64.S | 15 +++++++++------
  2 files changed, 26 insertions(+), 17 deletions(-)
...
quoted
I rebased the patch on
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master and got a conflict.
Seems that https://lore.kernel.org/linuxppc-dev/20220309061822.168173-1-aik@ozlabs.ru/T/#u
("[PATCH kernel v4] powerpc/64: Add UADDR64 relocation support") fixed
the issue.
It just doesn't change arch/powerpc/boot/crt0.S
Yeah sorry, I applied Alexey's v4 just before I saw your patch arrive on
the list.

If one of you can rework this so it applies on top that would be great :)

I guess it is me as now I have to add that UARRD64 thing to crt0.S as 
well, don't I?

And also we are giving up on the llvm ld having a bug with RELACOUNT?

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help