[PATCH kernel] powerpc/boot: Stop using RELACOUNT

[Michael Ellerman]

Alexey Kardashevskiy <aik at ozlabs.ru> writes:
> 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.

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 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.

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