Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.
From: Michael Ellerman
Date: Wed Aug 05 2020 - 22:03:46 EST
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
> 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.
>> 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.