linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH kernel] powerpc/boot: Stop using RELACOUNT
@ 2022-03-21  7:10 Alexey Kardashevskiy
  2022-03-22  2:12 ` Michael Ellerman
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2022-03-21  7:10 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Fangrui Song, Alexey Kardashevskiy, Nick Desaulniers,
	Nathan Chancellor

So far the RELACOUNT tag from the ELF header was containing the exact
number of R_PPC_RELATIVE/R_PPC64_RELATIVE relocations. However the LLVM's
recent change [1] make it equal-or-less than the actual number which
makes it useless.

This replaces RELACOUNT in zImage loader with a pair of RELASZ and RELAENT.
The vmlinux relocation code is fixed in [2].

To make it more future proof, this walks through the entire .rela.dyn
section instead of assuming that the section is sorter by a relocation
type. Unlike [1], this does not add unaligned UADDR/UADDR64 relocations
as in hardly possible to see those in arch-specific zImage.

[1] https://github.com/llvm/llvm-project/commit/da0e5b885b25cf4
[2] https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=d799769188529a
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/boot/crt0.S | 43 +++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
index feadee18e271..6ea3417da3b7 100644
--- a/arch/powerpc/boot/crt0.S
+++ b/arch/powerpc/boot/crt0.S
@@ -8,7 +8,8 @@
 #include "ppc_asm.h"
 
 RELA = 7
-RELACOUNT = 0x6ffffff9
+RELASZ = 8
+RELAENT = 9
 
 	.data
 	/* A procedure descriptor used when booting this as a COFF file.
@@ -75,34 +76,38 @@ 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	111f
+	lwz	r0,4(r12)       /* get RELASZ value in r0 */
+	b	12f
+111:	cmpwi	r8,RELAENT
 	bne	12f
-	lwz	r0,4(r12)	/* get RELACOUNT value in r0 */
+	lwz     r14,4(r12)      /* get RELAENT value in r14 */
 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. */
+	 * which need to be initialized with addend + offset */
 10:	/* skip relocation if we don't have both */
 	cmpwi	r0,0
 	beq	3f
 	cmpwi	r9,0
 	beq	3f
+	cmpwi	r14,0
+	beq	3f
 
 	add	r9,r9,r11	/* Relocate RELA pointer */
+	divd    r0,r0,r14       /* RELASZ / RELAENT */
 	mtctr	r0
 2:	lbz	r0,4+3(r9)	/* ELF32_R_INFO(reloc->r_info) */
 	cmpwi	r0,22		/* R_PPC_RELATIVE */
-	bne	3f
+	bne	22f
 	lwz	r12,0(r9)	/* reloc->r_offset */
 	lwz	r0,8(r9)	/* reloc->r_addend */
 	add	r0,r0,r11
 	stwx	r0,r11,r12
-	addi	r9,r9,12
+22:	add	r9,r9,r14
 	bdnz	2b
 
 	/* Do a cache flush for our text, in case the loader didn't */
@@ -160,32 +165,38 @@ 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
-	bne	11f
-	ld	r8,8(r11)       /* get RELACOUNT value in r8 */
+10:	cmpwi   r12,RELASZ
+	bne	101f
+	lwz	r8,8(r11)	/* get RELASZ pointer in r8 */
+	b	11f
+101:	cmpwi	r12,RELAENT
+	bne     11f
+	lwz     r14,8(r11)      /* get RELAENT pointer in r14 */
 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, RELASZ, RELAENT*/
 	cmpdi	cr1,r8,0
 	beq	3f
 	beq	cr1,3f
+	cmpdi	r14,0
+	beq	3f
 
 	/* Calcuate the runtime offset. */
 	subf	r13,r13,r9
 
 	/* Run through the list of relocations and process the
 	 * R_PPC64_RELATIVE ones. */
+	divd    r8,r8,r14       /* RELASZ / RELAENT */
 	mtctr	r8
 13:	ld	r0,8(r9)        /* ELF64_R_TYPE(reloc->r_info) */
 	cmpdi	r0,22           /* R_PPC64_RELATIVE */
-	bne	3f
+	bne	14f
 	ld	r12,0(r9)        /* reloc->r_offset */
 	ld	r0,16(r9)       /* reloc->r_addend */
 	add	r0,r0,r13
 	stdx	r0,r13,r12
-	addi	r9,r9,24
+14:	add	r9,r9,r14
 	bdnz	13b
 
 	/* Do a cache flush for our text, in case the loader didn't */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH kernel] powerpc/boot: Stop using RELACOUNT
  2022-03-21  7:10 [PATCH kernel] powerpc/boot: Stop using RELACOUNT Alexey Kardashevskiy
@ 2022-03-22  2:12 ` Michael Ellerman
  2022-03-23  0:18   ` Alexey Kardashevskiy
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2022-03-22  2:12 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev
  Cc: Alexey Kardashevskiy, Nathan Chancellor, Nick Desaulniers,
	Fangrui Song

Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> So far the RELACOUNT tag from the ELF header was containing the exact
> number of R_PPC_RELATIVE/R_PPC64_RELATIVE relocations. However the LLVM's
> recent change [1] make it equal-or-less than the actual number which
> makes it useless.
>
> This replaces RELACOUNT in zImage loader with a pair of RELASZ and RELAENT.
> The vmlinux relocation code is fixed in [2].

That's committed so you can say:
  in commit d79976918852 ("powerpc/64: Add UADDR64 relocation support")

> To make it more future proof, this walks through the entire .rela.dyn
> section instead of assuming that the section is sorter by a relocation
> type. Unlike [1], this does not add unaligned UADDR/UADDR64 relocations
                ^
                that should be 2?

> as in hardly possible to see those in arch-specific zImage.

I don't quite parse that. Is it true we can never see them in zImage?
Maybe it's true that we don't see them in practice.

> [1] https://github.com/llvm/llvm-project/commit/da0e5b885b25cf4
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=d799769188529a
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  arch/powerpc/boot/crt0.S | 43 +++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> index feadee18e271..6ea3417da3b7 100644
> --- a/arch/powerpc/boot/crt0.S
> +++ b/arch/powerpc/boot/crt0.S
> @@ -8,7 +8,8 @@
>  #include "ppc_asm.h"
>  
>  RELA = 7
> -RELACOUNT = 0x6ffffff9
> +RELASZ = 8
> +RELAENT = 9
>  
>  	.data
>  	/* A procedure descriptor used when booting this as a COFF file.
> @@ -75,34 +76,38 @@ 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	111f
> +	lwz	r0,4(r12)       /* get RELASZ value in r0 */
> +	b	12f
> +111:	cmpwi	r8,RELAENT

Can you use named local labels for new labels you introduce?

This could be .Lcheck_for_relaent: perhaps.

>  	bne	12f
> -	lwz	r0,4(r12)	/* get RELACOUNT value in r0 */
> +	lwz     r14,4(r12)      /* get RELAENT value in r14 */
>  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. */
> +	 * which need to be initialized with addend + offset */
>  10:	/* skip relocation if we don't have both */
>  	cmpwi	r0,0
>  	beq	3f
>  	cmpwi	r9,0
>  	beq	3f
> +	cmpwi	r14,0
> +	beq	3f
>  
>  	add	r9,r9,r11	/* Relocate RELA pointer */
> +	divd    r0,r0,r14       /* RELASZ / RELAENT */

This is in the 32-bit portion isn't it. AFAIK 32-bit CPUs don't
implement divd. I'm not sure why the toolchain allowed it. I would
expect it to trap if run on real 32-bit hardware.

>  	mtctr	r0
>  2:	lbz	r0,4+3(r9)	/* ELF32_R_INFO(reloc->r_info) */
>  	cmpwi	r0,22		/* R_PPC_RELATIVE */
> -	bne	3f
> +	bne	22f
>  	lwz	r12,0(r9)	/* reloc->r_offset */
>  	lwz	r0,8(r9)	/* reloc->r_addend */
>  	add	r0,r0,r11
>  	stwx	r0,r11,r12
> -	addi	r9,r9,12
> +22:	add	r9,r9,r14
>  	bdnz	2b
>  
>  	/* Do a cache flush for our text, in case the loader didn't */

cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH kernel] powerpc/boot: Stop using RELACOUNT
  2022-03-22  2:12 ` Michael Ellerman
