[PATCH kernel] powerpc/boot: Stop using RELACOUNT

[Alexey Kardashevskiy]

On 3/22/22 13:12, Michael Ellerman wrote:
> Alexey Kardashevskiy <aik at 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 at 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 at 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