All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fangrui Song <maskray@google.com>
To: Michael Ellerman <mpe@ellerman.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	 Alexey Kardashevskiy <aik@ozlabs.ru>,
	linuxppc-dev@lists.ozlabs.org
Cc: Nathan Chancellor <nathan@kernel.org>,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
	 Fangrui Song <maskray@google.com>
Subject: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24
Date: Tue,  8 Mar 2022 21:51:18 -0800	[thread overview]
Message-ID: <20220309055118.1551013-1-maskray@google.com> (raw)

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 <maskray@google.com>
---
 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


WARNING: multiple messages have this Message-ID (diff)
From: Fangrui Song <maskray@google.com>
To: Michael Ellerman <mpe@ellerman.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	 Alexey Kardashevskiy <aik@ozlabs.ru>,
	linuxppc-dev@lists.ozlabs.org
Cc: Nathan Chancellor <nathan@kernel.org>,
	llvm@lists.linux.dev, linux-kernel@vger.kernel.org,
	Fangrui Song <maskray@google.com>
Subject: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24
Date: Tue,  8 Mar 2022 21:51:18 -0800	[thread overview]
Message-ID: <20220309055118.1551013-1-maskray@google.com> (raw)

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 <maskray@google.com>
---
 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


             reply	other threads:[~2022-03-09  5:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-09  5:51 Fangrui Song [this message]
2022-03-09  5:51 ` [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24 Fangrui Song
2022-03-09 19:01 ` Nathan Chancellor
2022-03-09 19:01   ` Nathan Chancellor
2022-03-10 19:48 ` Nick Desaulniers
2022-03-10 19:48   ` Nick Desaulniers
2022-03-10 21:08   ` Fāng-ruì Sòng
2022-03-10 21:08     ` Fāng-ruì Sòng
2022-03-11  4:15     ` Michael Ellerman
2022-03-11  4:15       ` Michael Ellerman
2022-03-11  4:35       ` Alexey Kardashevskiy
2022-03-11  4:35         ` Alexey Kardashevskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220309055118.1551013-1-maskray@google.com \
    --to=maskray@google.com \
    --cc=aik@ozlabs.ru \
    --cc=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=llvm@lists.linux.dev \
    --cc=mpe@ellerman.id.au \
    --cc=nathan@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.