@ 2022-03-23  0:18   ` Alexey Kardashevskiy
  2022-03-23  6:04     ` Michael Ellerman
  2022-03-23  7:50     ` Gabriel Paubert
  0 siblings, 2 replies; 5+ messages in thread
From: Alexey Kardashevskiy @ 2022-03-23  0:18 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
  Cc: Nathan Chancellor, Nick Desaulniers, Fangrui Song



On 3/22/22 13:12, Michael Ellerman wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> So far the RELACOUNT tag from the ELF header was containing the exact
>> number of R_PPC_RELATIVE/R_PPC64_RELATIVE relocations. However the LLVM's
>> recent change [1] make it equal-or-less than the actual number which
>> makes it useless.
>>
>> This replaces RELACOUNT in zImage loader with a pair of RELASZ and RELAENT.
>> The vmlinux relocation code is fixed in [2].
> 
> That's committed so you can say:
>    in commit d79976918852 ("powerpc/64: Add UADDR64 relocation support")
> 
>> To make it more future proof, this walks through the entire .rela.dyn
>> section instead of assuming that the section is sorter by a relocation
>> type. Unlike [1], this does not add unaligned UADDR/UADDR64 relocations
>                  ^
>                  that should be 2?

Yes.

> 
>> as in hardly possible to see those in arch-specific zImage.
> 
> I don't quite parse that. Is it true we can never see them in zImage?
> Maybe it's true that we don't see them in practice.

I can force UADDR64 in zImage as I did for d79976918852 but zImage is 
lot smaller and more arch-specific than vmlinux and so far only 
PRINT_INDEX triggered UADDR64 in vmlinux and chances of the same thing 
happening in zImage are small.


> 
>> [1] https://github.com/llvm/llvm-project/commit/da0e5b885b25cf4
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=d799769188529a
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   arch/powerpc/boot/crt0.S | 43 +++++++++++++++++++++++++---------------
>>   1 file changed, 27 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
>> index feadee18e271..6ea3417da3b7 100644
>> --- a/arch/powerpc/boot/crt0.S
>> +++ b/arch/powerpc/boot/crt0.S
>> @@ -8,7 +8,8 @@
>>   #include "ppc_asm.h"
>>   
>>   RELA = 7
>> -RELACOUNT = 0x6ffffff9
>> +RELASZ = 8
>> +RELAENT = 9
>>   
>>   	.data
>>   	/* A procedure descriptor used when booting this as a COFF file.
>> @@ -75,34 +76,38 @@ 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	111f
>> +	lwz	r0,4(r12)       /* get RELASZ value in r0 */
>> +	b	12f
>> +111:	cmpwi	r8,RELAENT
> 
> Can you use named local labels for new labels you introduce?
> 
> This could be .Lcheck_for_relaent: perhaps.

Then I'll need to rename them all/most and add more noise to the patch 
which reduces chances of it being reviewed. But sure, I can rename labels.



>>   	bne	12f
>> -	lwz	r0,4(r12)	/* get RELACOUNT value in r0 */
>> +	lwz     r14,4(r12)      /* get RELAENT value in r14 */
>>   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. */
>> +	 * which need to be initialized with addend + offset */
>>   10:	/* skip relocation if we don't have both */
>>   	cmpwi	r0,0
>>   	beq	3f
>>   	cmpwi	r9,0
>>   	beq	3f
>> +	cmpwi	r14,0
>> +	beq	3f
>>   
>>   	add	r9,r9,r11	/* Relocate RELA pointer */
>> +	divd    r0,r0,r14       /* RELASZ / RELAENT */
> 
> This is in the 32-bit portion isn't it. AFAIK 32-bit CPUs don't
> implement divd. I'm not sure why the toolchain allowed it. I would
> expect it to trap if run on real 32-bit hardware.


Uff, my bad, "divw", right?

I am guessing it works as zImage for 64bit BigEndian is still ELF32 
which runs in 64bit CPU and I did not test on real PPC32 as I'm not 
quite sure how and I hoped your farm will do this for me :)



>>   	mtctr	r0
>>   2:	lbz	r0,4+3(r9)	/* ELF32_R_INFO(reloc->r_info) */
>>   	cmpwi	r0,22		/* R_PPC_RELATIVE */
>> -	bne	3f
>> +	bne	22f
>>   	lwz	r12,0(r9)	/* reloc->r_offset */
>>   	lwz	r0,8(r9)	/* reloc->r_addend */
>>   	add	r0,r0,r11
>>   	stwx	r0,r11,r12
>> -	addi	r9,r9,12
>> +22:	add	r9,r9,r14
>>   	bdnz	2b
>>   
>>   	/* Do a cache flush for our text, in case the loader didn't */
> 
> cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH kernel] powerpc/boot: Stop using RELACOUNT
  2022-03-23  0:18   ` Alexey Kardashevskiy
