Alexey Kardashevskiy [off-list ref] writes:

On 3/22/22 13:12, Michael Ellerman wrote:

quoted

Alexey Kardashevskiy [off-list ref] writes:

quoted

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")

quoted

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.

quoted

quoted

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.

quoted

quoted

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

quoted

quoted

  	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