On Wed, Mar 23, 2022 at 11:18:40AM +1100, Alexey Kardashevskiy wrote:
>
>
> On 3/22/22 13:12, Michael Ellerman wrote:
> > Alexey Kardashevskiy <a...@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 <a...@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