@ 2022-03-23  6:04     ` Michael Ellerman
  2022-03-23  7:50     ` Gabriel Paubert
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2022-03-23  6:04 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev
  Cc: Nathan Chancellor, Nick Desaulniers, Fangrui Song

Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 3/22/22 13:12, Michael Ellerman wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>> So far the RELACOUNT tag from the ELF header was containing the exact
>>> number of R_PPC_RELATIVE/R_PPC64_RELATIVE relocations. However the LLVM's
>>> recent change [1] make it equal-or-less than the actual number which
>>> makes it useless.
>>>
>>> This replaces RELACOUNT in zImage loader with a pair of RELASZ and RELAENT.
>>> The vmlinux relocation code is fixed in [2].
>> 
>> That's committed so you can say:
>>    in commit d79976918852 ("powerpc/64: Add UADDR64 relocation support")
>> 
>>> To make it more future proof, this walks through the entire .rela.dyn
>>> section instead of assuming that the section is sorter by a relocation
>>> type. Unlike [1], this does not add unaligned UADDR/UADDR64 relocations
>>                  ^
>>                  that should be 2?
>
> Yes.
>
>> 
>>> as in hardly possible to see those in arch-specific zImage.
>> 
>> I don't quite parse that. Is it true we can never see them in zImage?
>> Maybe it's true that we don't see them in practice.
>
> I can force UADDR64 in zImage as I did for d79976918852 but zImage is 
> lot smaller and more arch-specific than vmlinux and so far only 
> PRINT_INDEX triggered UADDR64 in vmlinux and chances of the same thing 
> happening in zImage are small.

