lkml.org 
[lkml]   [2020]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.
Michael Ellerman <mpe@ellerman.id.au> a écrit :

> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> Prepare for switching VDSO to generic C implementation in following
>> patch. Here, we:
>> - Modify __get_datapage() to take an offset
>> - Prepare the helpers to call the C VDSO functions
>> - Prepare the required callbacks for the C VDSO functions
>> - Prepare the clocksource.h files to define VDSO_ARCH_CLOCKMODES
>> - Add the C trampolines to the generic C VDSO functions
>>
>> powerpc is a bit special for VDSO as well as system calls in the
>> way that it requires setting CR SO bit which cannot be done in C.
>> Therefore, entry/exit needs to be performed in ASM.
>>
>> Implementing __arch_get_vdso_data() would clobber the link register,
>> requiring the caller to save it. As the ASM calling function already
>> has to set a stack frame and saves the link register before calling
>> the C vdso function, retriving the vdso data pointer there is lighter.
> ...
>
>> diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h
>> b/arch/powerpc/include/asm/vdso/gettimeofday.h
>> new file mode 100644
>> index 000000000000..4452897f9bd8
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
>> @@ -0,0 +1,175 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __ASM_VDSO_GETTIMEOFDAY_H
>> +#define __ASM_VDSO_GETTIMEOFDAY_H
>> +
>> +#include <asm/ptrace.h>
>> +
>> +#ifdef __ASSEMBLY__
>> +
>> +.macro cvdso_call funct
>> + .cfi_startproc
>> + PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
>> + mflr r0
>> + .cfi_register lr, r0
>> + PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
>
> This doesn't work for me on ppc64(le) with glibc.
>
> glibc doesn't create a stack frame before making the VDSO call, so the
> store of r0 (LR) goes into the caller's frame, corrupting the saved LR,
> leading to an infinite loop.

Where should it be saved if it can't be saved in the standard location ?

>
> This is an example from a statically built program that calls
> clock_gettime():
>
> 0000000010030cb0 <__clock_gettime>:
> 10030cb0: 0e 10 40 3c lis r2,4110
> 10030cb4: 00 7a 42 38 addi r2,r2,31232
> 10030cb8: a6 02 08 7c mflr r0
> 10030cbc: ff ff 22 3d addis r9,r2,-1
> 10030cc0: 58 6d 29 39 addi r9,r9,27992
> 10030cc4: f0 ff c1 fb std r30,-16(r1) <-- redzone store
> 10030cc8: 78 23 9e 7c mr r30,r4
> 10030ccc: f8 ff e1 fb std r31,-8(r1) <-- redzone store
> 10030cd0: 78 1b 7f 7c mr r31,r3
> 10030cd4: 10 00 01 f8 std r0,16(r1) <-- save LR to
> caller's frame
> 10030cd8: 00 00 09 e8 ld r0,0(r9)
> 10030cdc: 00 00 20 2c cmpdi r0,0
> 10030ce0: 50 00 82 41 beq 10030d30 <__clock_gettime+0x80>
> 10030ce4: a6 03 09 7c mtctr r0
> 10030ce8: 21 04 80 4e bctrl <-- vdso call
> 10030cec: 26 00 00 7c mfcr r0
> 10030cf0: 00 10 09 74 andis. r9,r0,4096
> 10030cf4: 78 1b 69 7c mr r9,r3
> 10030cf8: 28 00 82 40 bne 10030d20 <__clock_gettime+0x70>
> 10030cfc: b4 07 23 7d extsw r3,r9
> 10030d00: 10 00 01 e8 ld r0,16(r1) <-- load saved
> LR, since clobbered by the VDSO
> 10030d04: f0 ff c1 eb ld r30,-16(r1)
> 10030d08: f8 ff e1 eb ld r31,-8(r1)
> 10030d0c: a6 03 08 7c mtlr r0 <-- restore LR
> 10030d10: 20 00 80 4e blr <-- jumps to 10030cec
>
>
> I'm kind of confused how it worked for you on 32-bit.

So am I then. I'm away for 3 weeks, summer break. I'll check when I'm back.

>
> There's also no code to load/restore the TOC pointer on BE, which I
> think we'll need to handle.

What does it means exactly ? Just saving r2 all the time ? Is there a
dedicated location in the stack frame for it ? Is that only for 64 be ?

Christophe


\
 
 \ /
  Last update: 2020-07-15 20:46    [W:0.128 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site