[PATCH kernel] powerpc/boot: Stop using RELACOUNT

[Gabriel Paubert]

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

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