OK. Just update the change log to say something like that. ie. they're
not impossible, but not seen in practice.

>>> @@ -75,34 +76,38 @@ 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	111f
>>> +	lwz	r0,4(r12)       /* get RELASZ value in r0 */
>>> +	b	12f
>>> +111:	cmpwi	r8,RELAENT
>> 
>> Can you use named local labels for new labels you introduce?
>> 
>> This could be .Lcheck_for_relaent: perhaps.
>
> Then I'll need to rename them all/most and add more noise to the patch 
> which reduces chances of it being reviewed. But sure, I can rename labels.

I said for new labels you introduce :) We can do a follow-up to rename
existing labels if we want to.

>>>   	bne	12f
>>> -	lwz	r0,4(r12)	/* get RELACOUNT value in r0 */
>>> +	lwz     r14,4(r12)      /* get RELAENT value in r14 */
>>>   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. */
>>> +	 * which need to be initialized with addend + offset */
>>>   10:	/* skip relocation if we don't have both */
>>>   	cmpwi	r0,0
>>>   	beq	3f
>>>   	cmpwi	r9,0
>>>   	beq	3f
>>> +	cmpwi	r14,0
>>> +	beq	3f
>>>   
>>>   	add	r9,r9,r11	/* Relocate RELA pointer */
>>> +	divd    r0,r0,r14       /* RELASZ / RELAENT */
>> 
>> This is in the 32-bit portion isn't it. AFAIK 32-bit CPUs don't
>> implement divd. I'm not sure why the toolchain allowed it. I would
>> expect it to trap if run on real 32-bit hardware.
>
>
> Uff, my bad, "divw", right?

AFAIK yes.

> I am guessing it works as zImage for 64bit BigEndian is still ELF32 
> which runs in 64bit CPU and I did not test on real PPC32 as I'm not 
> quite sure how and I hoped your farm will do this for me :)

Yeah I was hoping they would catch it too. I build pmac32 which should
build a 32-bit zImage, but I build it with a 64-bit compiler using -m32,
so maybe that's why it's accepted. Or maybe we're passing the wrong
options to the assembler.

I don't have any tests of actually booting a 32-bit zImage, my automated
tests all use the vmlinux.

cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH kernel] powerpc/boot: Stop using RELACOUNT
  2022-03-23  0:18   ` Alexey Kardashevskiy
  2022-03-23  6:04     ` Michael Ellerman
