Segher Boessenkool <segher@xxxxxxxxxxxxxxxxxxx> writes: > On Wed, Aug 05, 2020 at 04:24:16PM +1000, Michael Ellerman wrote: >> Christophe Leroy <christophe.leroy@xxxxxxxxxx> writes: >> > Indeed, 32-bit doesn't have a redzone, so I believe it needs a stack >> > frame whenever it has anything to same. >> >> Yeah OK that would explain it. >> >> > Here is what I have in libc.so: >> > >> > 000fbb60 <__clock_gettime>: >> > fbb60: 94 21 ff e0 stwu r1,-32(r1) > > This is the *only* place where you can use a negative offset from r1: > in the stwu to extend the stack (set up a new stack frame, or make the > current one bigger). (You're talking about 32-bit code here right?) >> > diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h >> > b/arch/powerpc/include/asm/vdso/gettimeofday.h >> > index a0712a6e80d9..0b6fa245d54e 100644 >> > --- a/arch/powerpc/include/asm/vdso/gettimeofday.h >> > +++ b/arch/powerpc/include/asm/vdso/gettimeofday.h >> > @@ -10,6 +10,7 @@ >> > .cfi_startproc >> > PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1) >> > mflr r0 >> > + PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1) >> > .cfi_register lr, r0 >> >> The cfi_register should come directly after the mflr I think. > > That is the idiomatic way to write it, and most obviously correct. But > as long as the value in LR at function entry is available in multiple > places (like, in LR and in R0 here), it is fine to use either for > unwinding. Sometimes you can use this to optimise the unwind tables a > bit -- not really worth it in hand-written code, it's more important to > make it legible ;-) OK. Because LR still holds the LR value until it's clobbered later, by which point the cfi_register has taken effect. But yeah I think for readability it's best to keep the cfi_register next to the mflr. >> >> There's also no code to load/restore the TOC pointer on BE, which I >> >> think we'll need to handle. >> > >> > I see no code in the generated vdso64.so doing anything with r2, but if >> > you think that's needed, just let's do it: >> >> Hmm, true. >> >> The compiler will use the toc for globals (and possibly also for large >> constants?) > > And anything else it bloody well wants to, yeah :-) Haha yeah OK. >> AFAIK there's no way to disable use of the toc, or make it a build error >> if it's needed. > > Yes. > >> At the same time it's much safer for us to just save/restore r2, and >> probably in the noise performance wise. > > If you want a function to be able to work with ABI-compliant code safely > (in all cases), you'll have to make it itself ABI-compliant as well, > yes :-) True. Except this is the VDSO which has previously been a bit wild west as far as ABI goes :) >> So yeah we should probably do as below. > > [ snip ] > > Looks good yes. Thanks for reviewing. cheers