@ 2022-03-23  7:50     ` Gabriel Paubert
  1 sibling, 0 replies; 5+ messages in thread
From: Gabriel Paubert @ 2022-03-23  7:50 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Nathan Chancellor, Nick Desaulniers, linuxppc-dev, Fangrui Song

On Wed, Mar 23, 2022 at 11:18:40AM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 3/22/22 13:12, Michael Ellerman wrote:
> > Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> > > So far the RELACOUNT tag from the ELF header was containing the exact
> > > number of R_PPC_RELATIVE/R_PPC64_RELATIVE relocations. However the LLVM's
> > > recent change [1] make it equal-or-less than the actual number which
> > > makes it useless.
> > > 
> > > This replaces RELACOUNT in zImage loader with a pair of RELASZ and RELAENT.
> > > The vmlinux relocation code is fixed in [2].
> > 
> > That's committed so you can say:
> >    in commit d79976918852 ("powerpc/64: Add UADDR64 relocation support")
> > 
> > > To make it more future proof, this walks through the entire .rela.dyn
> > > section instead of assuming that the section is sorter by a relocation
> > > type. Unlike [1], this does not add unaligned UADDR/UADDR64 relocations
> >                  ^
> >                  that should be 2?
> 
> Yes.
> 
> > 
> > > as in hardly possible to see those in arch-specific zImage.
> > 
> > I don't quite parse that. Is it true we can never see them in zImage?
> > Maybe it's true that we don't see them in practice.
> 
> I can force UADDR64 in zImage as I did for d79976918852 but zImage is lot
> smaller and more arch-specific than vmlinux and so far only PRINT_INDEX
> triggered UADDR64 in vmlinux and chances of the same thing happening in
> zImage are small.
> 
> 
> > 
> > > [1] https://github.com/llvm/llvm-project/commit/da0e5b885b25cf4
> > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=d799769188529a
> > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > > ---
> > >   arch/powerpc/boot/crt0.S | 43 +++++++++++++++++++++++++---------------
> > >   1 file changed, 27 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S
> > > index feadee18e271..6ea3417da3b7 100644
> > > --- a/arch/powerpc/boot/crt0.S
> > > +++ b/arch/powerpc/boot/crt0.S
> > > @@ -8,7 +8,8 @@
> > >   #include "ppc_asm.h"
> > >   RELA = 7
> > > -RELACOUNT = 0x6ffffff9
> > > +RELASZ = 8
> > > +RELAENT = 9
> > >   	.data
> > >   	/* A procedure descriptor used when booting this as a COFF file.
> > > @@ -75,34 +76,38 @@ 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	111f
> > > +	lwz	r0,4(r12)       /* get RELASZ value in r0 */
> > > +	b	12f
> > > +111:	cmpwi	r8,RELAENT
> > 
> > Can you use named local labels for new labels you introduce?
> > 
> > This could be .Lcheck_for_relaent: perhaps.
> 
> Then I'll need to rename them all/most and add more noise to the patch which
> reduces chances of it being reviewed. But sure, I can rename labels.
> 
> 
> 
> > >   	bne	12f
> > > -	lwz	r0,4(r12)	/* get RELACOUNT value in r0 */
> > > +	lwz     r14,4(r12)      /* get RELAENT value in r14 */
> > >   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. */
> > > +	 * which need to be initialized with addend + offset */
> > >   10:	/* skip relocation if we don't have both */
> > >   	cmpwi	r0,0
> > >   	beq	3f
> > >   	cmpwi	r9,0
> > >   	beq	3f
> > > +	cmpwi	r14,0
> > > +	beq	3f
> > >   	add	r9,r9,r11	/* Relocate RELA pointer */
> > > +	divd    r0,r0,r14       /* RELASZ / RELAENT */
> > 
> > This is in the 32-bit portion isn't it. AFAIK 32-bit CPUs don't
> > implement divd. I'm not sure why the toolchain allowed it. I would
> > expect it to trap if run on real 32-bit hardware.
> 
> 
> Uff, my bad, "divw", right?

Or maybe even "divwu", since I suspect both values are unsigned.

Probably does not make a difference in practice.

	Gabriel

> 
> I am guessing it works as zImage for 64bit BigEndian is still ELF32 which
> runs in 64bit CPU and I did not test on real PPC32 as I'm not quite sure how
> and I hoped your farm will do this for me :)
> 
> 
> 
> > >   	mtctr	r0
> > >   2:	lbz	r0,4+3(r9)	/* ELF32_R_INFO(reloc->r_info) */
> > >   	cmpwi	r0,22		/* R_PPC_RELATIVE */
> > > -	bne	3f
> > > +	bne	22f
> > >   	lwz	r12,0(r9)	/* reloc->r_offset */
> > >   	lwz	r0,8(r9)	/* reloc->r_addend */
> > >   	add	r0,r0,r11
> > >   	stwx	r0,r11,r12
> > > -	addi	r9,r9,12
> > > +22:	add	r9,r9,r14
> > >   	bdnz	2b
> > >   	/* Do a cache flush for our text, in case the loader didn't */
> > 
> > cheers
 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-23  7:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-21  7:10 [PATCH kernel] powerpc/boot: Stop using RELACOUNT Alexey Kardashevskiy
2022-03-22  2:12 ` Michael Ellerman
2022-03-23  0:18   ` Alexey Kardashevskiy
2022-03-23  6:04     ` Michael Ellerman
2022-03-23  7:50     ` Gabriel Paubert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).