2020-04-28 13:18:38

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 0/8] powerpc: switch VDSO to C implementation

This is the seventh version of a series to switch powerpc VDSO to
generic C implementation.

Main changes since v7 are:
- Added gettime64 on PPC32

This series applies on today's powerpc/merge branch.

See the last patches for details on changes and performance.

Christophe Leroy (8):
powerpc/vdso64: Switch from __get_datapage() to get_datapage inline
macro
powerpc/vdso: Remove __kernel_datapage_offset and simplify
__get_datapage()
powerpc/vdso: Remove unused \tmp param in __get_datapage()
powerpc/processor: Move cpu_relax() into asm/vdso/processor.h
powerpc/vdso: Prepare for switching VDSO to generic C implementation.
powerpc/vdso: Switch VDSO to generic C implementation.
lib/vdso: force inlining of __cvdso_clock_gettime_common()
powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32

arch/powerpc/Kconfig | 2 +
arch/powerpc/include/asm/clocksource.h | 7 +
arch/powerpc/include/asm/processor.h | 10 +-
arch/powerpc/include/asm/vdso/clocksource.h | 7 +
arch/powerpc/include/asm/vdso/gettimeofday.h | 175 +++++++++++
arch/powerpc/include/asm/vdso/processor.h | 23 ++
arch/powerpc/include/asm/vdso/vsyscall.h | 25 ++
arch/powerpc/include/asm/vdso_datapage.h | 50 ++--
arch/powerpc/kernel/asm-offsets.c | 49 +--
arch/powerpc/kernel/time.c | 91 +-----
arch/powerpc/kernel/vdso.c | 58 +---
arch/powerpc/kernel/vdso32/Makefile | 32 +-
arch/powerpc/kernel/vdso32/cacheflush.S | 2 +-
arch/powerpc/kernel/vdso32/config-fake32.h | 34 +++
arch/powerpc/kernel/vdso32/datapage.S | 7 +-
arch/powerpc/kernel/vdso32/gettimeofday.S | 300 +------------------
arch/powerpc/kernel/vdso32/vdso32.lds.S | 8 +-
arch/powerpc/kernel/vdso32/vgettimeofday.c | 35 +++
arch/powerpc/kernel/vdso64/Makefile | 23 +-
arch/powerpc/kernel/vdso64/cacheflush.S | 9 +-
arch/powerpc/kernel/vdso64/datapage.S | 31 +-
arch/powerpc/kernel/vdso64/gettimeofday.S | 243 +--------------
arch/powerpc/kernel/vdso64/vdso64.lds.S | 7 +-
arch/powerpc/kernel/vdso64/vgettimeofday.c | 29 ++
lib/vdso/gettimeofday.c | 2 +-
25 files changed, 460 insertions(+), 799 deletions(-)
create mode 100644 arch/powerpc/include/asm/clocksource.h
create mode 100644 arch/powerpc/include/asm/vdso/clocksource.h
create mode 100644 arch/powerpc/include/asm/vdso/gettimeofday.h
create mode 100644 arch/powerpc/include/asm/vdso/processor.h
create mode 100644 arch/powerpc/include/asm/vdso/vsyscall.h
create mode 100644 arch/powerpc/kernel/vdso32/config-fake32.h
create mode 100644 arch/powerpc/kernel/vdso32/vgettimeofday.c
create mode 100644 arch/powerpc/kernel/vdso64/vgettimeofday.c

--
2.25.0


2020-04-28 13:18:41

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 1/8] powerpc/vdso64: Switch from __get_datapage() to get_datapage inline macro

On the same way as already done on PPC32, drop __get_datapage()
function and use get_datapage inline macro instead.

See commit ec0895f08f99 ("powerpc/vdso32: inline __get_datapage()")

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/vdso64/cacheflush.S | 9 ++++----
arch/powerpc/kernel/vdso64/datapage.S | 28 +++--------------------
arch/powerpc/kernel/vdso64/gettimeofday.S | 8 +++----
3 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/arch/powerpc/kernel/vdso64/cacheflush.S b/arch/powerpc/kernel/vdso64/cacheflush.S
index 526f5ba2593e..cab14324242b 100644
--- a/arch/powerpc/kernel/vdso64/cacheflush.S
+++ b/arch/powerpc/kernel/vdso64/cacheflush.S
@@ -8,6 +8,7 @@
#include <asm/processor.h>
#include <asm/ppc_asm.h>
#include <asm/vdso.h>
+#include <asm/vdso_datapage.h>
#include <asm/asm-offsets.h>

.text
@@ -24,14 +25,12 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
.cfi_startproc
mflr r12
.cfi_register lr,r12
- mr r11,r3
- bl V_LOCAL_FUNC(__get_datapage)
+ get_datapage r10, r0
mtlr r12
- mr r10,r3

lwz r7,CFG_DCACHE_BLOCKSZ(r10)
addi r5,r7,-1
- andc r6,r11,r5 /* round low to line bdy */
+ andc r6,r3,r5 /* round low to line bdy */
subf r8,r6,r4 /* compute length */
add r8,r8,r5 /* ensure we get enough */
lwz r9,CFG_DCACHE_LOGBLOCKSZ(r10)
@@ -48,7 +47,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)

lwz r7,CFG_ICACHE_BLOCKSZ(r10)
addi r5,r7,-1
- andc r6,r11,r5 /* round low to line bdy */
+ andc r6,r3,r5 /* round low to line bdy */
subf r8,r6,r4 /* compute length */
add r8,r8,r5
lwz r9,CFG_ICACHE_LOGBLOCKSZ(r10)
diff --git a/arch/powerpc/kernel/vdso64/datapage.S b/arch/powerpc/kernel/vdso64/datapage.S
index dc84f5ae3802..067247d3efb9 100644
--- a/arch/powerpc/kernel/vdso64/datapage.S
+++ b/arch/powerpc/kernel/vdso64/datapage.S
@@ -10,35 +10,13 @@
#include <asm/asm-offsets.h>
#include <asm/unistd.h>
#include <asm/vdso.h>
+#include <asm/vdso_datapage.h>

.text
.global __kernel_datapage_offset;
__kernel_datapage_offset:
.long 0

-V_FUNCTION_BEGIN(__get_datapage)
- .cfi_startproc
- /* We don't want that exposed or overridable as we want other objects
- * to be able to bl directly to here
- */
- .protected __get_datapage
- .hidden __get_datapage
-
- mflr r0
- .cfi_register lr,r0
-
- bcl 20,31,data_page_branch
-data_page_branch:
- mflr r3
- mtlr r0
- addi r3, r3, __kernel_datapage_offset-data_page_branch
- lwz r0,0(r3)
- .cfi_restore lr
- add r3,r0,r3
- blr
- .cfi_endproc
-V_FUNCTION_END(__get_datapage)
-
/*
* void *__kernel_get_syscall_map(unsigned int *syscall_count) ;
*
@@ -53,7 +31,7 @@ V_FUNCTION_BEGIN(__kernel_get_syscall_map)
mflr r12
.cfi_register lr,r12
mr r4,r3
- bl V_LOCAL_FUNC(__get_datapage)
+ get_datapage r3, r0
mtlr r12
addi r3,r3,CFG_SYSCALL_MAP64
cmpldi cr0,r4,0
@@ -75,7 +53,7 @@ V_FUNCTION_BEGIN(__kernel_get_tbfreq)
.cfi_startproc
mflr r12
.cfi_register lr,r12
- bl V_LOCAL_FUNC(__get_datapage)
+ get_datapage r3, r0
ld r3,CFG_TB_TICKS_PER_SEC(r3)
mtlr r12
crclr cr0*4+so
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
index 1c9a04703250..e54c4ce4d356 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
@@ -26,7 +26,7 @@ V_FUNCTION_BEGIN(__kernel_gettimeofday)

mr r11,r3 /* r11 holds tv */
mr r10,r4 /* r10 holds tz */
- bl V_LOCAL_FUNC(__get_datapage) /* get data page */
+ get_datapage r3, r0
cmpldi r11,0 /* check if tv is NULL */
beq 2f
lis r7,1000000@ha /* load up USEC_PER_SEC */
@@ -71,7 +71,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
mflr r12 /* r12 saves lr */
.cfi_register lr,r12
mr r11,r4 /* r11 saves tp */
- bl V_LOCAL_FUNC(__get_datapage) /* get data page */
+ get_datapage r3, r0
lis r7,NSEC_PER_SEC@h /* want nanoseconds */
ori r7,r7,NSEC_PER_SEC@l
beq cr5,70f
@@ -188,7 +188,7 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)

mflr r12
.cfi_register lr,r12
- bl V_LOCAL_FUNC(__get_datapage)
+ get_datapage r3, r0
lwz r5, CLOCK_HRTIMER_RES(r3)
mtlr r12
li r3,0
@@ -221,7 +221,7 @@ V_FUNCTION_BEGIN(__kernel_time)
.cfi_register lr,r12

mr r11,r3 /* r11 holds t */
- bl V_LOCAL_FUNC(__get_datapage)
+ get_datapage r3, r0

ld r4,STAMP_XTIME_SEC(r3)

--
2.25.0

2020-04-28 13:18:45

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

The VDSO datapage and the text pages are always located immediately
next to each other, so it can be hardcoded without an indirection
through __kernel_datapage_offset

In order to ease things, move the data page in front like other
arches, that way there is no need to know the size of the library
to locate the data page.

Before:
clock-getres-realtime-coarse: vdso: 714 nsec/call
clock-gettime-realtime-coarse: vdso: 792 nsec/call
clock-gettime-realtime: vdso: 1243 nsec/call

After:
clock-getres-realtime-coarse: vdso: 699 nsec/call
clock-gettime-realtime-coarse: vdso: 784 nsec/call
clock-gettime-realtime: vdso: 1231 nsec/call

Signed-off-by: Christophe Leroy <[email protected]>
---
v7:
- Moved the removal of the tmp param of __get_datapage()
in a subsequent patch
- Included the addition of the offset param to __get_datapage()
in the further preparatory patch
---
arch/powerpc/include/asm/vdso_datapage.h | 8 ++--
arch/powerpc/kernel/vdso.c | 53 ++++--------------------
arch/powerpc/kernel/vdso32/datapage.S | 3 --
arch/powerpc/kernel/vdso32/vdso32.lds.S | 7 +---
arch/powerpc/kernel/vdso64/datapage.S | 3 --
arch/powerpc/kernel/vdso64/vdso64.lds.S | 7 +---
6 files changed, 16 insertions(+), 65 deletions(-)

diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
index b9ef6cf50ea5..11886467dfdf 100644
--- a/arch/powerpc/include/asm/vdso_datapage.h
+++ b/arch/powerpc/include/asm/vdso_datapage.h
@@ -118,10 +118,12 @@ extern struct vdso_data *vdso_data;

.macro get_datapage ptr, tmp
bcl 20, 31, .+4
+999:
mflr \ptr
- addi \ptr, \ptr, (__kernel_datapage_offset - (.-4))@l
- lwz \tmp, 0(\ptr)
- add \ptr, \tmp, \ptr
+#if CONFIG_PPC_PAGE_SHIFT > 14
+ addis \ptr, \ptr, (_vdso_datapage - 999b)@ha
+#endif
+ addi \ptr, \ptr, (_vdso_datapage - 999b)@l
.endm

#endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index f38f26e844b6..d33fa22ddbed 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -190,7 +190,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
* install_special_mapping or the perf counter mmap tracking code
* will fail to recognise it as a vDSO (since arch_vma_name fails).
*/
- current->mm->context.vdso_base = vdso_base;
+ current->mm->context.vdso_base = vdso_base + PAGE_SIZE;

/*
* our vma flags don't have VM_WRITE so by default, the process isn't
@@ -482,42 +482,6 @@ static __init void vdso_setup_trampolines(struct lib32_elfinfo *v32,
vdso32_rt_sigtramp = find_function32(v32, "__kernel_sigtramp_rt32");
}

-static __init int vdso_fixup_datapage(struct lib32_elfinfo *v32,
- struct lib64_elfinfo *v64)
-{
-#ifdef CONFIG_VDSO32
- Elf32_Sym *sym32;
-#endif
-#ifdef CONFIG_PPC64
- Elf64_Sym *sym64;
-
- sym64 = find_symbol64(v64, "__kernel_datapage_offset");
- if (sym64 == NULL) {
- printk(KERN_ERR "vDSO64: Can't find symbol "
- "__kernel_datapage_offset !\n");
- return -1;
- }
- *((int *)(vdso64_kbase + sym64->st_value - VDSO64_LBASE)) =
- (vdso64_pages << PAGE_SHIFT) -
- (sym64->st_value - VDSO64_LBASE);
-#endif /* CONFIG_PPC64 */
-
-#ifdef CONFIG_VDSO32
- sym32 = find_symbol32(v32, "__kernel_datapage_offset");
- if (sym32 == NULL) {
- printk(KERN_ERR "vDSO32: Can't find symbol "
- "__kernel_datapage_offset !\n");
- return -1;
- }
- *((int *)(vdso32_kbase + (sym32->st_value - VDSO32_LBASE))) =
- (vdso32_pages << PAGE_SHIFT) -
- (sym32->st_value - VDSO32_LBASE);
-#endif
-
- return 0;
-}
-
-
static __init int vdso_fixup_features(struct lib32_elfinfo *v32,
struct lib64_elfinfo *v64)
{
@@ -618,9 +582,6 @@ static __init int vdso_setup(void)
if (vdso_do_find_sections(&v32, &v64))
return -1;

- if (vdso_fixup_datapage(&v32, &v64))
- return -1;
-
if (vdso_fixup_features(&v32, &v64))
return -1;

@@ -761,26 +722,26 @@ static int __init vdso_init(void)
vdso32_pagelist = kcalloc(vdso32_pages + 2, sizeof(struct page *),
GFP_KERNEL);
BUG_ON(vdso32_pagelist == NULL);
+ vdso32_pagelist[0] = virt_to_page(vdso_data);
for (i = 0; i < vdso32_pages; i++) {
struct page *pg = virt_to_page(vdso32_kbase + i*PAGE_SIZE);
get_page(pg);
- vdso32_pagelist[i] = pg;
+ vdso32_pagelist[i + 1] = pg;
}
- vdso32_pagelist[i++] = virt_to_page(vdso_data);
- vdso32_pagelist[i] = NULL;
+ vdso32_pagelist[i + 1] = NULL;
#endif

#ifdef CONFIG_PPC64
vdso64_pagelist = kcalloc(vdso64_pages + 2, sizeof(struct page *),
GFP_KERNEL);
BUG_ON(vdso64_pagelist == NULL);
+ vdso64_pagelist[0] = virt_to_page(vdso_data);
for (i = 0; i < vdso64_pages; i++) {
struct page *pg = virt_to_page(vdso64_kbase + i*PAGE_SIZE);
get_page(pg);
- vdso64_pagelist[i] = pg;
+ vdso64_pagelist[i + 1] = pg;
}
- vdso64_pagelist[i++] = virt_to_page(vdso_data);
- vdso64_pagelist[i] = NULL;
+ vdso64_pagelist[i + 1] = NULL;
#endif /* CONFIG_PPC64 */

get_page(virt_to_page(vdso_data));
diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
index 217bb630f8f9..5513a4f8253e 100644
--- a/arch/powerpc/kernel/vdso32/datapage.S
+++ b/arch/powerpc/kernel/vdso32/datapage.S
@@ -13,9 +13,6 @@
#include <asm/vdso_datapage.h>

.text
- .global __kernel_datapage_offset;
-__kernel_datapage_offset:
- .long 0

/*
* void *__kernel_get_syscall_map(unsigned int *syscall_count) ;
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 5206c2eb2a1d..6cf729612268 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -4,6 +4,7 @@
* library
*/
#include <asm/vdso.h>
+#include <asm/page.h>

#ifdef __LITTLE_ENDIAN__
OUTPUT_FORMAT("elf32-powerpcle", "elf32-powerpcle", "elf32-powerpcle")
@@ -15,6 +16,7 @@ ENTRY(_start)

SECTIONS
{
+ PROVIDE(_vdso_datapage = . - PAGE_SIZE);
. = VDSO32_LBASE + SIZEOF_HEADERS;

.hash : { *(.hash) } :text
@@ -138,11 +140,6 @@ VERSION
{
VDSO_VERSION_STRING {
global:
- /*
- * Has to be there for the kernel to find
- */
- __kernel_datapage_offset;
-
__kernel_get_syscall_map;
#ifndef CONFIG_PPC_BOOK3S_601
__kernel_gettimeofday;
diff --git a/arch/powerpc/kernel/vdso64/datapage.S b/arch/powerpc/kernel/vdso64/datapage.S
index 067247d3efb9..03bb72c440dc 100644
--- a/arch/powerpc/kernel/vdso64/datapage.S
+++ b/arch/powerpc/kernel/vdso64/datapage.S
@@ -13,9 +13,6 @@
#include <asm/vdso_datapage.h>

.text
-.global __kernel_datapage_offset;
-__kernel_datapage_offset:
- .long 0

/*
* void *__kernel_get_syscall_map(unsigned int *syscall_count) ;
diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
index 256fb9720298..f58c7e2e9cbd 100644
--- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
+++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
@@ -4,6 +4,7 @@
* library
*/
#include <asm/vdso.h>
+#include <asm/page.h>

#ifdef __LITTLE_ENDIAN__
OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
@@ -15,6 +16,7 @@ ENTRY(_start)

SECTIONS
{
+ PROVIDE(_vdso_datapage = . - PAGE_SIZE);
. = VDSO64_LBASE + SIZEOF_HEADERS;

.hash : { *(.hash) } :text
@@ -138,11 +140,6 @@ VERSION
{
VDSO_VERSION_STRING {
global:
- /*
- * Has to be there for the kernel to find
- */
- __kernel_datapage_offset;
-
__kernel_get_syscall_map;
__kernel_gettimeofday;
__kernel_clock_gettime;
--
2.25.0

2020-04-28 13:18:53

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 7/8] lib/vdso: force inlining of __cvdso_clock_gettime_common()

When adding gettime64() to a 32 bit architecture (namely powerpc/32)
it has been noticed that GCC doesn't inline anymore
__cvdso_clock_gettime_common() because it is called twice
(Once by __cvdso_clock_gettime() and once by
__cvdso_clock_gettime32).

This has the effect of seriously degrading the performance:

Before the implementation of gettime64(), gettime() runs in:

clock-gettime-monotonic-raw: vdso: 1003 nsec/call
clock-gettime-monotonic-coarse: vdso: 592 nsec/call
clock-gettime-monotonic: vdso: 942 nsec/call

When adding a gettime64() entry point, the standard gettime()
performance is degraded by 30% to 50%:

clock-gettime-monotonic-raw: vdso: 1300 nsec/call
clock-gettime-monotonic-coarse: vdso: 900 nsec/call
clock-gettime-monotonic: vdso: 1232 nsec/call

Adding __always_inline() to __cvdso_clock_gettime_common()
regains the original performance.

In terms of code size, the inlining increases the code size
by only 176 bytes. This is in the noise for a kernel image.

Signed-off-by: Christophe Leroy <[email protected]>
---
lib/vdso/gettimeofday.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index a2909af4b924..7938d3c4901d 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -210,7 +210,7 @@ static __always_inline int do_coarse(const struct vdso_data *vd, clockid_t clk,
return 0;
}

-static __maybe_unused int
+static __always_inline int
__cvdso_clock_gettime_common(const struct vdso_data *vd, clockid_t clock,
struct __kernel_timespec *ts)
{
--
2.25.0

2020-04-28 13:19:00

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 8/8] powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32

Provides __kernel_clock_gettime64() on vdso32. This is the
64 bits version of __kernel_clock_gettime() which is
y2038 compliant.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/vdso32/gettimeofday.S | 9 +++++++++
arch/powerpc/kernel/vdso32/vdso32.lds.S | 1 +
arch/powerpc/kernel/vdso32/vgettimeofday.c | 6 ++++++
3 files changed, 16 insertions(+)

diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index fd7b01c51281..a6e29f880e0e 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -35,6 +35,15 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
cvdso_call __c_kernel_clock_gettime
V_FUNCTION_END(__kernel_clock_gettime)

+/*
+ * Exact prototype of clock_gettime64()
+ *
+ * int __kernel_clock_gettime64(clockid_t clock_id, struct __timespec64 *ts);
+ *
+ */
+V_FUNCTION_BEGIN(__kernel_clock_gettime64)
+ cvdso_call __c_kernel_clock_gettime64
+V_FUNCTION_END(__kernel_clock_gettime64)

/*
* Exact prototype of clock_getres()
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 6cf729612268..05b462143238 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -144,6 +144,7 @@ VERSION
#ifndef CONFIG_PPC_BOOK3S_601
__kernel_gettimeofday;
__kernel_clock_gettime;
+ __kernel_clock_gettime64;
__kernel_clock_getres;
__kernel_time;
__kernel_get_tbfreq;
diff --git a/arch/powerpc/kernel/vdso32/vgettimeofday.c b/arch/powerpc/kernel/vdso32/vgettimeofday.c
index 0b9ab4c22ef2..f7f71fecf4ed 100644
--- a/arch/powerpc/kernel/vdso32/vgettimeofday.c
+++ b/arch/powerpc/kernel/vdso32/vgettimeofday.c
@@ -11,6 +11,12 @@ int __c_kernel_clock_gettime(clockid_t clock, struct old_timespec32 *ts,
return __cvdso_clock_gettime32_data(vd, clock, ts);
}

+int __c_kernel_clock_gettime64(clockid_t clock, struct __kernel_timespec *ts,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_gettime_data(vd, clock, ts);
+}
+
int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
const struct vdso_data *vd)
{
--
2.25.0

2020-04-28 13:19:22

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

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.

Implement __arch_vdso_capable() and:
- When the timebase is used, make it always return true.
- When the RTC clock is used, make it always return false.

Provide vdso_shift_ns(), as the generic x >> s gives the following
bad result:

18: 35 25 ff e0 addic. r9,r5,-32
1c: 41 80 00 10 blt 2c <shift+0x14>
20: 7c 64 4c 30 srw r4,r3,r9
24: 38 60 00 00 li r3,0
...
2c: 54 69 08 3c rlwinm r9,r3,1,0,30
30: 21 45 00 1f subfic r10,r5,31
34: 7c 84 2c 30 srw r4,r4,r5
38: 7d 29 50 30 slw r9,r9,r10
3c: 7c 63 2c 30 srw r3,r3,r5
40: 7d 24 23 78 or r4,r9,r4

In our case the shift is always <= 32. In addition, the upper 32 bits
of the result are likely nul. Lets GCC know it, it also optimises the
following calculations.

With the patch, we get:
0: 21 25 00 20 subfic r9,r5,32
4: 7c 69 48 30 slw r9,r3,r9
8: 7c 84 2c 30 srw r4,r4,r5
c: 7d 24 23 78 or r4,r9,r4
10: 7c 63 2c 30 srw r3,r3,r5

Signed-off-by: Christophe Leroy <[email protected]>
---
v8:
- New, splitted out of last patch of the series
---
arch/powerpc/include/asm/clocksource.h | 7 +
arch/powerpc/include/asm/vdso/clocksource.h | 7 +
arch/powerpc/include/asm/vdso/gettimeofday.h | 175 +++++++++++++++++++
arch/powerpc/include/asm/vdso_datapage.h | 6 +-
arch/powerpc/kernel/vdso32/vgettimeofday.c | 29 +++
arch/powerpc/kernel/vdso64/vgettimeofday.c | 29 +++
6 files changed, 250 insertions(+), 3 deletions(-)
create mode 100644 arch/powerpc/include/asm/clocksource.h
create mode 100644 arch/powerpc/include/asm/vdso/clocksource.h
create mode 100644 arch/powerpc/include/asm/vdso/gettimeofday.h
create mode 100644 arch/powerpc/kernel/vdso32/vgettimeofday.c
create mode 100644 arch/powerpc/kernel/vdso64/vgettimeofday.c

diff --git a/arch/powerpc/include/asm/clocksource.h b/arch/powerpc/include/asm/clocksource.h
new file mode 100644
index 000000000000..482185566b0c
--- /dev/null
+++ b/arch/powerpc/include/asm/clocksource.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_CLOCKSOURCE_H
+#define _ASM_CLOCKSOURCE_H
+
+#include <asm/vdso/clocksource.h>
+
+#endif
diff --git a/arch/powerpc/include/asm/vdso/clocksource.h b/arch/powerpc/include/asm/vdso/clocksource.h
new file mode 100644
index 000000000000..ec5d672d2569
--- /dev/null
+++ b/arch/powerpc/include/asm/vdso/clocksource.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VDSOCLOCKSOURCE_H
+#define __ASM_VDSOCLOCKSOURCE_H
+
+#define VDSO_ARCH_CLOCKMODES VDSO_CLOCKMODE_ARCHTIMER
+
+#endif
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)
+ get_datapage r5, VDSO_DATA_OFFSET
+ bl \funct
+ PPC_LL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
+ cmpwi r3, 0
+ mtlr r0
+ .cfi_restore lr
+ addi r1, r1, STACK_FRAME_OVERHEAD
+ crclr so
+ beqlr+
+ crset so
+ neg r3, r3
+ blr
+ .cfi_endproc
+.endm
+
+.macro cvdso_call_time 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)
+ get_datapage r4, VDSO_DATA_OFFSET
+ bl \funct
+ PPC_LL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
+ crclr so
+ mtlr r0
+ .cfi_restore lr
+ addi r1, r1, STACK_FRAME_OVERHEAD
+ blr
+ .cfi_endproc
+.endm
+
+#else
+
+#include <asm/time.h>
+#include <asm/unistd.h>
+#include <uapi/linux/time.h>
+
+#define VDSO_HAS_CLOCK_GETRES 1
+
+#define VDSO_HAS_TIME 1
+
+static __always_inline int do_syscall_2(const unsigned long _r0, const unsigned long _r3,
+ const unsigned long _r4)
+{
+ register long r0 asm("r0") = _r0;
+ register unsigned long r3 asm("r3") = _r3;
+ register unsigned long r4 asm("r4") = _r4;
+ register int ret asm ("r3");
+
+ asm volatile(
+ " sc\n"
+ " bns+ 1f\n"
+ " neg %0, %0\n"
+ "1:\n"
+ : "=r" (ret), "+r" (r4), "+r" (r0)
+ : "r" (r3)
+ : "memory", "r5", "r6", "r7", "r8", "r9", "r10", "r11", "r12", "cr0", "ctr");
+
+ return ret;
+}
+
+static __always_inline
+int gettimeofday_fallback(struct __kernel_old_timeval *_tv, struct timezone *_tz)
+{
+ return do_syscall_2(__NR_gettimeofday, (unsigned long)_tv, (unsigned long)_tz);
+}
+
+static __always_inline
+int clock_gettime_fallback(clockid_t _clkid, struct __kernel_timespec *_ts)
+{
+ return do_syscall_2(__NR_clock_gettime, _clkid, (unsigned long)_ts);
+}
+
+static __always_inline
+int clock_getres_fallback(clockid_t _clkid, struct __kernel_timespec *_ts)
+{
+ return do_syscall_2(__NR_clock_getres, _clkid, (unsigned long)_ts);
+}
+
+#ifdef CONFIG_VDSO32
+
+#define BUILD_VDSO32 1
+
+static __always_inline
+int clock_gettime32_fallback(clockid_t _clkid, struct old_timespec32 *_ts)
+{
+ return do_syscall_2(__NR_clock_gettime, _clkid, (unsigned long)_ts);
+}
+
+static __always_inline
+int clock_getres32_fallback(clockid_t _clkid, struct old_timespec32 *_ts)
+{
+ return do_syscall_2(__NR_clock_getres, _clkid, (unsigned long)_ts);
+}
+#endif
+
+static __always_inline u64 __arch_get_hw_counter(s32 clock_mode)
+{
+ return get_tb();
+}
+
+const struct vdso_data *__arch_get_vdso_data(void);
+
+static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
+{
+ return !__USE_RTC();
+}
+#define vdso_clocksource_ok vdso_clocksource_ok
+
+/*
+ * powerpc specific delta calculation.
+ *
+ * This variant removes the masking of the subtraction because the
+ * clocksource mask of all VDSO capable clocksources on powerpc is U64_MAX
+ * which would result in a pointless operation. The compiler cannot
+ * optimize it away as the mask comes from the vdso data and is not compile
+ * time constant.
+ */
+static __always_inline u64 vdso_calc_delta(u64 cycles, u64 last, u64 mask, u32 mult)
+{
+ return (cycles - last) * mult;
+}
+#define vdso_calc_delta vdso_calc_delta
+
+#ifndef __powerpc64__
+static __always_inline u64 vdso_shift_ns(u64 ns, unsigned long shift)
+{
+ u32 hi = ns >> 32;
+ u32 lo = ns;
+
+ lo >>= shift;
+ lo |= hi << (32 - shift);
+ hi >>= shift;
+
+ if (likely(hi == 0))
+ return lo;
+
+ return ((u64)hi << 32) | lo;
+}
+#define vdso_shift_ns vdso_shift_ns
+#endif
+
+#ifdef __powerpc64__
+int __c_kernel_clock_gettime(clockid_t clock, struct __kernel_timespec *ts,
+ const struct vdso_data *vd);
+int __c_kernel_clock_getres(clockid_t clock_id, struct __kernel_timespec *res,
+ const struct vdso_data *vd);
+#else
+int __c_kernel_clock_gettime(clockid_t clock, struct old_timespec32 *ts,
+ const struct vdso_data *vd);
+int __c_kernel_clock_getres(clockid_t clock_id, struct old_timespec32 *res,
+ const struct vdso_data *vd);
+#endif
+int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
+ const struct vdso_data *vd);
+__kernel_old_time_t __c_kernel_time(__kernel_old_time_t *time,
+ const struct vdso_data *vd);
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_GETTIMEOFDAY_H */
diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
index f3d7e4e2a45b..78b8f67e9873 100644
--- a/arch/powerpc/include/asm/vdso_datapage.h
+++ b/arch/powerpc/include/asm/vdso_datapage.h
@@ -116,14 +116,14 @@ extern struct vdso_data *vdso_data;

#else /* __ASSEMBLY__ */

-.macro get_datapage ptr
+.macro get_datapage ptr, offset=0
bcl 20, 31, .+4
999:
mflr \ptr
#if CONFIG_PPC_PAGE_SHIFT > 14
- addis \ptr, \ptr, (_vdso_datapage - 999b)@ha
+ addis \ptr, \ptr, (_vdso_datapage + \offset - 999b)@ha
#endif
- addi \ptr, \ptr, (_vdso_datapage - 999b)@l
+ addi \ptr, \ptr, (_vdso_datapage + \offset - 999b)@l
.endm

#endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/kernel/vdso32/vgettimeofday.c b/arch/powerpc/kernel/vdso32/vgettimeofday.c
new file mode 100644
index 000000000000..0b9ab4c22ef2
--- /dev/null
+++ b/arch/powerpc/kernel/vdso32/vgettimeofday.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Powerpc userspace implementations of gettimeofday() and similar.
+ */
+#include <linux/time.h>
+#include <linux/types.h>
+
+int __c_kernel_clock_gettime(clockid_t clock, struct old_timespec32 *ts,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_gettime32_data(vd, clock, ts);
+}
+
+int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
+ const struct vdso_data *vd)
+{
+ return __cvdso_gettimeofday_data(vd, tv, tz);
+}
+
+int __c_kernel_clock_getres(clockid_t clock_id, struct old_timespec32 *res,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_getres_time32_data(vd, clock_id, res);
+}
+
+__kernel_old_time_t __c_kernel_time(__kernel_old_time_t *time, const struct vdso_data *vd)
+{
+ return __cvdso_time_data(vd, time);
+}
diff --git a/arch/powerpc/kernel/vdso64/vgettimeofday.c b/arch/powerpc/kernel/vdso64/vgettimeofday.c
new file mode 100644
index 000000000000..5b5500058344
--- /dev/null
+++ b/arch/powerpc/kernel/vdso64/vgettimeofday.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Powerpc userspace implementations of gettimeofday() and similar.
+ */
+#include <linux/time.h>
+#include <linux/types.h>
+
+int __c_kernel_clock_gettime(clockid_t clock, struct __kernel_timespec *ts,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_gettime_data(vd, clock, ts);
+}
+
+int __c_kernel_gettimeofday(struct __kernel_old_timeval *tv, struct timezone *tz,
+ const struct vdso_data *vd)
+{
+ return __cvdso_gettimeofday_data(vd, tv, tz);
+}
+
+int __c_kernel_clock_getres(clockid_t clock_id, struct __kernel_timespec *res,
+ const struct vdso_data *vd)
+{
+ return __cvdso_clock_getres_data(vd, clock_id, res);
+}
+
+__kernel_old_time_t __c_kernel_time(__kernel_old_time_t *time, const struct vdso_data *vd)
+{
+ return __cvdso_time_data(vd, time);
+}
--
2.25.0

2020-04-28 13:19:50

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 6/8] powerpc/vdso: Switch VDSO to generic C implementation.

For VDSO32 on PPC64, we create a fake 32 bits config, on the same
principle as MIPS architecture, in order to get the correct parts of
the different asm header files.

With the C VDSO, the performance is slightly lower, but it is worth
it as it will ease maintenance and evolution, and also brings clocks
that are not supported with the ASM VDSO.

On an 8xx at 132 MHz, vdsotest with the ASM VDSO:
gettimeofday: vdso: 828 nsec/call
clock-getres-realtime-coarse: vdso: 391 nsec/call
clock-gettime-realtime-coarse: vdso: 614 nsec/call
clock-getres-realtime: vdso: 460 nsec/call
clock-gettime-realtime: vdso: 876 nsec/call
clock-getres-monotonic-coarse: vdso: 399 nsec/call
clock-gettime-monotonic-coarse: vdso: 691 nsec/call
clock-getres-monotonic: vdso: 460 nsec/call
clock-gettime-monotonic: vdso: 1026 nsec/call

On an 8xx at 132 MHz, vdsotest with the C VDSO:
gettimeofday: vdso: 955 nsec/call
clock-getres-realtime-coarse: vdso: 545 nsec/call
clock-gettime-realtime-coarse: vdso: 592 nsec/call
clock-getres-realtime: vdso: 545 nsec/call
clock-gettime-realtime: vdso: 941 nsec/call
clock-getres-monotonic-coarse: vdso: 545 nsec/call
clock-gettime-monotonic-coarse: vdso: 591 nsec/call
clock-getres-monotonic: vdso: 545 nsec/call
clock-gettime-monotonic: vdso: 940 nsec/call

It is even better for gettime with monotonic clocks.

Unsupported clocks with ASM VDSO:
clock-gettime-boottime: vdso: 3851 nsec/call
clock-gettime-tai: vdso: 3852 nsec/call
clock-gettime-monotonic-raw: vdso: 3396 nsec/call

Same clocks with C VDSO:
clock-gettime-tai: vdso: 941 nsec/call
clock-gettime-monotonic-raw: vdso: 1001 nsec/call
clock-gettime-monotonic-coarse: vdso: 591 nsec/call

On an 8321E at 333 MHz, vdsotest with the ASM VDSO:
gettimeofday: vdso: 220 nsec/call
clock-getres-realtime-coarse: vdso: 102 nsec/call
clock-gettime-realtime-coarse: vdso: 178 nsec/call
clock-getres-realtime: vdso: 129 nsec/call
clock-gettime-realtime: vdso: 235 nsec/call
clock-getres-monotonic-coarse: vdso: 105 nsec/call
clock-gettime-monotonic-coarse: vdso: 208 nsec/call
clock-getres-monotonic: vdso: 129 nsec/call
clock-gettime-monotonic: vdso: 274 nsec/call

On an 8321E at 333 MHz, vdsotest with the C VDSO:
gettimeofday: vdso: 272 nsec/call
clock-getres-realtime-coarse: vdso: 160 nsec/call
clock-gettime-realtime-coarse: vdso: 184 nsec/call
clock-getres-realtime: vdso: 166 nsec/call
clock-gettime-realtime: vdso: 281 nsec/call
clock-getres-monotonic-coarse: vdso: 160 nsec/call
clock-gettime-monotonic-coarse: vdso: 184 nsec/call
clock-getres-monotonic: vdso: 169 nsec/call
clock-gettime-monotonic: vdso: 275 nsec/call

Signed-off-by: Christophe Leroy <[email protected]>
---
v7:
- Split out preparatory changes in a new preceding patch
- Added -fasynchronous-unwind-tables to CC flags.

v6:
- Added missing prototypes in asm/vdso/gettimeofday.h for __c_kernel_ functions.
- Using STACK_FRAME_OVERHEAD instead of INT_FRAME_SIZE
- Rebased on powerpc/merge as of 7 Apr 2020
- Fixed build failure with gcc 9
- Added a patch to create asm/vdso/processor.h and more cpu_relax() in it

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/Kconfig | 2 +
arch/powerpc/include/asm/vdso/vsyscall.h | 25 ++
arch/powerpc/include/asm/vdso_datapage.h | 40 +--
arch/powerpc/kernel/asm-offsets.c | 49 +---
arch/powerpc/kernel/time.c | 91 +------
arch/powerpc/kernel/vdso.c | 5 +-
arch/powerpc/kernel/vdso32/Makefile | 32 ++-
arch/powerpc/kernel/vdso32/config-fake32.h | 34 +++
arch/powerpc/kernel/vdso32/gettimeofday.S | 291 +--------------------
arch/powerpc/kernel/vdso64/Makefile | 23 +-
arch/powerpc/kernel/vdso64/gettimeofday.S | 243 +----------------
11 files changed, 144 insertions(+), 691 deletions(-)
create mode 100644 arch/powerpc/include/asm/vdso/vsyscall.h
create mode 100644 arch/powerpc/kernel/vdso32/config-fake32.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 924c541a9260..4e51454a3e7b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -166,6 +166,7 @@ config PPC
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
+ select GENERIC_GETTIMEOFDAY
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
select HAVE_ARCH_JUMP_LABEL
@@ -197,6 +198,7 @@ config PPC
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_FUNCTION_TRACER
select HAVE_GCC_PLUGINS if GCC_VERSION >= 50200 # plugin support on gcc <= 5.1 is buggy on PPC
+ select HAVE_GENERIC_VDSO
select HAVE_HW_BREAKPOINT if PERF_EVENTS && (PPC_BOOK3S || PPC_8xx)
select HAVE_IDE
select HAVE_IOREMAP_PROT
diff --git a/arch/powerpc/include/asm/vdso/vsyscall.h b/arch/powerpc/include/asm/vdso/vsyscall.h
new file mode 100644
index 000000000000..c56a030c0623
--- /dev/null
+++ b/arch/powerpc/include/asm/vdso/vsyscall.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VDSO_VSYSCALL_H
+#define __ASM_VDSO_VSYSCALL_H
+
+#ifndef __ASSEMBLY__
+
+#include <linux/timekeeper_internal.h>
+#include <asm/vdso_datapage.h>
+
+/*
+ * Update the vDSO data page to keep in sync with kernel timekeeping.
+ */
+static __always_inline
+struct vdso_data *__arch_get_k_vdso_data(void)
+{
+ return vdso_data->data;
+}
+#define __arch_get_k_vdso_data __arch_get_k_vdso_data
+
+/* The asm-generic header needs to be included after the definitions above */
+#include <asm-generic/vdso/vsyscall.h>
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_VSYSCALL_H */
diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
index 78b8f67e9873..de7221be8de4 100644
--- a/arch/powerpc/include/asm/vdso_datapage.h
+++ b/arch/powerpc/include/asm/vdso_datapage.h
@@ -36,6 +36,7 @@

#include <linux/unistd.h>
#include <linux/time.h>
+#include <vdso/datapage.h>

#define SYSCALL_MAP_SIZE ((NR_syscalls + 31) / 32)

@@ -45,7 +46,7 @@

#ifdef CONFIG_PPC64

-struct vdso_data {
+struct vdso_arch_data {
__u8 eye_catcher[16]; /* Eyecatcher: SYSTEMCFG:PPC64 0x00 */
struct { /* Systemcfg version numbers */
__u32 major; /* Major number 0x10 */
@@ -59,13 +60,13 @@ struct vdso_data {
__u32 processor; /* Processor type 0x1C */
__u64 processorCount; /* # of physical processors 0x20 */
__u64 physicalMemorySize; /* Size of real memory(B) 0x28 */
- __u64 tb_orig_stamp; /* Timebase at boot 0x30 */
+ __u64 tb_orig_stamp; /* (NU) Timebase at boot 0x30 */
__u64 tb_ticks_per_sec; /* Timebase tics / sec 0x38 */
- __u64 tb_to_xs; /* Inverse of TB to 2^20 0x40 */
- __u64 stamp_xsec; /* 0x48 */
- __u64 tb_update_count; /* Timebase atomicity ctr 0x50 */
- __u32 tz_minuteswest; /* Minutes west of Greenwich 0x58 */
- __u32 tz_dsttime; /* Type of dst correction 0x5C */
+ __u64 tb_to_xs; /* (NU) Inverse of TB to 2^20 0x40 */
+ __u64 stamp_xsec; /* (NU) 0x48 */
+ __u64 tb_update_count; /* (NU) Timebase atomicity ctr 0x50 */
+ __u32 tz_minuteswest; /* (NU) Min. west of Greenwich 0x58 */
+ __u32 tz_dsttime; /* (NU) Type of dst correction 0x5C */
__u32 dcache_size; /* L1 d-cache size 0x60 */
__u32 dcache_line_size; /* L1 d-cache line size 0x64 */
__u32 icache_size; /* L1 i-cache size 0x68 */
@@ -78,14 +79,10 @@ struct vdso_data {
__u32 icache_block_size; /* L1 i-cache block size */
__u32 dcache_log_block_size; /* L1 d-cache log block size */
__u32 icache_log_block_size; /* L1 i-cache log block size */
- __u32 stamp_sec_fraction; /* fractional seconds of stamp_xtime */
- __s32 wtom_clock_nsec; /* Wall to monotonic clock nsec */
- __s64 wtom_clock_sec; /* Wall to monotonic clock sec */
- __s64 stamp_xtime_sec; /* xtime secs as at tb_orig_stamp */
- __s64 stamp_xtime_nsec; /* xtime nsecs as at tb_orig_stamp */
- __u32 hrtimer_res; /* hrtimer resolution */
__u32 syscall_map_64[SYSCALL_MAP_SIZE]; /* map of syscalls */
__u32 syscall_map_32[SYSCALL_MAP_SIZE]; /* map of syscalls */
+
+ struct vdso_data data[CS_BASES];
};

#else /* CONFIG_PPC64 */
@@ -93,26 +90,15 @@ struct vdso_data {
/*
* And here is the simpler 32 bits version
*/
-struct vdso_data {
- __u64 tb_orig_stamp; /* Timebase at boot 0x30 */
+struct vdso_arch_data {
__u64 tb_ticks_per_sec; /* Timebase tics / sec 0x38 */
- __u64 tb_to_xs; /* Inverse of TB to 2^20 0x40 */
- __u64 stamp_xsec; /* 0x48 */
- __u32 tb_update_count; /* Timebase atomicity ctr 0x50 */
- __u32 tz_minuteswest; /* Minutes west of Greenwich 0x58 */
- __u32 tz_dsttime; /* Type of dst correction 0x5C */
- __s32 wtom_clock_sec; /* Wall to monotonic clock */
- __s32 wtom_clock_nsec;
- __s32 stamp_xtime_sec; /* xtime seconds as at tb_orig_stamp */
- __s32 stamp_xtime_nsec; /* xtime nsecs as at tb_orig_stamp */
- __u32 stamp_sec_fraction; /* fractional seconds of stamp_xtime */
- __u32 hrtimer_res; /* hrtimer resolution */
__u32 syscall_map_32[SYSCALL_MAP_SIZE]; /* map of syscalls */
+ struct vdso_data data[CS_BASES];
};

#endif /* CONFIG_PPC64 */

-extern struct vdso_data *vdso_data;
+extern struct vdso_arch_data *vdso_data;

#else /* __ASSEMBLY__ */

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index fcf24a365fc0..9a9b4a9353ff 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -394,47 +394,16 @@ int main(void)
#endif /* ! CONFIG_PPC64 */

/* datapage offsets for use by vdso */
- OFFSET(CFG_TB_ORIG_STAMP, vdso_data, tb_orig_stamp);
- OFFSET(CFG_TB_TICKS_PER_SEC, vdso_data, tb_ticks_per_sec);
- OFFSET(CFG_TB_TO_XS, vdso_data, tb_to_xs);
- OFFSET(CFG_TB_UPDATE_COUNT, vdso_data, tb_update_count);
- OFFSET(CFG_TZ_MINUTEWEST, vdso_data, tz_minuteswest);
- OFFSET(CFG_TZ_DSTTIME, vdso_data, tz_dsttime);
- OFFSET(CFG_SYSCALL_MAP32, vdso_data, syscall_map_32);
- OFFSET(WTOM_CLOCK_SEC, vdso_data, wtom_clock_sec);
- OFFSET(WTOM_CLOCK_NSEC, vdso_data, wtom_clock_nsec);
- OFFSET(STAMP_XTIME_SEC, vdso_data, stamp_xtime_sec);
- OFFSET(STAMP_XTIME_NSEC, vdso_data, stamp_xtime_nsec);
- OFFSET(STAMP_SEC_FRAC, vdso_data, stamp_sec_fraction);
- OFFSET(CLOCK_HRTIMER_RES, vdso_data, hrtimer_res);
+ OFFSET(VDSO_DATA_OFFSET, vdso_arch_data, data);
+ OFFSET(CFG_TB_TICKS_PER_SEC, vdso_arch_data, tb_ticks_per_sec);
+ OFFSET(CFG_SYSCALL_MAP32, vdso_arch_data, syscall_map_32);
#ifdef CONFIG_PPC64
- OFFSET(CFG_ICACHE_BLOCKSZ, vdso_data, icache_block_size);
- OFFSET(CFG_DCACHE_BLOCKSZ, vdso_data, dcache_block_size);
- OFFSET(CFG_ICACHE_LOGBLOCKSZ, vdso_data, icache_log_block_size);
- OFFSET(CFG_DCACHE_LOGBLOCKSZ, vdso_data, dcache_log_block_size);
- OFFSET(CFG_SYSCALL_MAP64, vdso_data, syscall_map_64);
- OFFSET(TVAL64_TV_SEC, __kernel_old_timeval, tv_sec);
- OFFSET(TVAL64_TV_USEC, __kernel_old_timeval, tv_usec);
-#endif
- OFFSET(TSPC64_TV_SEC, __kernel_timespec, tv_sec);
- OFFSET(TSPC64_TV_NSEC, __kernel_timespec, tv_nsec);
- OFFSET(TVAL32_TV_SEC, old_timeval32, tv_sec);
- OFFSET(TVAL32_TV_USEC, old_timeval32, tv_usec);
- OFFSET(TSPC32_TV_SEC, old_timespec32, tv_sec);
- OFFSET(TSPC32_TV_NSEC, old_timespec32, tv_nsec);
- /* timeval/timezone offsets for use by vdso */
- OFFSET(TZONE_TZ_MINWEST, timezone, tz_minuteswest);
- OFFSET(TZONE_TZ_DSTTIME, timezone, tz_dsttime);
-
- /* Other bits used by the vdso */
- DEFINE(CLOCK_REALTIME, CLOCK_REALTIME);
- DEFINE(CLOCK_MONOTONIC, CLOCK_MONOTONIC);
- DEFINE(CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE);
- DEFINE(CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE);
- DEFINE(CLOCK_MAX, CLOCK_TAI);
- DEFINE(NSEC_PER_SEC, NSEC_PER_SEC);
- DEFINE(EINVAL, EINVAL);
- DEFINE(KTIME_LOW_RES, KTIME_LOW_RES);
+ OFFSET(CFG_ICACHE_BLOCKSZ, vdso_arch_data, icache_block_size);
+ OFFSET(CFG_DCACHE_BLOCKSZ, vdso_arch_data, dcache_block_size);
+ OFFSET(CFG_ICACHE_LOGBLOCKSZ, vdso_arch_data, icache_log_block_size);
+ OFFSET(CFG_DCACHE_LOGBLOCKSZ, vdso_arch_data, dcache_log_block_size);
+ OFFSET(CFG_SYSCALL_MAP64, vdso_arch_data, syscall_map_64);
+#endif

#ifdef CONFIG_BUG
DEFINE(BUG_ENTRY_SIZE, sizeof(struct bug_entry));
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 6fcae436ae51..b63b1f97a1b3 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -91,6 +91,7 @@ static struct clocksource clocksource_timebase = {
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
.mask = CLOCKSOURCE_MASK(64),
.read = timebase_read,
+ .vdso_clock_mode = VDSO_CLOCKMODE_ARCHTIMER,
};

#define DECREMENTER_DEFAULT_MAX 0x7FFFFFFF
@@ -855,95 +856,6 @@ static notrace u64 timebase_read(struct clocksource *cs)
return (u64)get_tb();
}

-
-void update_vsyscall(struct timekeeper *tk)
-{
- struct timespec64 xt;
- struct clocksource *clock = tk->tkr_mono.clock;
- u32 mult = tk->tkr_mono.mult;
- u32 shift = tk->tkr_mono.shift;
- u64 cycle_last = tk->tkr_mono.cycle_last;
- u64 new_tb_to_xs, new_stamp_xsec;
- u64 frac_sec;
-
- if (clock != &clocksource_timebase)
- return;
-
- xt.tv_sec = tk->xtime_sec;
- xt.tv_nsec = (long)(tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift);
-
- /* Make userspace gettimeofday spin until we're done. */
- ++vdso_data->tb_update_count;
- smp_mb();
-
- /*
- * This computes ((2^20 / 1e9) * mult) >> shift as a
- * 0.64 fixed-point fraction.
- * The computation in the else clause below won't overflow
- * (as long as the timebase frequency is >= 1.049 MHz)
- * but loses precision because we lose the low bits of the constant
- * in the shift. Note that 19342813113834067 ~= 2^(20+64) / 1e9.
- * For a shift of 24 the error is about 0.5e-9, or about 0.5ns
- * over a second. (Shift values are usually 22, 23 or 24.)
- * For high frequency clocks such as the 512MHz timebase clock
- * on POWER[6789], the mult value is small (e.g. 32768000)
- * and so we can shift the constant by 16 initially
- * (295147905179 ~= 2^(20+64-16) / 1e9) and then do the
- * remaining shifts after the multiplication, which gives a
- * more accurate result (e.g. with mult = 32768000, shift = 24,
- * the error is only about 1.2e-12, or 0.7ns over 10 minutes).
- */
- if (mult <= 62500000 && clock->shift >= 16)
- new_tb_to_xs = ((u64) mult * 295147905179ULL) >> (clock->shift - 16);
- else
- new_tb_to_xs = (u64) mult * (19342813113834067ULL >> clock->shift);
-
- /*
- * Compute the fractional second in units of 2^-32 seconds.
- * The fractional second is tk->tkr_mono.xtime_nsec >> tk->tkr_mono.shift
- * in nanoseconds, so multiplying that by 2^32 / 1e9 gives
- * it in units of 2^-32 seconds.
- * We assume shift <= 32 because clocks_calc_mult_shift()
- * generates shift values in the range 0 - 32.
- */
- frac_sec = tk->tkr_mono.xtime_nsec << (32 - shift);
- do_div(frac_sec, NSEC_PER_SEC);
-
- /*
- * Work out new stamp_xsec value for any legacy users of systemcfg.
- * stamp_xsec is in units of 2^-20 seconds.
- */
- new_stamp_xsec = frac_sec >> 12;
- new_stamp_xsec += tk->xtime_sec * XSEC_PER_SEC;
-
- /*
- * tb_update_count is used to allow the userspace gettimeofday code
- * to assure itself that it sees a consistent view of the tb_to_xs and
- * stamp_xsec variables. It reads the tb_update_count, then reads
- * tb_to_xs and stamp_xsec and then reads tb_update_count again. If
- * the two values of tb_update_count match and are even then the
- * tb_to_xs and stamp_xsec values are consistent. If not, then it
- * loops back and reads them again until this criteria is met.
- */
- vdso_data->tb_orig_stamp = cycle_last;
- vdso_data->stamp_xsec = new_stamp_xsec;
- vdso_data->tb_to_xs = new_tb_to_xs;
- vdso_data->wtom_clock_sec = tk->wall_to_monotonic.tv_sec;
- vdso_data->wtom_clock_nsec = tk->wall_to_monotonic.tv_nsec;
- vdso_data->stamp_xtime_sec = xt.tv_sec;
- vdso_data->stamp_xtime_nsec = xt.tv_nsec;
- vdso_data->stamp_sec_fraction = frac_sec;
- vdso_data->hrtimer_res = hrtimer_resolution;
- smp_wmb();
- ++(vdso_data->tb_update_count);
-}
-
-void update_vsyscall_tz(void)
-{
- vdso_data->tz_minuteswest = sys_tz.tz_minuteswest;
- vdso_data->tz_dsttime = sys_tz.tz_dsttime;
-}
-
static void __init clocksource_init(void)
{
struct clocksource *clock;
@@ -1113,7 +1025,6 @@ void __init time_init(void)
sys_tz.tz_dsttime = 0;
}

- vdso_data->tb_update_count = 0;
vdso_data->tb_ticks_per_sec = tb_ticks_per_sec;

/* initialise and enable the large decrementer (if we have one) */
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index d33fa22ddbed..6642dbe92946 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -17,6 +17,7 @@
#include <linux/elf.h>
#include <linux/security.h>
#include <linux/memblock.h>
+#include <vdso/datapage.h>

#include <asm/pgtable.h>
#include <asm/processor.h>
@@ -71,10 +72,10 @@ static int vdso_ready;
* with it, it will become dynamically allocated
*/
static union {
- struct vdso_data data;
+ struct vdso_arch_data data;
u8 page[PAGE_SIZE];
} vdso_data_store __page_aligned_data;
-struct vdso_data *vdso_data = &vdso_data_store.data;
+struct vdso_arch_data *vdso_data = &vdso_data_store.data;

/* Format of the patch table */
struct vdso_patch_def
diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
index e147bbdc12cd..b46c21ed9316 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -2,7 +2,23 @@

# List of files in the vdso, has to be asm only for now

-obj-vdso32 = sigtramp.o gettimeofday.o datapage.o cacheflush.o note.o getcpu.o
+ARCH_REL_TYPE_ABS := R_PPC_JUMP_SLOT|R_PPC_GLOB_DAT|R_PPC_ADDR32|R_PPC_ADDR24|R_PPC_ADDR16|R_PPC_ADDR16_LO|R_PPC_ADDR16_HI|R_PPC_ADDR16_HA|R_PPC_ADDR14|R_PPC_ADDR14_BRTAKEN|R_PPC_ADDR14_BRNTAKEN
+include $(srctree)/lib/vdso/Makefile
+
+obj-vdso32 = sigtramp.o datapage.o cacheflush.o note.o getcpu.o $(obj-vdso32-y)
+obj-vdso32 += gettimeofday.o
+
+ifneq ($(c-gettimeofday-y),)
+ ifdef CONFIG_PPC64
+ CFLAGS_vgettimeofday.o += -include $(srctree)/$(src)/config-fake32.h
+ endif
+ CFLAGS_vgettimeofday.o += -include $(c-gettimeofday-y)
+ CFLAGS_vgettimeofday.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
+ CFLAGS_vgettimeofday.o += $(call cc-option, -fno-stack-protector)
+ CFLAGS_vgettimeofday.o += -DDISABLE_BRANCH_PROFILING
+ CFLAGS_vgettimeofday.o += -ffreestanding -fasynchronous-unwind-tables
+ CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE)
+endif

# Build rules

@@ -15,6 +31,7 @@ endif
CC32FLAGS :=
ifdef CONFIG_PPC64
CC32FLAGS += -m32
+KBUILD_CFLAGS := $(filter-out -mcmodel=medium,$(KBUILD_CFLAGS))
endif

targets := $(obj-vdso32) vdso32.so vdso32.so.dbg
@@ -23,6 +40,7 @@ obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
+KASAN_SANITIZE := n

ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
-Wl,-soname=linux-vdso32.so.1 -Wl,--hash-style=both
@@ -36,8 +54,8 @@ CPPFLAGS_vdso32.lds += -P -C -Upowerpc
$(obj)/vdso32_wrapper.o : $(obj)/vdso32.so

# link rule for the .so file, .lds has to be first
-$(obj)/vdso32.so.dbg: $(src)/vdso32.lds $(obj-vdso32) FORCE
- $(call if_changed,vdso32ld)
+$(obj)/vdso32.so.dbg: $(src)/vdso32.lds $(obj-vdso32) $(obj)/vgettimeofday.o FORCE
+ $(call if_changed,vdso32ld_and_check)

# strip rule for the .so file
$(obj)/%.so: OBJCOPYFLAGS := -S
@@ -47,12 +65,16 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
# assembly rules for the .S files
$(obj-vdso32): %.o: %.S FORCE
$(call if_changed_dep,vdso32as)
+$(obj)/vgettimeofday.o: %.o: %.c FORCE
+ $(call if_changed_dep,vdso32cc)

# actual build commands
-quiet_cmd_vdso32ld = VDSO32L $@
- cmd_vdso32ld = $(VDSOCC) $(c_flags) $(CC32FLAGS) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^)
+quiet_cmd_vdso32ld_and_check = VDSO32L $@
+ cmd_vdso32ld_and_check = $(VDSOCC) $(c_flags) $(CC32FLAGS) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) ; $(cmd_vdso_check)
quiet_cmd_vdso32as = VDSO32A $@
cmd_vdso32as = $(VDSOCC) $(a_flags) $(CC32FLAGS) -c -o $@ $<
+quiet_cmd_vdso32cc = VDSO32C $@
+ cmd_vdso32cc = $(VDSOCC) $(c_flags) $(CC32FLAGS) -c -o $@ $<

# install commands for the unstripped file
quiet_cmd_vdso_install = INSTALL $@
diff --git a/arch/powerpc/kernel/vdso32/config-fake32.h b/arch/powerpc/kernel/vdso32/config-fake32.h
new file mode 100644
index 000000000000..e16041fc15c9
--- /dev/null
+++ b/arch/powerpc/kernel/vdso32/config-fake32.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * In case of a 32 bit VDSO for a 64 bit kernel fake a 32 bit kernel
+ * configuration.
+ */
+
+#undef CONFIG_PPC64
+#undef CONFIG_64BIT
+#define CONFIG_PPC32
+#define CONFIG_32BIT 1
+#define CONFIG_GENERIC_ATOMIC64 1
+
+#ifdef CONFIG_PPC_BOOK3S_64
+#undef CONFIG_PPC_BOOK3S_64
+#undef CONFIG_PPC_PSERIES
+#define CONFIG_PPC_BOOK3S_32
+#else
+#define CONFIG_PPC_MMU_NOHASH_32
+#define CONFIG_FSL_BOOKE
+#endif
+
+#define CONFIG_TASK_SIZE 0
+#undef CONFIG_MMIOWB
+#undef CONFIG_PPC_SPLPAR
+#undef CONFIG_SPARSEMEM
+#undef CONFIG_PGTABLE_LEVELS
+#define CONFIG_PGTABLE_LEVELS 2
+#undef CONFIG_TRANSPARENT_HUGEPAGE
+#undef CONFIG_SPARSEMEM_VMEMMAP
+#undef CONFIG_FLATMEM
+#define CONFIG_FLATMEM
+#undef CONFIG_PPC_INDIRECT_MMIO
+#undef CONFIG_PPC_INDIRECT_PIO
+#undef CONFIG_EEH
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index 0bbdce0f2a9c..fd7b01c51281 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -12,13 +12,7 @@
#include <asm/vdso_datapage.h>
#include <asm/asm-offsets.h>
#include <asm/unistd.h>
-
-/* Offset for the low 32-bit part of a field of long type */
-#ifdef CONFIG_PPC64
-#define LOPART 4
-#else
-#define LOPART 0
-#endif
+#include <asm/vdso/gettimeofday.h>

.text
/*
@@ -28,32 +22,7 @@
*
*/
V_FUNCTION_BEGIN(__kernel_gettimeofday)
- .cfi_startproc
- mflr r12
- .cfi_register lr,r12
-
- mr. r10,r3 /* r10 saves tv */
- mr r11,r4 /* r11 saves tz */
- get_datapage r9
- beq 3f
- LOAD_REG_IMMEDIATE(r7, 1000000) /* load up USEC_PER_SEC */
- bl __do_get_tspec@local /* get sec/usec from tb & kernel */
- stw r3,TVAL32_TV_SEC(r10)
- stw r4,TVAL32_TV_USEC(r10)
-
-3: cmplwi r11,0 /* check if tz is NULL */
- mtlr r12
- crclr cr0*4+so
- li r3,0
- beqlr
-
- lwz r4,CFG_TZ_MINUTEWEST(r9)/* fill tz */
- lwz r5,CFG_TZ_DSTTIME(r9)
- stw r4,TZONE_TZ_MINWEST(r11)
- stw r5,TZONE_TZ_DSTTIME(r11)
-
- blr
- .cfi_endproc
+ cvdso_call __c_kernel_gettimeofday
V_FUNCTION_END(__kernel_gettimeofday)

/*
@@ -63,127 +32,7 @@ V_FUNCTION_END(__kernel_gettimeofday)
*
*/
V_FUNCTION_BEGIN(__kernel_clock_gettime)
- .cfi_startproc
- /* Check for supported clock IDs */
- cmpli cr0,r3,CLOCK_REALTIME
- cmpli cr1,r3,CLOCK_MONOTONIC
- cror cr0*4+eq,cr0*4+eq,cr1*4+eq
-
- cmpli cr5,r3,CLOCK_REALTIME_COARSE
- cmpli cr6,r3,CLOCK_MONOTONIC_COARSE
- cror cr5*4+eq,cr5*4+eq,cr6*4+eq
-
- cror cr0*4+eq,cr0*4+eq,cr5*4+eq
- bne cr0, .Lgettime_fallback
-
- mflr r12 /* r12 saves lr */
- .cfi_register lr,r12
- mr r11,r4 /* r11 saves tp */
- get_datapage r9
- LOAD_REG_IMMEDIATE(r7, NSEC_PER_SEC) /* load up NSEC_PER_SEC */
- beq cr5, .Lcoarse_clocks
-.Lprecise_clocks:
- bl __do_get_tspec@local /* get sec/nsec from tb & kernel */
- bne cr1, .Lfinish /* not monotonic -> all done */
-
- /*
- * CLOCK_MONOTONIC
- */
-
- /* now we must fixup using wall to monotonic. We need to snapshot
- * that value and do the counter trick again. Fortunately, we still
- * have the counter value in r8 that was returned by __do_get_xsec.
- * At this point, r3,r4 contain our sec/nsec values, r5 and r6
- * can be used, r7 contains NSEC_PER_SEC.
- */
-
- lwz r5,(WTOM_CLOCK_SEC+LOPART)(r9)
- lwz r6,WTOM_CLOCK_NSEC(r9)
-
- /* We now have our offset in r5,r6. We create a fake dependency
- * on that value and re-check the counter
- */
- or r0,r6,r5
- xor r0,r0,r0
- add r9,r9,r0
- lwz r0,(CFG_TB_UPDATE_COUNT+LOPART)(r9)
- cmpl cr0,r8,r0 /* check if updated */
- bne- .Lprecise_clocks
- b .Lfinish_monotonic
-
- /*
- * For coarse clocks we get data directly from the vdso data page, so
- * we don't need to call __do_get_tspec, but we still need to do the
- * counter trick.
- */
-.Lcoarse_clocks:
- lwz r8,(CFG_TB_UPDATE_COUNT+LOPART)(r9)
- andi. r0,r8,1 /* pending update ? loop */
- bne- .Lcoarse_clocks
- add r9,r9,r0 /* r0 is already 0 */
-
- /*
- * CLOCK_REALTIME_COARSE, below values are needed for MONOTONIC_COARSE
- * too
- */
- lwz r3,STAMP_XTIME_SEC+LOPART(r9)
- lwz r4,STAMP_XTIME_NSEC+LOPART(r9)
- bne cr6,1f
-
- /* CLOCK_MONOTONIC_COARSE */
- lwz r5,(WTOM_CLOCK_SEC+LOPART)(r9)
- lwz r6,WTOM_CLOCK_NSEC(r9)
-
- /* check if counter has updated */
- or r0,r6,r5
-1: or r0,r0,r3
- or r0,r0,r4
- xor r0,r0,r0
- add r3,r3,r0
- lwz r0,CFG_TB_UPDATE_COUNT+LOPART(r9)
- cmpl cr0,r0,r8 /* check if updated */
- bne- .Lcoarse_clocks
-
- /* Counter has not updated, so continue calculating proper values for
- * sec and nsec if monotonic coarse, or just return with the proper
- * values for realtime.
- */
- bne cr6, .Lfinish
-
- /* Calculate and store result. Note that this mimics the C code,
- * which may cause funny results if nsec goes negative... is that
- * possible at all ?
- */
-.Lfinish_monotonic:
- add r3,r3,r5
- add r4,r4,r6
- cmpw cr0,r4,r7
- cmpwi cr1,r4,0
- blt 1f
- subf r4,r7,r4
- addi r3,r3,1
-1: bge cr1, .Lfinish
- addi r3,r3,-1
- add r4,r4,r7
-
-.Lfinish:
- stw r3,TSPC32_TV_SEC(r11)
- stw r4,TSPC32_TV_NSEC(r11)
-
- mtlr r12
- crclr cr0*4+so
- li r3,0
- blr
-
- /*
- * syscall fallback
- */
-.Lgettime_fallback:
- li r0,__NR_clock_gettime
- .cfi_restore lr
- sc
- blr
- .cfi_endproc
+ cvdso_call __c_kernel_clock_gettime
V_FUNCTION_END(__kernel_clock_gettime)


@@ -194,37 +43,7 @@ V_FUNCTION_END(__kernel_clock_gettime)
*
*/
V_FUNCTION_BEGIN(__kernel_clock_getres)
- .cfi_startproc
- /* Check for supported clock IDs */
- cmplwi cr0, r3, CLOCK_MAX
- cmpwi cr1, r3, CLOCK_REALTIME_COARSE
- cmpwi cr7, r3, CLOCK_MONOTONIC_COARSE
- bgt cr0, 99f
- LOAD_REG_IMMEDIATE(r5, KTIME_LOW_RES)
- beq cr1, 1f
- beq cr7, 1f
-
- mflr r12
- .cfi_register lr,r12
- get_datapage r3
- lwz r5, CLOCK_HRTIMER_RES(r3)
- mtlr r12
-1: li r3,0
- cmpli cr0,r4,0
- crclr cr0*4+so
- beqlr
- stw r3,TSPC32_TV_SEC(r4)
- stw r5,TSPC32_TV_NSEC(r4)
- blr
-
- /*
- * invalid clock
- */
-99:
- li r3, EINVAL
- crset so
- blr
- .cfi_endproc
+ cvdso_call __c_kernel_clock_getres
V_FUNCTION_END(__kernel_clock_getres)


@@ -235,105 +54,5 @@ V_FUNCTION_END(__kernel_clock_getres)
*
*/
V_FUNCTION_BEGIN(__kernel_time)
- .cfi_startproc
- mflr r12
- .cfi_register lr,r12
-
- mr r11,r3 /* r11 holds t */
- get_datapage r9
-
- lwz r3,STAMP_XTIME_SEC+LOPART(r9)
-
- cmplwi r11,0 /* check if t is NULL */
- mtlr r12
- crclr cr0*4+so
- beqlr
- stw r3,0(r11) /* store result at *t */
- blr
- .cfi_endproc
+ cvdso_call_time __c_kernel_time
V_FUNCTION_END(__kernel_time)
-
-/*
- * This is the core of clock_gettime() and gettimeofday(),
- * it returns the current time in r3 (seconds) and r4.
- * On entry, r7 gives the resolution of r4, either USEC_PER_SEC
- * or NSEC_PER_SEC, giving r4 in microseconds or nanoseconds.
- * It expects the datapage ptr in r9 and doesn't clobber it.
- * It clobbers r0, r5 and r6.
- * On return, r8 contains the counter value that can be reused.
- * This clobbers cr0 but not any other cr field.
- */
-__do_get_tspec:
- .cfi_startproc
- /* Check for update count & load values. We use the low
- * order 32 bits of the update count
- */
-1: lwz r8,(CFG_TB_UPDATE_COUNT+LOPART)(r9)
- andi. r0,r8,1 /* pending update ? loop */
- bne- 1b
- xor r0,r8,r8 /* create dependency */
- add r9,r9,r0
-
- /* Load orig stamp (offset to TB) */
- lwz r5,CFG_TB_ORIG_STAMP(r9)
- lwz r6,(CFG_TB_ORIG_STAMP+4)(r9)
-
- /* Get a stable TB value */
-2: MFTBU(r3)
- MFTBL(r4)
- MFTBU(r0)
- cmplw cr0,r3,r0
- bne- 2b
-
- /* Subtract tb orig stamp and shift left 12 bits.
- */
- subfc r4,r6,r4
- subfe r0,r5,r3
- slwi r0,r0,12
- rlwimi. r0,r4,12,20,31
- slwi r4,r4,12
-
- /*
- * Load scale factor & do multiplication.
- * We only use the high 32 bits of the tb_to_xs value.
- * Even with a 1GHz timebase clock, the high 32 bits of
- * tb_to_xs will be at least 4 million, so the error from
- * ignoring the low 32 bits will be no more than 0.25ppm.
- * The error will just make the clock run very very slightly
- * slow until the next time the kernel updates the VDSO data,
- * at which point the clock will catch up to the kernel's value,
- * so there is no long-term error accumulation.
- */
- lwz r5,CFG_TB_TO_XS(r9) /* load values */
- mulhwu r4,r4,r5
- li r3,0
-
- beq+ 4f /* skip high part computation if 0 */
- mulhwu r3,r0,r5
- mullw r5,r0,r5
- addc r4,r4,r5
- addze r3,r3
-4:
- /* At this point, we have seconds since the xtime stamp
- * as a 32.32 fixed-point number in r3 and r4.
- * Load & add the xtime stamp.
- */
- lwz r5,STAMP_XTIME_SEC+LOPART(r9)
- lwz r6,STAMP_SEC_FRAC(r9)
- addc r4,r4,r6
- adde r3,r3,r5
-
- /* We create a fake dependency on the result in r3/r4
- * and re-check the counter
- */
- or r6,r4,r3
- xor r0,r6,r6
- add r9,r9,r0
- lwz r0,(CFG_TB_UPDATE_COUNT+LOPART)(r9)
- cmplw cr0,r8,r0 /* check if updated */
- bne- 1b
-
- mulhwu r4,r4,r7 /* convert to micro or nanoseconds */
-
- blr
- .cfi_endproc
diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
index 32ebb3522ea1..29bffd4863f2 100644
--- a/arch/powerpc/kernel/vdso64/Makefile
+++ b/arch/powerpc/kernel/vdso64/Makefile
@@ -1,8 +1,20 @@
# SPDX-License-Identifier: GPL-2.0
# List of files in the vdso, has to be asm only for now

+ARCH_REL_TYPE_ABS := R_PPC_JUMP_SLOT|R_PPC_GLOB_DAT|R_PPC_ADDR32|R_PPC_ADDR24|R_PPC_ADDR16|R_PPC_ADDR16_LO|R_PPC_ADDR16_HI|R_PPC_ADDR16_HA|R_PPC_ADDR14|R_PPC_ADDR14_BRTAKEN|R_PPC_ADDR14_BRNTAKEN
+include $(srctree)/lib/vdso/Makefile
+
obj-vdso64 = sigtramp.o gettimeofday.o datapage.o cacheflush.o note.o getcpu.o

+ifneq ($(c-gettimeofday-y),)
+ CFLAGS_vgettimeofday.o += -include $(c-gettimeofday-y)
+ CFLAGS_vgettimeofday.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
+ CFLAGS_vgettimeofday.o += $(call cc-option, -fno-stack-protector)
+ CFLAGS_vgettimeofday.o += -DDISABLE_BRANCH_PROFILING
+ CFLAGS_vgettimeofday.o += -ffreestanding -fasynchronous-unwind-tables
+ CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE)
+endif
+
# Build rules

targets := $(obj-vdso64) vdso64.so vdso64.so.dbg
@@ -11,6 +23,7 @@ obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
+KASAN_SANITIZE := n

ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
-Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
@@ -20,12 +33,14 @@ obj-y += vdso64_wrapper.o
extra-y += vdso64.lds
CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)

+$(obj)/vgettimeofday.o: %.o: %.c FORCE
+
# Force dependency (incbin is bad)
$(obj)/vdso64_wrapper.o : $(obj)/vdso64.so

# link rule for the .so file, .lds has to be first
-$(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) FORCE
- $(call if_changed,vdso64ld)
+$(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) $(obj)/vgettimeofday.o FORCE
+ $(call if_changed,vdso64ld_and_check)

# strip rule for the .so file
$(obj)/%.so: OBJCOPYFLAGS := -S
@@ -33,8 +48,8 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
$(call if_changed,objcopy)

# actual build commands
-quiet_cmd_vdso64ld = VDSO64L $@
- cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^)
+quiet_cmd_vdso64ld_and_check = VDSO64L $@
+ cmd_vdso64ld_and_check = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) ; $(cmd_vdso_check)

# install commands for the unstripped file
quiet_cmd_vdso_install = INSTALL $@
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
index 275f031d0bf1..d7a7bfb51081 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
@@ -9,8 +9,10 @@
#include <asm/processor.h>
#include <asm/ppc_asm.h>
#include <asm/vdso.h>
+#include <asm/vdso_datapage.h>
#include <asm/asm-offsets.h>
#include <asm/unistd.h>
+#include <asm/vdso/gettimeofday.h>

.text
/*
@@ -20,31 +22,7 @@
*
*/
V_FUNCTION_BEGIN(__kernel_gettimeofday)
- .cfi_startproc
- mflr r12
- .cfi_register lr,r12
-
- mr r11,r3 /* r11 holds tv */
- mr r10,r4 /* r10 holds tz */
- get_datapage r3
- cmpldi r11,0 /* check if tv is NULL */
- beq 2f
- lis r7,1000000@ha /* load up USEC_PER_SEC */
- addi r7,r7,1000000@l
- bl V_LOCAL_FUNC(__do_get_tspec) /* get sec/us from tb & kernel */
- std r4,TVAL64_TV_SEC(r11) /* store sec in tv */
- std r5,TVAL64_TV_USEC(r11) /* store usec in tv */
-2: cmpldi r10,0 /* check if tz is NULL */
- beq 1f
- lwz r4,CFG_TZ_MINUTEWEST(r3)/* fill tz */
- lwz r5,CFG_TZ_DSTTIME(r3)
- stw r4,TZONE_TZ_MINWEST(r10)
- stw r5,TZONE_TZ_DSTTIME(r10)
-1: mtlr r12
- crclr cr0*4+so
- li r3,0 /* always success */
- blr
- .cfi_endproc
+ cvdso_call __c_kernel_gettimeofday
V_FUNCTION_END(__kernel_gettimeofday)


@@ -55,120 +33,7 @@ V_FUNCTION_END(__kernel_gettimeofday)
*
*/
V_FUNCTION_BEGIN(__kernel_clock_gettime)
- .cfi_startproc
- /* Check for supported clock IDs */
- cmpwi cr0,r3,CLOCK_REALTIME
- cmpwi cr1,r3,CLOCK_MONOTONIC
- cror cr0*4+eq,cr0*4+eq,cr1*4+eq
-
- cmpwi cr5,r3,CLOCK_REALTIME_COARSE
- cmpwi cr6,r3,CLOCK_MONOTONIC_COARSE
- cror cr5*4+eq,cr5*4+eq,cr6*4+eq
-
- cror cr0*4+eq,cr0*4+eq,cr5*4+eq
- bne cr0,99f
-
- mflr r12 /* r12 saves lr */
- .cfi_register lr,r12
- mr r11,r4 /* r11 saves tp */
- get_datapage r3
- lis r7,NSEC_PER_SEC@h /* want nanoseconds */
- ori r7,r7,NSEC_PER_SEC@l
- beq cr5,70f
-50: bl V_LOCAL_FUNC(__do_get_tspec) /* get time from tb & kernel */
- bne cr1,80f /* if not monotonic, all done */
-
- /*
- * CLOCK_MONOTONIC
- */
-
- /* now we must fixup using wall to monotonic. We need to snapshot
- * that value and do the counter trick again. Fortunately, we still
- * have the counter value in r8 that was returned by __do_get_tspec.
- * At this point, r4,r5 contain our sec/nsec values.
- */
-
- ld r6,WTOM_CLOCK_SEC(r3)
- lwa r9,WTOM_CLOCK_NSEC(r3)
-
- /* We now have our result in r6,r9. We create a fake dependency
- * on that result and re-check the counter
- */
- or r0,r6,r9
- xor r0,r0,r0
- add r3,r3,r0
- ld r0,CFG_TB_UPDATE_COUNT(r3)
- cmpld cr0,r0,r8 /* check if updated */
- bne- 50b
- b 78f
-
- /*
- * For coarse clocks we get data directly from the vdso data page, so
- * we don't need to call __do_get_tspec, but we still need to do the
- * counter trick.
- */
-70: ld r8,CFG_TB_UPDATE_COUNT(r3)
- andi. r0,r8,1 /* pending update ? loop */
- bne- 70b
- add r3,r3,r0 /* r0 is already 0 */
-
- /*
- * CLOCK_REALTIME_COARSE, below values are needed for MONOTONIC_COARSE
- * too
- */
- ld r4,STAMP_XTIME_SEC(r3)
- ld r5,STAMP_XTIME_NSEC(r3)
- bne cr6,75f
-
- /* CLOCK_MONOTONIC_COARSE */
- ld r6,WTOM_CLOCK_SEC(r3)
- lwa r9,WTOM_CLOCK_NSEC(r3)
-
- /* check if counter has updated */
- or r0,r6,r9
-75: or r0,r0,r4
- or r0,r0,r5
- xor r0,r0,r0
- add r3,r3,r0
- ld r0,CFG_TB_UPDATE_COUNT(r3)
- cmpld cr0,r0,r8 /* check if updated */
- bne- 70b
-
- /* Counter has not updated, so continue calculating proper values for
- * sec and nsec if monotonic coarse, or just return with the proper
- * values for realtime.
- */
- bne cr6,80f
-
- /* Add wall->monotonic offset and check for overflow or underflow */
-78: add r4,r4,r6
- add r5,r5,r9
- cmpd cr0,r5,r7
- cmpdi cr1,r5,0
- blt 79f
- subf r5,r7,r5
- addi r4,r4,1
-79: bge cr1,80f
- addi r4,r4,-1
- add r5,r5,r7
-
-80: std r4,TSPC64_TV_SEC(r11)
- std r5,TSPC64_TV_NSEC(r11)
-
- mtlr r12
- crclr cr0*4+so
- li r3,0
- blr
-
- /*
- * syscall fallback
- */
-99:
- li r0,__NR_clock_gettime
- .cfi_restore lr
- sc
- blr
- .cfi_endproc
+ cvdso_call __c_kernel_clock_gettime
V_FUNCTION_END(__kernel_clock_gettime)


@@ -179,34 +44,7 @@ V_FUNCTION_END(__kernel_clock_gettime)
*
*/
V_FUNCTION_BEGIN(__kernel_clock_getres)
- .cfi_startproc
- /* Check for supported clock IDs */
- cmpwi cr0,r3,CLOCK_REALTIME
- cmpwi cr1,r3,CLOCK_MONOTONIC
- cror cr0*4+eq,cr0*4+eq,cr1*4+eq
- bne cr0,99f
-
- mflr r12
- .cfi_register lr,r12
- get_datapage r3
- lwz r5, CLOCK_HRTIMER_RES(r3)
- mtlr r12
- li r3,0
- cmpldi cr0,r4,0
- crclr cr0*4+so
- beqlr
- std r3,TSPC64_TV_SEC(r4)
- std r5,TSPC64_TV_NSEC(r4)
- blr
-
- /*
- * syscall fallback
- */
-99:
- li r0,__NR_clock_getres
- sc
- blr
- .cfi_endproc
+ cvdso_call __c_kernel_clock_getres
V_FUNCTION_END(__kernel_clock_getres)

/*
@@ -216,74 +54,5 @@ V_FUNCTION_END(__kernel_clock_getres)
*
*/
V_FUNCTION_BEGIN(__kernel_time)
- .cfi_startproc
- mflr r12
- .cfi_register lr,r12
-
- mr r11,r3 /* r11 holds t */
- get_datapage r3
-
- ld r4,STAMP_XTIME_SEC(r3)
-
- cmpldi r11,0 /* check if t is NULL */
- beq 2f
- std r4,0(r11) /* store result at *t */
-2: mtlr r12
- crclr cr0*4+so
- mr r3,r4
- blr
- .cfi_endproc
+ cvdso_call_time __c_kernel_time
V_FUNCTION_END(__kernel_time)
-
-
-/*
- * This is the core of clock_gettime() and gettimeofday(),
- * it returns the current time in r4 (seconds) and r5.
- * On entry, r7 gives the resolution of r5, either USEC_PER_SEC
- * or NSEC_PER_SEC, giving r5 in microseconds or nanoseconds.
- * It expects the datapage ptr in r3 and doesn't clobber it.
- * It clobbers r0, r6 and r9.
- * On return, r8 contains the counter value that can be reused.
- * This clobbers cr0 but not any other cr field.
- */
-V_FUNCTION_BEGIN(__do_get_tspec)
- .cfi_startproc
- /* check for update count & load values */
-1: ld r8,CFG_TB_UPDATE_COUNT(r3)
- andi. r0,r8,1 /* pending update ? loop */
- bne- 1b
- xor r0,r8,r8 /* create dependency */
- add r3,r3,r0
-
- /* Get TB & offset it. We use the MFTB macro which will generate
- * workaround code for Cell.
- */
- MFTB(r6)
- ld r9,CFG_TB_ORIG_STAMP(r3)
- subf r6,r9,r6
-
- /* Scale result */
- ld r5,CFG_TB_TO_XS(r3)
- sldi r6,r6,12 /* compute time since stamp_xtime */
- mulhdu r6,r6,r5 /* in units of 2^-32 seconds */
-
- /* Add stamp since epoch */
- ld r4,STAMP_XTIME_SEC(r3)
- lwz r5,STAMP_SEC_FRAC(r3)
- or r0,r4,r5
- or r0,r0,r6
- xor r0,r0,r0
- add r3,r3,r0
- ld r0,CFG_TB_UPDATE_COUNT(r3)
- cmpld r0,r8 /* check if updated */
- bne- 1b /* reload if so */
-
- /* convert to seconds & nanoseconds and add to stamp */
- add r6,r6,r5 /* add on fractional seconds of xtime */
- mulhwu r5,r6,r7 /* compute micro or nanoseconds and */
- srdi r6,r6,32 /* seconds since stamp_xtime */
- clrldi r5,r5,32
- add r4,r4,r6
- blr
- .cfi_endproc
-V_FUNCTION_END(__do_get_tspec)
--
2.25.0



2020-04-28 13:20:16

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 3/8] powerpc/vdso: Remove unused \tmp param in __get_datapage()

The \tmp param is not used anymore, remove it.

Signed-off-by: Christophe Leroy <[email protected]>
---
v7: New patch, splitted out of preceding patch
---
arch/powerpc/include/asm/vdso_datapage.h | 2 +-
arch/powerpc/kernel/vdso32/cacheflush.S | 2 +-
arch/powerpc/kernel/vdso32/datapage.S | 4 ++--
arch/powerpc/kernel/vdso32/gettimeofday.S | 8 ++++----
arch/powerpc/kernel/vdso64/cacheflush.S | 2 +-
arch/powerpc/kernel/vdso64/datapage.S | 4 ++--
arch/powerpc/kernel/vdso64/gettimeofday.S | 8 ++++----
7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
index 11886467dfdf..f3d7e4e2a45b 100644
--- a/arch/powerpc/include/asm/vdso_datapage.h
+++ b/arch/powerpc/include/asm/vdso_datapage.h
@@ -116,7 +116,7 @@ extern struct vdso_data *vdso_data;

#else /* __ASSEMBLY__ */

-.macro get_datapage ptr, tmp
+.macro get_datapage ptr
bcl 20, 31, .+4
999:
mflr \ptr
diff --git a/arch/powerpc/kernel/vdso32/cacheflush.S b/arch/powerpc/kernel/vdso32/cacheflush.S
index 3440ddf21c8b..017843bf5382 100644
--- a/arch/powerpc/kernel/vdso32/cacheflush.S
+++ b/arch/powerpc/kernel/vdso32/cacheflush.S
@@ -27,7 +27,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
#ifdef CONFIG_PPC64
mflr r12
.cfi_register lr,r12
- get_datapage r10, r0
+ get_datapage r10
mtlr r12
#endif

diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
index 5513a4f8253e..0513a2eabec8 100644
--- a/arch/powerpc/kernel/vdso32/datapage.S
+++ b/arch/powerpc/kernel/vdso32/datapage.S
@@ -28,7 +28,7 @@ V_FUNCTION_BEGIN(__kernel_get_syscall_map)
mflr r12
.cfi_register lr,r12
mr. r4,r3
- get_datapage r3, r0
+ get_datapage r3
mtlr r12
addi r3,r3,CFG_SYSCALL_MAP32
beqlr
@@ -49,7 +49,7 @@ V_FUNCTION_BEGIN(__kernel_get_tbfreq)
.cfi_startproc
mflr r12
.cfi_register lr,r12
- get_datapage r3, r0
+ get_datapage r3
lwz r4,(CFG_TB_TICKS_PER_SEC + 4)(r3)
lwz r3,CFG_TB_TICKS_PER_SEC(r3)
mtlr r12
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index a3951567118a..0bbdce0f2a9c 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -34,7 +34,7 @@ V_FUNCTION_BEGIN(__kernel_gettimeofday)

mr. r10,r3 /* r10 saves tv */
mr r11,r4 /* r11 saves tz */
- get_datapage r9, r0
+ get_datapage r9
beq 3f
LOAD_REG_IMMEDIATE(r7, 1000000) /* load up USEC_PER_SEC */
bl __do_get_tspec@local /* get sec/usec from tb & kernel */
@@ -79,7 +79,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
mflr r12 /* r12 saves lr */
.cfi_register lr,r12
mr r11,r4 /* r11 saves tp */
- get_datapage r9, r0
+ get_datapage r9
LOAD_REG_IMMEDIATE(r7, NSEC_PER_SEC) /* load up NSEC_PER_SEC */
beq cr5, .Lcoarse_clocks
.Lprecise_clocks:
@@ -206,7 +206,7 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)

mflr r12
.cfi_register lr,r12
- get_datapage r3, r0
+ get_datapage r3
lwz r5, CLOCK_HRTIMER_RES(r3)
mtlr r12
1: li r3,0
@@ -240,7 +240,7 @@ V_FUNCTION_BEGIN(__kernel_time)
.cfi_register lr,r12

mr r11,r3 /* r11 holds t */
- get_datapage r9, r0
+ get_datapage r9

lwz r3,STAMP_XTIME_SEC+LOPART(r9)

diff --git a/arch/powerpc/kernel/vdso64/cacheflush.S b/arch/powerpc/kernel/vdso64/cacheflush.S
index cab14324242b..61985de5758f 100644
--- a/arch/powerpc/kernel/vdso64/cacheflush.S
+++ b/arch/powerpc/kernel/vdso64/cacheflush.S
@@ -25,7 +25,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
.cfi_startproc
mflr r12
.cfi_register lr,r12
- get_datapage r10, r0
+ get_datapage r10
mtlr r12

lwz r7,CFG_DCACHE_BLOCKSZ(r10)
diff --git a/arch/powerpc/kernel/vdso64/datapage.S b/arch/powerpc/kernel/vdso64/datapage.S
index 03bb72c440dc..00760dc69d68 100644
--- a/arch/powerpc/kernel/vdso64/datapage.S
+++ b/arch/powerpc/kernel/vdso64/datapage.S
@@ -28,7 +28,7 @@ V_FUNCTION_BEGIN(__kernel_get_syscall_map)
mflr r12
.cfi_register lr,r12
mr r4,r3
- get_datapage r3, r0
+ get_datapage r3
mtlr r12
addi r3,r3,CFG_SYSCALL_MAP64
cmpldi cr0,r4,0
@@ -50,7 +50,7 @@ V_FUNCTION_BEGIN(__kernel_get_tbfreq)
.cfi_startproc
mflr r12
.cfi_register lr,r12
- get_datapage r3, r0
+ get_datapage r3
ld r3,CFG_TB_TICKS_PER_SEC(r3)
mtlr r12
crclr cr0*4+so
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
index e54c4ce4d356..275f031d0bf1 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
@@ -26,7 +26,7 @@ V_FUNCTION_BEGIN(__kernel_gettimeofday)

mr r11,r3 /* r11 holds tv */
mr r10,r4 /* r10 holds tz */
- get_datapage r3, r0
+ get_datapage r3
cmpldi r11,0 /* check if tv is NULL */
beq 2f
lis r7,1000000@ha /* load up USEC_PER_SEC */
@@ -71,7 +71,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
mflr r12 /* r12 saves lr */
.cfi_register lr,r12
mr r11,r4 /* r11 saves tp */
- get_datapage r3, r0
+ get_datapage r3
lis r7,NSEC_PER_SEC@h /* want nanoseconds */
ori r7,r7,NSEC_PER_SEC@l
beq cr5,70f
@@ -188,7 +188,7 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)

mflr r12
.cfi_register lr,r12
- get_datapage r3, r0
+ get_datapage r3
lwz r5, CLOCK_HRTIMER_RES(r3)
mtlr r12
li r3,0
@@ -221,7 +221,7 @@ V_FUNCTION_BEGIN(__kernel_time)
.cfi_register lr,r12

mr r11,r3 /* r11 holds t */
- get_datapage r3, r0
+ get_datapage r3

ld r4,STAMP_XTIME_SEC(r3)

--
2.25.0

2020-04-28 13:21:41

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v8 4/8] powerpc/processor: Move cpu_relax() into asm/vdso/processor.h

cpu_relax() need to be in asm/vdso/processor.h to be used by
the C VDSO generic library.

Move it there.

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/processor.h | 10 ++--------
arch/powerpc/include/asm/vdso/processor.h | 23 +++++++++++++++++++++++
2 files changed, 25 insertions(+), 8 deletions(-)
create mode 100644 arch/powerpc/include/asm/vdso/processor.h

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index bfa336fbcfeb..8390503c44a2 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -6,6 +6,8 @@
* Copyright (C) 2001 PPC 64 Team, IBM Corp
*/

+#include <vdso/processor.h>
+
#include <asm/reg.h>

#ifdef CONFIG_VSX
@@ -63,14 +65,6 @@ extern int _chrp_type;

#endif /* defined(__KERNEL__) && defined(CONFIG_PPC32) */

-/* Macros for adjusting thread priority (hardware multi-threading) */
-#define HMT_very_low() asm volatile("or 31,31,31 # very low priority")
-#define HMT_low() asm volatile("or 1,1,1 # low priority")
-#define HMT_medium_low() asm volatile("or 6,6,6 # medium low priority")
-#define HMT_medium() asm volatile("or 2,2,2 # medium priority")
-#define HMT_medium_high() asm volatile("or 5,5,5 # medium high priority")
-#define HMT_high() asm volatile("or 3,3,3 # high priority")
-
#ifdef __KERNEL__

#ifdef CONFIG_PPC64
diff --git a/arch/powerpc/include/asm/vdso/processor.h b/arch/powerpc/include/asm/vdso/processor.h
new file mode 100644
index 000000000000..39b9beace9ca
--- /dev/null
+++ b/arch/powerpc/include/asm/vdso/processor.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_VDSO_PROCESSOR_H
+#define __ASM_VDSO_PROCESSOR_H
+
+#ifndef __ASSEMBLY__
+
+/* Macros for adjusting thread priority (hardware multi-threading) */
+#define HMT_very_low() asm volatile("or 31, 31, 31 # very low priority")
+#define HMT_low() asm volatile("or 1, 1, 1 # low priority")
+#define HMT_medium_low() asm volatile("or 6, 6, 6 # medium low priority")
+#define HMT_medium() asm volatile("or 2, 2, 2 # medium priority")
+#define HMT_medium_high() asm volatile("or 5, 5, 5 # medium high priority")
+#define HMT_high() asm volatile("or 3, 3, 3 # high priority")
+
+#ifdef CONFIG_PPC64
+#define cpu_relax() do { HMT_low(); HMT_medium(); barrier(); } while (0)
+#else
+#define cpu_relax() barrier()
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_PROCESSOR_H */
--
2.25.0

2020-04-28 16:07:38

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v8 8/8] powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32

On Tue, Apr 28, 2020 at 3:16 PM Christophe Leroy
<[email protected]> wrote:
>
> Provides __kernel_clock_gettime64() on vdso32. This is the
> 64 bits version of __kernel_clock_gettime() which is
> y2038 compliant.
>
> Signed-off-by: Christophe Leroy <[email protected]>

Looks good to me

Reviewed-by: Arnd Bergmann <[email protected]>

There was a bug on ARM for the corresponding function, so far it is unclear
if this was a problem related to particular hardware, the 32-bit kernel code,
or the common implementation of clock_gettime64 in the vdso library,
see https://github.com/richfelker/musl-cross-make/issues/96

Just to be sure that powerpc is not affected by the same issue, can you
confirm that repeatedly calling clock_gettime64 on powerpc32, alternating
between vdso and syscall, results in monotically increasing times?

Arnd

2020-05-09 15:56:43

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 8/8] powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32



Le 28/04/2020 à 18:05, Arnd Bergmann a écrit :
> On Tue, Apr 28, 2020 at 3:16 PM Christophe Leroy
> <[email protected]> wrote:
>>
>> Provides __kernel_clock_gettime64() on vdso32. This is the
>> 64 bits version of __kernel_clock_gettime() which is
>> y2038 compliant.
>>
>> Signed-off-by: Christophe Leroy <[email protected]>
>
> Looks good to me
>
> Reviewed-by: Arnd Bergmann <[email protected]>
>
> There was a bug on ARM for the corresponding function, so far it is unclear
> if this was a problem related to particular hardware, the 32-bit kernel code,
> or the common implementation of clock_gettime64 in the vdso library,
> see https://github.com/richfelker/musl-cross-make/issues/96
>
> Just to be sure that powerpc is not affected by the same issue, can you
> confirm that repeatedly calling clock_gettime64 on powerpc32, alternating
> between vdso and syscall, results in monotically increasing times?
>

I think that's one of the things vdsotest checks, so yes that's ok I think.

Christophe

2020-05-09 18:51:36

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 8/8] powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32



Le 09/05/2020 à 17:54, Christophe Leroy a écrit :
>
>
> Le 28/04/2020 à 18:05, Arnd Bergmann a écrit :
>> On Tue, Apr 28, 2020 at 3:16 PM Christophe Leroy
>> <[email protected]> wrote:
>>>
>>> Provides __kernel_clock_gettime64() on vdso32. This is the
>>> 64 bits version of __kernel_clock_gettime() which is
>>> y2038 compliant.
>>>
>>> Signed-off-by: Christophe Leroy <[email protected]>
>>
>> Looks good to me
>>
>> Reviewed-by: Arnd Bergmann <[email protected]>
>>
>> There was a bug on ARM for the corresponding function, so far it is
>> unclear
>> if this was a problem related to particular hardware, the 32-bit
>> kernel code,
>> or the common implementation of clock_gettime64 in the vdso library,
>> see https://github.com/richfelker/musl-cross-make/issues/96
>>
>> Just to be sure that powerpc is not affected by the same issue, can you
>> confirm that repeatedly calling clock_gettime64 on powerpc32, alternating
>> between vdso and syscall, results in monotically increasing times?
>>
>
> I think that's one of the things vdsotest checks, so yes that's ok I think.
>

Here is the full result with vdsotest:

gettimeofday: syscall: 3715 nsec/call
gettimeofday: libc: 794 nsec/call
gettimeofday: vdso: 947 nsec/call
getcpu: syscall: 1614 nsec/call
getcpu: libc: 484 nsec/call
getcpu: vdso: 184 nsec/call
clock-gettime64-realtime-coarse: syscall: 3152 nsec/call
clock-gettime64-realtime-coarse: libc: not tested
clock-gettime64-realtime-coarse: vdso: 653 nsec/call
clock-getres-realtime-coarse: syscall: 2963 nsec/call
clock-getres-realtime-coarse: libc: 745 nsec/call
clock-getres-realtime-coarse: vdso: 553 nsec/call
clock-gettime-realtime-coarse: syscall: 5120 nsec/call
clock-gettime-realtime-coarse: libc: 731 nsec/call
clock-gettime-realtime-coarse: vdso: 577 nsec/call
clock-gettime64-realtime: syscall: 3719 nsec/call
clock-gettime64-realtime: libc: not tested
clock-gettime64-realtime: vdso: 976 nsec/call
clock-getres-realtime: syscall: 2496 nsec/call
clock-getres-realtime: libc: 745 nsec/call
clock-getres-realtime: vdso: 546 nsec/call
clock-gettime-realtime: syscall: 4800 nsec/call
clock-gettime-realtime: libc: 1080 nsec/call
clock-gettime-realtime: vdso: 1798 nsec/call
clock-gettime64-boottime: syscall: 4132 nsec/call
clock-gettime64-boottime: libc: not tested
clock-gettime64-boottime: vdso: 975 nsec/call
clock-getres-boottime: syscall: 2497 nsec/call
clock-getres-boottime: libc: 730 nsec/call
clock-getres-boottime: vdso: 546 nsec/call
clock-gettime-boottime: syscall: 3728 nsec/call
clock-gettime-boottime: libc: 1079 nsec/call
clock-gettime-boottime: vdso: 941 nsec/call
clock-gettime64-tai: syscall: 4148 nsec/call
clock-gettime64-tai: libc: not tested
clock-gettime64-tai: vdso: 955 nsec/call
clock-getres-tai: syscall: 2494 nsec/call
clock-getres-tai: libc: 730 nsec/call
clock-getres-tai: vdso: 545 nsec/call
clock-gettime-tai: syscall: 3729 nsec/call
clock-gettime-tai: libc: 1079 nsec/call
clock-gettime-tai: vdso: 927 nsec/call
clock-gettime64-monotonic-raw: syscall: 3677 nsec/call
clock-gettime64-monotonic-raw: libc: not tested
clock-gettime64-monotonic-raw: vdso: 1032 nsec/call
clock-getres-monotonic-raw: syscall: 2494 nsec/call
clock-getres-monotonic-raw: libc: 745 nsec/call
clock-getres-monotonic-raw: vdso: 545 nsec/call
clock-gettime-monotonic-raw: syscall: 3276 nsec/call
clock-gettime-monotonic-raw: libc: 1140 nsec/call
clock-gettime-monotonic-raw: vdso: 1002 nsec/call
clock-gettime64-monotonic-coarse: syscall: 4099 nsec/call
clock-gettime64-monotonic-coarse: libc: not tested
clock-gettime64-monotonic-coarse: vdso: 653 nsec/call
clock-getres-monotonic-coarse: syscall: 2962 nsec/call
clock-getres-monotonic-coarse: libc: 745 nsec/call
clock-getres-monotonic-coarse: vdso: 545 nsec/call
clock-gettime-monotonic-coarse: syscall: 4297 nsec/call
clock-gettime-monotonic-coarse: libc: 730 nsec/call
clock-gettime-monotonic-coarse: vdso: 592 nsec/call
clock-gettime64-monotonic: syscall: 3863 nsec/call
clock-gettime64-monotonic: libc: not tested
clock-gettime64-monotonic: vdso: 975 nsec/call
clock-getres-monotonic: syscall: 2494 nsec/call
clock-getres-monotonic: libc: 745 nsec/call
clock-getres-monotonic: vdso: 545 nsec/call
clock-gettime-monotonic: syscall: 3465 nsec/call
clock-gettime-monotonic: libc: 1079 nsec/call
clock-gettime-monotonic: vdso: 927 nsec/call

Christophe

2020-05-29 19:00:18

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 0/8] powerpc: switch VDSO to C implementation

Hi Michael,

Le 28/04/2020 à 15:16, Christophe Leroy a écrit :
> This is the seventh version of a series to switch powerpc VDSO to
> generic C implementation.
>
> Main changes since v7 are:
> - Added gettime64 on PPC32
>
> This series applies on today's powerpc/merge branch.
>
> See the last patches for details on changes and performance.

Do you have any plans for this series ?

Even if you don't feel like merging it this cycle, I think patches 1 to
3 are worth it.

Christophe

>
> Christophe Leroy (8):
> powerpc/vdso64: Switch from __get_datapage() to get_datapage inline
> macro
> powerpc/vdso: Remove __kernel_datapage_offset and simplify
> __get_datapage()
> powerpc/vdso: Remove unused \tmp param in __get_datapage()
> powerpc/processor: Move cpu_relax() into asm/vdso/processor.h
> powerpc/vdso: Prepare for switching VDSO to generic C implementation.
> powerpc/vdso: Switch VDSO to generic C implementation.
> lib/vdso: force inlining of __cvdso_clock_gettime_common()
> powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32
>
> arch/powerpc/Kconfig | 2 +
> arch/powerpc/include/asm/clocksource.h | 7 +
> arch/powerpc/include/asm/processor.h | 10 +-
> arch/powerpc/include/asm/vdso/clocksource.h | 7 +
> arch/powerpc/include/asm/vdso/gettimeofday.h | 175 +++++++++++
> arch/powerpc/include/asm/vdso/processor.h | 23 ++
> arch/powerpc/include/asm/vdso/vsyscall.h | 25 ++
> arch/powerpc/include/asm/vdso_datapage.h | 50 ++--
> arch/powerpc/kernel/asm-offsets.c | 49 +--
> arch/powerpc/kernel/time.c | 91 +-----
> arch/powerpc/kernel/vdso.c | 58 +---
> arch/powerpc/kernel/vdso32/Makefile | 32 +-
> arch/powerpc/kernel/vdso32/cacheflush.S | 2 +-
> arch/powerpc/kernel/vdso32/config-fake32.h | 34 +++
> arch/powerpc/kernel/vdso32/datapage.S | 7 +-
> arch/powerpc/kernel/vdso32/gettimeofday.S | 300 +------------------
> arch/powerpc/kernel/vdso32/vdso32.lds.S | 8 +-
> arch/powerpc/kernel/vdso32/vgettimeofday.c | 35 +++
> arch/powerpc/kernel/vdso64/Makefile | 23 +-
> arch/powerpc/kernel/vdso64/cacheflush.S | 9 +-
> arch/powerpc/kernel/vdso64/datapage.S | 31 +-
> arch/powerpc/kernel/vdso64/gettimeofday.S | 243 +--------------
> arch/powerpc/kernel/vdso64/vdso64.lds.S | 7 +-
> arch/powerpc/kernel/vdso64/vgettimeofday.c | 29 ++
> lib/vdso/gettimeofday.c | 2 +-
> 25 files changed, 460 insertions(+), 799 deletions(-)
> create mode 100644 arch/powerpc/include/asm/clocksource.h
> create mode 100644 arch/powerpc/include/asm/vdso/clocksource.h
> create mode 100644 arch/powerpc/include/asm/vdso/gettimeofday.h
> create mode 100644 arch/powerpc/include/asm/vdso/processor.h
> create mode 100644 arch/powerpc/include/asm/vdso/vsyscall.h
> create mode 100644 arch/powerpc/kernel/vdso32/config-fake32.h
> create mode 100644 arch/powerpc/kernel/vdso32/vgettimeofday.c
> create mode 100644 arch/powerpc/kernel/vdso64/vgettimeofday.c
>

2020-06-03 10:06:48

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 0/8] powerpc: switch VDSO to C implementation

Christophe Leroy <[email protected]> writes:
> Hi Michael,
>
> Le 28/04/2020 à 15:16, Christophe Leroy a écrit :
>> This is the seventh version of a series to switch powerpc VDSO to
>> generic C implementation.
>>
>> Main changes since v7 are:
>> - Added gettime64 on PPC32
>>
>> This series applies on today's powerpc/merge branch.
>>
>> See the last patches for details on changes and performance.
>
> Do you have any plans for this series ?

Review it and merge it one day :/

> Even if you don't feel like merging it this cycle, I think patches 1 to
> 3 are worth it.

I'd rather take the whole series for v5.9.

Sorry it missed this window, I just didn't get time to look at it.

cheers

2020-07-15 01:25:15

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

Christophe Leroy <[email protected]> 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.

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.

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

cheers

2020-07-15 18:47:02

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

Michael Ellerman <[email protected]> a écrit :

> Christophe Leroy <[email protected]> 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


2020-07-16 03:00:11

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

Christophe Leroy <[email protected]> writes:
> The VDSO datapage and the text pages are always located immediately
> next to each other, so it can be hardcoded without an indirection
> through __kernel_datapage_offset
>
> In order to ease things, move the data page in front like other
> arches, that way there is no need to know the size of the library
> to locate the data page.
>
> Before:
> clock-getres-realtime-coarse: vdso: 714 nsec/call
> clock-gettime-realtime-coarse: vdso: 792 nsec/call
> clock-gettime-realtime: vdso: 1243 nsec/call
>
> After:
> clock-getres-realtime-coarse: vdso: 699 nsec/call
> clock-gettime-realtime-coarse: vdso: 784 nsec/call
> clock-gettime-realtime: vdso: 1231 nsec/call
>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> v7:
> - Moved the removal of the tmp param of __get_datapage()
> in a subsequent patch
> - Included the addition of the offset param to __get_datapage()
> in the further preparatory patch
> ---
> arch/powerpc/include/asm/vdso_datapage.h | 8 ++--
> arch/powerpc/kernel/vdso.c | 53 ++++--------------------
> arch/powerpc/kernel/vdso32/datapage.S | 3 --
> arch/powerpc/kernel/vdso32/vdso32.lds.S | 7 +---
> arch/powerpc/kernel/vdso64/datapage.S | 3 --
> arch/powerpc/kernel/vdso64/vdso64.lds.S | 7 +---
> 6 files changed, 16 insertions(+), 65 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
> index b9ef6cf50ea5..11886467dfdf 100644
> --- a/arch/powerpc/include/asm/vdso_datapage.h
> +++ b/arch/powerpc/include/asm/vdso_datapage.h
> @@ -118,10 +118,12 @@ extern struct vdso_data *vdso_data;
>
> .macro get_datapage ptr, tmp
> bcl 20, 31, .+4
> +999:
> mflr \ptr
> - addi \ptr, \ptr, (__kernel_datapage_offset - (.-4))@l
> - lwz \tmp, 0(\ptr)
> - add \ptr, \tmp, \ptr
> +#if CONFIG_PPC_PAGE_SHIFT > 14
> + addis \ptr, \ptr, (_vdso_datapage - 999b)@ha
> +#endif
> + addi \ptr, \ptr, (_vdso_datapage - 999b)@l
> .endm
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> index f38f26e844b6..d33fa22ddbed 100644
> --- a/arch/powerpc/kernel/vdso.c
> +++ b/arch/powerpc/kernel/vdso.c
> @@ -190,7 +190,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
> * install_special_mapping or the perf counter mmap tracking code
> * will fail to recognise it as a vDSO (since arch_vma_name fails).
> */
> - current->mm->context.vdso_base = vdso_base;
> + current->mm->context.vdso_base = vdso_base + PAGE_SIZE;

I merged this but then realised it breaks the display of the vdso in /proc/self/maps.

ie. the vdso vma gets no name:

# cat /proc/self/maps
110f90000-110fa0000 r-xp 00000000 08:03 17021844 /usr/bin/cat
110fa0000-110fb0000 r--p 00000000 08:03 17021844 /usr/bin/cat
110fb0000-110fc0000 rw-p 00010000 08:03 17021844 /usr/bin/cat
126000000-126030000 rw-p 00000000 00:00 0 [heap]
7fffa8790000-7fffa87d0000 rw-p 00000000 00:00 0
7fffa87d0000-7fffa8830000 r--p 00000000 08:03 17521786 /usr/lib/locale/en_AU.utf8/LC_CTYPE
7fffa8830000-7fffa8840000 r--p 00000000 08:03 16958337 /usr/lib/locale/en_AU.utf8/LC_NUMERIC
7fffa8840000-7fffa8850000 r--p 00000000 08:03 8501358 /usr/lib/locale/en_AU.utf8/LC_TIME
7fffa8850000-7fffa8ad0000 r--p 00000000 08:03 16870886 /usr/lib/locale/en_AU.utf8/LC_COLLATE
7fffa8ad0000-7fffa8ae0000 r--p 00000000 08:03 8509433 /usr/lib/locale/en_AU.utf8/LC_MONETARY
7fffa8ae0000-7fffa8af0000 r--p 00000000 08:03 25383753 /usr/lib/locale/en_AU.utf8/LC_MESSAGES/SYS_LC_MESSAGES
7fffa8af0000-7fffa8b00000 r--p 00000000 08:03 17521790 /usr/lib/locale/en_AU.utf8/LC_PAPER
7fffa8b00000-7fffa8b10000 r--p 00000000 08:03 8501354 /usr/lib/locale/en_AU.utf8/LC_NAME
7fffa8b10000-7fffa8b20000 r--p 00000000 08:03 8509431 /usr/lib/locale/en_AU.utf8/LC_ADDRESS
7fffa8b20000-7fffa8b30000 r--p 00000000 08:03 8509434 /usr/lib/locale/en_AU.utf8/LC_TELEPHONE
7fffa8b30000-7fffa8b40000 r--p 00000000 08:03 17521787 /usr/lib/locale/en_AU.utf8/LC_MEASUREMENT
7fffa8b40000-7fffa8b50000 r--s 00000000 08:03 25623315 /usr/lib64/gconv/gconv-modules.cache
7fffa8b50000-7fffa8d40000 r-xp 00000000 08:03 25383789 /usr/lib64/libc-2.30.so
7fffa8d40000-7fffa8d50000 r--p 001e0000 08:03 25383789 /usr/lib64/libc-2.30.so
7fffa8d50000-7fffa8d60000 rw-p 001f0000 08:03 25383789 /usr/lib64/libc-2.30.so
7fffa8d60000-7fffa8d70000 r--p 00000000 08:03 8509432 /usr/lib/locale/en_AU.utf8/LC_IDENTIFICATION
7fffa8d70000-7fffa8d90000 r-xp 00000000 00:00 0 <--- missing
7fffa8d90000-7fffa8dc0000 r-xp 00000000 08:03 25383781 /usr/lib64/ld-2.30.so
7fffa8dc0000-7fffa8dd0000 r--p 00020000 08:03 25383781 /usr/lib64/ld-2.30.so
7fffa8dd0000-7fffa8de0000 rw-p 00030000 08:03 25383781 /usr/lib64/ld-2.30.so
7fffc31c0000-7fffc31f0000 rw-p 00000000 00:00 0 [stack]


And it's also going to break the logic in arch_unmap() to detect if
we're unmapping (part of) the VDSO. And it will break arch_remap() too.

And the logic to recognise the signal trampoline in
arch/powerpc/perf/callchain_*.c as well.

So I'm going to rebase and drop this for now.

Basically we have a bunch of places that assume that vdso_base is == the
start of the VDSO vma, and also that the code starts there. So that will
need some work to tease out all those assumptions and make them work
with this change.

cheers

2020-07-16 12:58:23

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 0/8] powerpc: switch VDSO to C implementation

On Tue, 28 Apr 2020 13:16:46 +0000 (UTC), Christophe Leroy wrote:
> This is the seventh version of a series to switch powerpc VDSO to
> generic C implementation.
>
> Main changes since v7 are:
> - Added gettime64 on PPC32
>
> This series applies on today's powerpc/merge branch.
>
> [...]

Patch 1 applied to powerpc/next.

[1/8] powerpc/vdso64: Switch from __get_datapage() to get_datapage inline macro
https://git.kernel.org/powerpc/c/793d74a8c78e05d6833bfcf582e24e40bd92518f

cheers

Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

Christophe Leroy <[email protected]> writes:

> Michael Ellerman <[email protected]> a écrit :
>
>> Christophe Leroy <[email protected]> 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 ?

As Michael pointed out, userspace doesn't treat the VDSO as a normal function
call. In order to keep compatibility with existent software, LR would need to
be saved on another stack frame.

--
Tulio Magno

2020-08-04 11:19:44

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.



On 07/15/2020 01:04 AM, Michael Ellerman wrote:
> Christophe Leroy <[email protected]> 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.
>
> 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.

Indeed, 32-bit doesn't have a redzone, so I believe it needs a stack
frame whenever it has anything to same. Here is what I have in libc.so:

000fbb60 <__clock_gettime>:
fbb60: 94 21 ff e0 stwu r1,-32(r1)
fbb64: 7c 08 02 a6 mflr r0
fbb68: 48 09 75 c9 bl 193130 <_IO_stdout_+0x24b0>
fbb6c: 93 c1 00 18 stw r30,24(r1)
fbb70: 7f c8 02 a6 mflr r30
fbb74: 90 01 00 24 stw r0,36(r1)
fbb78: 93 81 00 10 stw r28,16(r1)
fbb7c: 93 a1 00 14 stw r29,20(r1)
fbb80: 83 9e fc 98 lwz r28,-872(r30)
fbb84: 93 e1 00 1c stw r31,28(r1)
fbb88: 80 1c 00 00 lwz r0,0(r28)
fbb8c: 83 82 8f f4 lwz r28,-28684(r2)
fbb90: 7c 7f 1b 78 mr r31,r3
fbb94: 7c 00 e2 79 xor. r0,r0,r28
fbb98: 7c 9d 23 78 mr r29,r4
fbb9c: 41 82 00 40 beq fbbdc <__clock_gettime+0x7c>
fbba0: 7c 09 03 a6 mtctr r0
fbba4: 4e 80 04 21 bctrl
fbba8: 7c 00 00 26 mfcr r0
fbbac: 74 1c 10 00 andis. r28,r0,4096
fbbb0: 40 82 00 24 bne fbbd4 <__clock_gettime+0x74>
fbbb4: 80 01 00 24 lwz r0,36(r1)
fbbb8: 83 81 00 10 lwz r28,16(r1)
fbbbc: 7c 08 03 a6 mtlr r0
fbbc0: 83 a1 00 14 lwz r29,20(r1)
fbbc4: 83 c1 00 18 lwz r30,24(r1)
fbbc8: 83 e1 00 1c lwz r31,28(r1)
fbbcc: 38 21 00 20 addi r1,r1,32
fbbd0: 4e 80 00 20 blr
...
193130: 4e 80 00 21 blrl

But I guess if a prog has a way to avoid having anything to save, we may
face the same issue. So lets create two frames:

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
PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
get_datapage r5, r0
@@ -19,7 +20,7 @@
cmpwi r3, 0
mtlr r0
.cfi_restore lr
- addi r1, r1, STACK_FRAME_OVERHEAD
+ addi r1, r1, 2 * STACK_FRAME_OVERHEAD
crclr so
beqlr+
crset so
@@ -32,6 +33,7 @@
.cfi_startproc
PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
mflr r0
+ PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
.cfi_register lr, r0
PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
get_datapage r4, r0
@@ -41,7 +43,7 @@
crclr so
mtlr r0
.cfi_restore lr
- addi r1, r1, STACK_FRAME_OVERHEAD
+ addi r1, r1, 2 * STACK_FRAME_OVERHEAD
blr
.cfi_endproc
.endm



>
> 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:

commit 5a704d89bd5d7aac39194fb4c775f406905bf0a4
Author: Christophe Leroy <[email protected]>
Date: Tue Aug 4 06:31:48 2020 +0000

Save and restore GOT

diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h
b/arch/powerpc/include/asm/vdso/gettimeofday.h
index 0b6fa245d54e..fa774086173b 100644
--- a/arch/powerpc/include/asm/vdso/gettimeofday.h
+++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
@@ -13,10 +13,16 @@
PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
.cfi_register lr, r0
PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
+#ifdef CONFIG_PPC64
+ PPC_STL r2, STACK_FRAME_OVERHEAD + STK_GOT(r1)
+#endif
get_datapage r5, r0
addi r5, r5, VDSO_DATA_OFFSET
bl \funct
PPC_LL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
+#ifdef CONFIG_PPC64
+ PPC_LL r2, STACK_FRAME_OVERHEAD + STK_GOT(r1)
+#endif
cmpwi r3, 0
mtlr r0
.cfi_restore lr
@@ -36,10 +42,16 @@
PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
.cfi_register lr, r0
PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
+#ifdef CONFIG_PPC64
+ PPC_STL r2, STACK_FRAME_OVERHEAD + STK_GOT(r1)
+#endif
get_datapage r4, r0
addi r4, r4, VDSO_DATA_OFFSET
bl \funct
PPC_LL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
+#ifdef CONFIG_PPC64
+ PPC_LL r2, STACK_FRAME_OVERHEAD + STK_GOT(r1)
+#endif
crclr so
mtlr r0
.cfi_restore lr


Christophe

2020-08-04 11:21:19

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()



On 07/16/2020 02:59 AM, Michael Ellerman wrote:
> Christophe Leroy <[email protected]> writes:
>> The VDSO datapage and the text pages are always located immediately
>> next to each other, so it can be hardcoded without an indirection
>> through __kernel_datapage_offset
>>
>> In order to ease things, move the data page in front like other
>> arches, that way there is no need to know the size of the library
>> to locate the data page.
>>
>> Before:
>> clock-getres-realtime-coarse: vdso: 714 nsec/call
>> clock-gettime-realtime-coarse: vdso: 792 nsec/call
>> clock-gettime-realtime: vdso: 1243 nsec/call
>>
>> After:
>> clock-getres-realtime-coarse: vdso: 699 nsec/call
>> clock-gettime-realtime-coarse: vdso: 784 nsec/call
>> clock-gettime-realtime: vdso: 1231 nsec/call
>>
>> Signed-off-by: Christophe Leroy <[email protected]>
>> ---
>> v7:
>> - Moved the removal of the tmp param of __get_datapage()
>> in a subsequent patch
>> - Included the addition of the offset param to __get_datapage()
>> in the further preparatory patch
>> ---
>> arch/powerpc/include/asm/vdso_datapage.h | 8 ++--
>> arch/powerpc/kernel/vdso.c | 53 ++++--------------------
>> arch/powerpc/kernel/vdso32/datapage.S | 3 --
>> arch/powerpc/kernel/vdso32/vdso32.lds.S | 7 +---
>> arch/powerpc/kernel/vdso64/datapage.S | 3 --
>> arch/powerpc/kernel/vdso64/vdso64.lds.S | 7 +---
>> 6 files changed, 16 insertions(+), 65 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
>> index b9ef6cf50ea5..11886467dfdf 100644
>> --- a/arch/powerpc/include/asm/vdso_datapage.h
>> +++ b/arch/powerpc/include/asm/vdso_datapage.h
>> @@ -118,10 +118,12 @@ extern struct vdso_data *vdso_data;
>>
>> .macro get_datapage ptr, tmp
>> bcl 20, 31, .+4
>> +999:
>> mflr \ptr
>> - addi \ptr, \ptr, (__kernel_datapage_offset - (.-4))@l
>> - lwz \tmp, 0(\ptr)
>> - add \ptr, \tmp, \ptr
>> +#if CONFIG_PPC_PAGE_SHIFT > 14
>> + addis \ptr, \ptr, (_vdso_datapage - 999b)@ha
>> +#endif
>> + addi \ptr, \ptr, (_vdso_datapage - 999b)@l
>> .endm
>>
>> #endif /* __ASSEMBLY__ */
>> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
>> index f38f26e844b6..d33fa22ddbed 100644
>> --- a/arch/powerpc/kernel/vdso.c
>> +++ b/arch/powerpc/kernel/vdso.c
>> @@ -190,7 +190,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
>> * install_special_mapping or the perf counter mmap tracking code
>> * will fail to recognise it as a vDSO (since arch_vma_name fails).
>> */
>> - current->mm->context.vdso_base = vdso_base;
>> + current->mm->context.vdso_base = vdso_base + PAGE_SIZE;
>
> I merged this but then realised it breaks the display of the vdso in /proc/self/maps.
>
> ie. the vdso vma gets no name:
>
> # cat /proc/self/maps
> 110f90000-110fa0000 r-xp 00000000 08:03 17021844 /usr/bin/cat
> 110fa0000-110fb0000 r--p 00000000 08:03 17021844 /usr/bin/cat
> 110fb0000-110fc0000 rw-p 00010000 08:03 17021844 /usr/bin/cat
> 126000000-126030000 rw-p 00000000 00:00 0 [heap]
> 7fffa8790000-7fffa87d0000 rw-p 00000000 00:00 0
> 7fffa87d0000-7fffa8830000 r--p 00000000 08:03 17521786 /usr/lib/locale/en_AU.utf8/LC_CTYPE
> 7fffa8830000-7fffa8840000 r--p 00000000 08:03 16958337 /usr/lib/locale/en_AU.utf8/LC_NUMERIC
> 7fffa8840000-7fffa8850000 r--p 00000000 08:03 8501358 /usr/lib/locale/en_AU.utf8/LC_TIME
> 7fffa8850000-7fffa8ad0000 r--p 00000000 08:03 16870886 /usr/lib/locale/en_AU.utf8/LC_COLLATE
> 7fffa8ad0000-7fffa8ae0000 r--p 00000000 08:03 8509433 /usr/lib/locale/en_AU.utf8/LC_MONETARY
> 7fffa8ae0000-7fffa8af0000 r--p 00000000 08:03 25383753 /usr/lib/locale/en_AU.utf8/LC_MESSAGES/SYS_LC_MESSAGES
> 7fffa8af0000-7fffa8b00000 r--p 00000000 08:03 17521790 /usr/lib/locale/en_AU.utf8/LC_PAPER
> 7fffa8b00000-7fffa8b10000 r--p 00000000 08:03 8501354 /usr/lib/locale/en_AU.utf8/LC_NAME
> 7fffa8b10000-7fffa8b20000 r--p 00000000 08:03 8509431 /usr/lib/locale/en_AU.utf8/LC_ADDRESS
> 7fffa8b20000-7fffa8b30000 r--p 00000000 08:03 8509434 /usr/lib/locale/en_AU.utf8/LC_TELEPHONE
> 7fffa8b30000-7fffa8b40000 r--p 00000000 08:03 17521787 /usr/lib/locale/en_AU.utf8/LC_MEASUREMENT
> 7fffa8b40000-7fffa8b50000 r--s 00000000 08:03 25623315 /usr/lib64/gconv/gconv-modules.cache
> 7fffa8b50000-7fffa8d40000 r-xp 00000000 08:03 25383789 /usr/lib64/libc-2.30.so
> 7fffa8d40000-7fffa8d50000 r--p 001e0000 08:03 25383789 /usr/lib64/libc-2.30.so
> 7fffa8d50000-7fffa8d60000 rw-p 001f0000 08:03 25383789 /usr/lib64/libc-2.30.so
> 7fffa8d60000-7fffa8d70000 r--p 00000000 08:03 8509432 /usr/lib/locale/en_AU.utf8/LC_IDENTIFICATION
> 7fffa8d70000-7fffa8d90000 r-xp 00000000 00:00 0 <--- missing
> 7fffa8d90000-7fffa8dc0000 r-xp 00000000 08:03 25383781 /usr/lib64/ld-2.30.so
> 7fffa8dc0000-7fffa8dd0000 r--p 00020000 08:03 25383781 /usr/lib64/ld-2.30.so
> 7fffa8dd0000-7fffa8de0000 rw-p 00030000 08:03 25383781 /usr/lib64/ld-2.30.so
> 7fffc31c0000-7fffc31f0000 rw-p 00000000 00:00 0 [stack]
>
>
> And it's also going to break the logic in arch_unmap() to detect if
> we're unmapping (part of) the VDSO. And it will break arch_remap() too.
>
> And the logic to recognise the signal trampoline in
> arch/powerpc/perf/callchain_*.c as well.

I don't think it breaks that one, because ->vdsobase is still the start
of text.

>
> So I'm going to rebase and drop this for now.
>
> Basically we have a bunch of places that assume that vdso_base is == the
> start of the VDSO vma, and also that the code starts there. So that will
> need some work to tease out all those assumptions and make them work
> with this change.

Ok, one day I need to look at it in more details and see how other
architectures handle it etc ...

As the benefit is only 2 CPU cycles, I'll drop it for now.

Christophe

2020-08-05 06:25:44

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

Christophe Leroy <[email protected]> writes:
> On 07/15/2020 01:04 AM, Michael Ellerman wrote:
>> Christophe Leroy <[email protected]> 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.
>>
>> 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.
>
> 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)
> fbb64: 7c 08 02 a6 mflr r0
> fbb68: 48 09 75 c9 bl 193130 <_IO_stdout_+0x24b0>
> fbb6c: 93 c1 00 18 stw r30,24(r1)
> fbb70: 7f c8 02 a6 mflr r30
> fbb74: 90 01 00 24 stw r0,36(r1)
> fbb78: 93 81 00 10 stw r28,16(r1)
> fbb7c: 93 a1 00 14 stw r29,20(r1)
> fbb80: 83 9e fc 98 lwz r28,-872(r30)
> fbb84: 93 e1 00 1c stw r31,28(r1)
> fbb88: 80 1c 00 00 lwz r0,0(r28)
> fbb8c: 83 82 8f f4 lwz r28,-28684(r2)
> fbb90: 7c 7f 1b 78 mr r31,r3
> fbb94: 7c 00 e2 79 xor. r0,r0,r28
> fbb98: 7c 9d 23 78 mr r29,r4
> fbb9c: 41 82 00 40 beq fbbdc <__clock_gettime+0x7c>
> fbba0: 7c 09 03 a6 mtctr r0
> fbba4: 4e 80 04 21 bctrl
> fbba8: 7c 00 00 26 mfcr r0
> fbbac: 74 1c 10 00 andis. r28,r0,4096
> fbbb0: 40 82 00 24 bne fbbd4 <__clock_gettime+0x74>
> fbbb4: 80 01 00 24 lwz r0,36(r1)
> fbbb8: 83 81 00 10 lwz r28,16(r1)
> fbbbc: 7c 08 03 a6 mtlr r0
> fbbc0: 83 a1 00 14 lwz r29,20(r1)
> fbbc4: 83 c1 00 18 lwz r30,24(r1)
> fbbc8: 83 e1 00 1c lwz r31,28(r1)
> fbbcc: 38 21 00 20 addi r1,r1,32
> fbbd0: 4e 80 00 20 blr
> ...
> 193130: 4e 80 00 21 blrl
>
> But I guess if a prog has a way to avoid having anything to save, we may
> face the same issue.

Yes I think so.

> So lets create two frames:

Yeah I think that's what's required. Might need a comment explaining why
though ;)

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

> PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
> get_datapage r5, r0
> @@ -19,7 +20,7 @@
> cmpwi r3, 0
> mtlr r0
> .cfi_restore lr
> - addi r1, r1, STACK_FRAME_OVERHEAD
> + addi r1, r1, 2 * STACK_FRAME_OVERHEAD
> crclr so
> beqlr+
> crset so
> @@ -32,6 +33,7 @@
> .cfi_startproc
> PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
> mflr r0
> + PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
> .cfi_register lr, r0
> PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
> get_datapage r4, r0
> @@ -41,7 +43,7 @@
> crclr so
> mtlr r0
> .cfi_restore lr
> - addi r1, r1, STACK_FRAME_OVERHEAD
> + addi r1, r1, 2 * STACK_FRAME_OVERHEAD
> blr
> .cfi_endproc
> .endm
>
>
>> 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?)

AFAIK there's no way to disable use of the toc, or make it a build error
if it's needed.

But at least currently you can't add any global to the vdso code as it
won't link, because the .data and .bss are discarded.

At the same time it's much safer for us to just save/restore r2, and
probably in the noise performance wise.

So yeah we should probably do as below.

> commit 5a704d89bd5d7aac39194fb4c775f406905bf0a4
> Author: Christophe Leroy <[email protected]>
> Date: Tue Aug 4 06:31:48 2020 +0000
>
> Save and restore GOT
>
> diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h
> b/arch/powerpc/include/asm/vdso/gettimeofday.h
> index 0b6fa245d54e..fa774086173b 100644
> --- a/arch/powerpc/include/asm/vdso/gettimeofday.h
> +++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
> @@ -13,10 +13,16 @@
> PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
> .cfi_register lr, r0
> PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
> +#ifdef CONFIG_PPC64
> + PPC_STL r2, STACK_FRAME_OVERHEAD + STK_GOT(r1)
> +#endif
> get_datapage r5, r0
> addi r5, r5, VDSO_DATA_OFFSET
> bl \funct
> PPC_LL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
> +#ifdef CONFIG_PPC64
> + PPC_LL r2, STACK_FRAME_OVERHEAD + STK_GOT(r1)
> +#endif
> cmpwi r3, 0
> mtlr r0
> .cfi_restore lr
> @@ -36,10 +42,16 @@
> PPC_STLU r1, -STACK_FRAME_OVERHEAD(r1)
> .cfi_register lr, r0
> PPC_STL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
> +#ifdef CONFIG_PPC64
> + PPC_STL r2, STACK_FRAME_OVERHEAD + STK_GOT(r1)
> +#endif
> get_datapage r4, r0
> addi r4, r4, VDSO_DATA_OFFSET
> bl \funct
> PPC_LL r0, STACK_FRAME_OVERHEAD + PPC_LR_STKOFF(r1)
> +#ifdef CONFIG_PPC64
> + PPC_LL r2, STACK_FRAME_OVERHEAD + STK_GOT(r1)
> +#endif
> crclr so
> mtlr r0
> .cfi_restore lr


cheers

2020-08-05 16:24:08

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

Hi!

On Wed, Aug 05, 2020 at 04:24:16PM +1000, Michael Ellerman wrote:
> Christophe Leroy <[email protected]> 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).

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

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

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

> So yeah we should probably do as below.

[ snip ]

Looks good yes.


Segher

2020-08-06 02:06:20

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

Segher Boessenkool <[email protected]> writes:
> On Wed, Aug 05, 2020 at 04:24:16PM +1000, Michael Ellerman wrote:
>> Christophe Leroy <[email protected]> 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

2020-08-06 18:42:00

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

Hi!

On Thu, Aug 06, 2020 at 12:03:33PM +1000, Michael Ellerman wrote:
> Segher Boessenkool <[email protected]> writes:
> > On Wed, Aug 05, 2020 at 04:24:16PM +1000, Michael Ellerman wrote:
> >> Christophe Leroy <[email protected]> writes:
> >> > Indeed, 32-bit doesn't have a redzone, so I believe it needs a stack
> >> > frame whenever it has anything to same.

^^^

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

The "SYSV" ELF binding, yeah, which is used for 32-bit on Linux (give or
take, ho hum).

The ABIs that have a red zone are much nicer here (but less simple) :-)

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

It could get away with many things because it was guaranteed to be a
leaf function. Some of those things even violate the ABIs, but you can
get away with it easily, much reduced scope. Now if this is generated
code, violating the rules will catch up with you sooner rather than
later ;-)


Segher

2020-08-07 02:48:04

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 5/8] powerpc/vdso: Prepare for switching VDSO to generic C implementation.

Segher Boessenkool <[email protected]> writes:
> On Thu, Aug 06, 2020 at 12:03:33PM +1000, Michael Ellerman wrote:
>> Segher Boessenkool <[email protected]> writes:
>> > On Wed, Aug 05, 2020 at 04:24:16PM +1000, Michael Ellerman wrote:
>> >> Christophe Leroy <[email protected]> writes:
>> >> > Indeed, 32-bit doesn't have a redzone, so I believe it needs a stack
>> >> > frame whenever it has anything to same.
>
> ^^^
>
>> >> > 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?)
>
> The "SYSV" ELF binding, yeah, which is used for 32-bit on Linux (give or
> take, ho hum).
>
> The ABIs that have a red zone are much nicer here (but less simple) :-)

Yep, just checking I wasn't misunderstanding your comment about negative
offsets.

>> >> 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 :)
>
> It could get away with many things because it was guaranteed to be a
> leaf function. Some of those things even violate the ABIs, but you can
> get away with it easily, much reduced scope. Now if this is generated
> code, violating the rules will catch up with you sooner rather than
> later ;-)

Agreed.

cheers

2020-08-25 14:19:22

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()



Le 04/08/2020 à 13:17, Christophe Leroy a écrit :
>
>
> On 07/16/2020 02:59 AM, Michael Ellerman wrote:
>> Christophe Leroy <[email protected]> writes:
>>> The VDSO datapage and the text pages are always located immediately
>>> next to each other, so it can be hardcoded without an indirection
>>> through __kernel_datapage_offset
>>>
>>> In order to ease things, move the data page in front like other
>>> arches, that way there is no need to know the size of the library
>>> to locate the data page.
>>>
[...]

>>
>> I merged this but then realised it breaks the display of the vdso in
>> /proc/self/maps.
>>
>> ie. the vdso vma gets no name:
>>
>>    # cat /proc/self/maps

[...]

>>
>>
>> And it's also going to break the logic in arch_unmap() to detect if
>> we're unmapping (part of) the VDSO. And it will break arch_remap() too.
>>
>> And the logic to recognise the signal trampoline in
>> arch/powerpc/perf/callchain_*.c as well.
>
> I don't think it breaks that one, because ->vdsobase is still the start
> of text.
>
>>
>> So I'm going to rebase and drop this for now.
>>
>> Basically we have a bunch of places that assume that vdso_base is == the
>> start of the VDSO vma, and also that the code starts there. So that will
>> need some work to tease out all those assumptions and make them work
>> with this change.
>
> Ok, one day I need to look at it in more details and see how other
> architectures handle it etc ...
>

I just sent out a series which switches powerpc to the new
_install_special_mapping() API, the one powerpc uses being deprecated
since commit a62c34bd2a8a ("x86, mm: Improve _install_special_mapping
and fix x86 vdso naming")

arch_remap() gets replaced by vdso_remap()

For arch_unmap(), I'm wondering how/what other architectures do, because
powerpc seems to be the only one to erase the vdso context pointer when
unmapping the vdso. So far I updated it to take into account the pages
switch.

Everything else is not impacted because our vdso_base is still the base
of the text and that's what those things (signal trampoline, callchain,
...) expect.

Maybe we should change it to 'void *vdso' in the same way as other
architectures, as it is not anymore the exact vdso_base but the start of
VDSO text.

Note that the series applies on top of the generic C VDSO implementation
series. However all but the last commit cleanly apply without that
series. As that last commit is just an afterwork cleanup, it can come in
a second step.

Christophe

2020-08-26 14:36:26

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

Christophe Leroy <[email protected]> writes:
> Le 04/08/2020 à 13:17, Christophe Leroy a écrit :
>> On 07/16/2020 02:59 AM, Michael Ellerman wrote:
>>> Christophe Leroy <[email protected]> writes:
>>>> The VDSO datapage and the text pages are always located immediately
>>>> next to each other, so it can be hardcoded without an indirection
>>>> through __kernel_datapage_offset
>>>>
>>>> In order to ease things, move the data page in front like other
>>>> arches, that way there is no need to know the size of the library
>>>> to locate the data page.
>>>>
> [...]
>
>>>
>>> I merged this but then realised it breaks the display of the vdso in
>>> /proc/self/maps.
>>>
>>> ie. the vdso vma gets no name:
>>>
>>>    # cat /proc/self/maps
>
> [...]
>
>>> And it's also going to break the logic in arch_unmap() to detect if
>>> we're unmapping (part of) the VDSO. And it will break arch_remap() too.
>>>
>>> And the logic to recognise the signal trampoline in
>>> arch/powerpc/perf/callchain_*.c as well.
>>
>> I don't think it breaks that one, because ->vdsobase is still the start
>> of text.
>>
>>>
>>> So I'm going to rebase and drop this for now.
>>>
>>> Basically we have a bunch of places that assume that vdso_base is == the
>>> start of the VDSO vma, and also that the code starts there. So that will
>>> need some work to tease out all those assumptions and make them work
>>> with this change.
>>
>> Ok, one day I need to look at it in more details and see how other
>> architectures handle it etc ...
>>
>
> I just sent out a series which switches powerpc to the new
> _install_special_mapping() API, the one powerpc uses being deprecated
> since commit a62c34bd2a8a ("x86, mm: Improve _install_special_mapping
> and fix x86 vdso naming")
>
> arch_remap() gets replaced by vdso_remap()
>
> For arch_unmap(), I'm wondering how/what other architectures do, because
> powerpc seems to be the only one to erase the vdso context pointer when
> unmapping the vdso.

Yeah. The original unmap/remap stuff was added for CRIU, which I thought
people tested on other architectures (more than powerpc even).

Possibly no one really cares about vdso unmap though, vs just moving the
vdso.

We added a test for vdso unmap recently because it happened to trigger a
KAUP failure, and someone actually hit it & reported it.

Running that test on arm64 segfaults:

# ./sigreturn_vdso
VDSO is at 0xffff8191f000-0xffff8191ffff (4096 bytes)
Signal delivered OK with VDSO mapped
VDSO moved to 0xffff8191a000-0xffff8191afff (4096 bytes)
Signal delivered OK with VDSO moved
Unmapped VDSO
Remapped the stack executable
[ 48.556191] potentially unexpected fatal signal 11.
[ 48.556752] CPU: 0 PID: 140 Comm: sigreturn_vdso Not tainted 5.9.0-rc2-00057-g2ac69819ba9e #190
[ 48.556990] Hardware name: linux,dummy-virt (DT)
[ 48.557336] pstate: 60001000 (nZCv daif -PAN -UAO BTYPE=--)
[ 48.557475] pc : 0000ffff8191a7bc
[ 48.557603] lr : 0000ffff8191a7bc
[ 48.557697] sp : 0000ffffc13c9e90
[ 48.557873] x29: 0000ffffc13cb0e0 x28: 0000000000000000
[ 48.558201] x27: 0000000000000000 x26: 0000000000000000
[ 48.558337] x25: 0000000000000000 x24: 0000000000000000
[ 48.558754] x23: 0000000000000000 x22: 0000000000000000
[ 48.558893] x21: 00000000004009b0 x20: 0000000000000000
[ 48.559046] x19: 0000000000400ff0 x18: 0000000000000000
[ 48.559180] x17: 0000ffff817da300 x16: 0000000000412010
[ 48.559312] x15: 0000000000000000 x14: 000000000000001c
[ 48.559443] x13: 656c626174756365 x12: 7865206b63617473
[ 48.559625] x11: 0000000000000003 x10: 0101010101010101
[ 48.559828] x9 : 0000ffff818afda8 x8 : 0000000000000081
[ 48.559973] x7 : 6174732065687420 x6 : 64657070616d6552
[ 48.560115] x5 : 000000000e0388bd x4 : 000000000040135d
[ 48.560270] x3 : 0000000000000000 x2 : 0000000000000001
[ 48.560412] x1 : 0000000000000003 x0 : 00000000004120b8
Segmentation fault
#

So I think we need to keep the unmap hook. Maybe it should be handled by
the special_mapping stuff generically.

cheers

2020-08-27 20:36:14

by Dmitry Safonov

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

Hello,

On Wed, 26 Aug 2020 at 15:39, Michael Ellerman <[email protected]> wrote:
> Christophe Leroy <[email protected]> writes:
[..]
> > arch_remap() gets replaced by vdso_remap()
> >
> > For arch_unmap(), I'm wondering how/what other architectures do, because
> > powerpc seems to be the only one to erase the vdso context pointer when
> > unmapping the vdso.
>
> Yeah. The original unmap/remap stuff was added for CRIU, which I thought
> people tested on other architectures (more than powerpc even).
>
> Possibly no one really cares about vdso unmap though, vs just moving the
> vdso.
>
> We added a test for vdso unmap recently because it happened to trigger a
> KAUP failure, and someone actually hit it & reported it.

You right, CRIU cares much more about moving vDSO.
It's done for each restoree and as on most setups vDSO is premapped and
used by the application - it's actively tested.
Speaking about vDSO unmap - that's concerning only for heterogeneous C/R,
i.e when an application is migrated from a system that uses vDSO to the one
which doesn't - it's much rare scenario.
(for arm it's !CONFIG_VDSO, for x86 it's `vdso=0` boot parameter)

Looking at the code, it seems quite easy to provide/maintain .close() for
vm_special_mapping. A bit harder to add a test from CRIU side
(as glibc won't know on restore that it can't use vdso anymore),
but totally not impossible.

> Running that test on arm64 segfaults:
>
> # ./sigreturn_vdso
> VDSO is at 0xffff8191f000-0xffff8191ffff (4096 bytes)
> Signal delivered OK with VDSO mapped
> VDSO moved to 0xffff8191a000-0xffff8191afff (4096 bytes)
> Signal delivered OK with VDSO moved
> Unmapped VDSO
> Remapped the stack executable
> [ 48.556191] potentially unexpected fatal signal 11.
> [ 48.556752] CPU: 0 PID: 140 Comm: sigreturn_vdso Not tainted 5.9.0-rc2-00057-g2ac69819ba9e #190
> [ 48.556990] Hardware name: linux,dummy-virt (DT)
> [ 48.557336] pstate: 60001000 (nZCv daif -PAN -UAO BTYPE=--)
> [ 48.557475] pc : 0000ffff8191a7bc
> [ 48.557603] lr : 0000ffff8191a7bc
> [ 48.557697] sp : 0000ffffc13c9e90
> [ 48.557873] x29: 0000ffffc13cb0e0 x28: 0000000000000000
> [ 48.558201] x27: 0000000000000000 x26: 0000000000000000
> [ 48.558337] x25: 0000000000000000 x24: 0000000000000000
> [ 48.558754] x23: 0000000000000000 x22: 0000000000000000
> [ 48.558893] x21: 00000000004009b0 x20: 0000000000000000
> [ 48.559046] x19: 0000000000400ff0 x18: 0000000000000000
> [ 48.559180] x17: 0000ffff817da300 x16: 0000000000412010
> [ 48.559312] x15: 0000000000000000 x14: 000000000000001c
> [ 48.559443] x13: 656c626174756365 x12: 7865206b63617473
> [ 48.559625] x11: 0000000000000003 x10: 0101010101010101
> [ 48.559828] x9 : 0000ffff818afda8 x8 : 0000000000000081
> [ 48.559973] x7 : 6174732065687420 x6 : 64657070616d6552
> [ 48.560115] x5 : 000000000e0388bd x4 : 000000000040135d
> [ 48.560270] x3 : 0000000000000000 x2 : 0000000000000001
> [ 48.560412] x1 : 0000000000000003 x0 : 00000000004120b8
> Segmentation fault
> #
>
> So I think we need to keep the unmap hook. Maybe it should be handled by
> the special_mapping stuff generically.

I'll cook a patch for vm_special_mapping if you don't mind :-)

Thanks,
Dmitry

2020-08-28 02:16:29

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

Dmitry Safonov <[email protected]> writes:
> Hello,
>
> On Wed, 26 Aug 2020 at 15:39, Michael Ellerman <[email protected]> wrote:
>> Christophe Leroy <[email protected]> writes:
> [..]
>> > arch_remap() gets replaced by vdso_remap()
>> >
>> > For arch_unmap(), I'm wondering how/what other architectures do, because
>> > powerpc seems to be the only one to erase the vdso context pointer when
>> > unmapping the vdso.
>>
>> Yeah. The original unmap/remap stuff was added for CRIU, which I thought
>> people tested on other architectures (more than powerpc even).
>>
>> Possibly no one really cares about vdso unmap though, vs just moving the
>> vdso.
>>
>> We added a test for vdso unmap recently because it happened to trigger a
>> KAUP failure, and someone actually hit it & reported it.
>
> You right, CRIU cares much more about moving vDSO.
> It's done for each restoree and as on most setups vDSO is premapped and
> used by the application - it's actively tested.
> Speaking about vDSO unmap - that's concerning only for heterogeneous C/R,
> i.e when an application is migrated from a system that uses vDSO to the one
> which doesn't - it's much rare scenario.
> (for arm it's !CONFIG_VDSO, for x86 it's `vdso=0` boot parameter)

Ah OK that explains it.

The case we hit of VDSO unmapping was some strange "library OS" thing
which had explicitly unmapped the VDSO, so also very rare.

> Looking at the code, it seems quite easy to provide/maintain .close() for
> vm_special_mapping. A bit harder to add a test from CRIU side
> (as glibc won't know on restore that it can't use vdso anymore),
> but totally not impossible.
>
>> Running that test on arm64 segfaults:
>>
>> # ./sigreturn_vdso
>> VDSO is at 0xffff8191f000-0xffff8191ffff (4096 bytes)
>> Signal delivered OK with VDSO mapped
>> VDSO moved to 0xffff8191a000-0xffff8191afff (4096 bytes)
>> Signal delivered OK with VDSO moved
>> Unmapped VDSO
>> Remapped the stack executable
>> [ 48.556191] potentially unexpected fatal signal 11.
>> [ 48.556752] CPU: 0 PID: 140 Comm: sigreturn_vdso Not tainted 5.9.0-rc2-00057-g2ac69819ba9e #190
>> [ 48.556990] Hardware name: linux,dummy-virt (DT)
>> [ 48.557336] pstate: 60001000 (nZCv daif -PAN -UAO BTYPE=--)
>> [ 48.557475] pc : 0000ffff8191a7bc
>> [ 48.557603] lr : 0000ffff8191a7bc
>> [ 48.557697] sp : 0000ffffc13c9e90
>> [ 48.557873] x29: 0000ffffc13cb0e0 x28: 0000000000000000
>> [ 48.558201] x27: 0000000000000000 x26: 0000000000000000
>> [ 48.558337] x25: 0000000000000000 x24: 0000000000000000
>> [ 48.558754] x23: 0000000000000000 x22: 0000000000000000
>> [ 48.558893] x21: 00000000004009b0 x20: 0000000000000000
>> [ 48.559046] x19: 0000000000400ff0 x18: 0000000000000000
>> [ 48.559180] x17: 0000ffff817da300 x16: 0000000000412010
>> [ 48.559312] x15: 0000000000000000 x14: 000000000000001c
>> [ 48.559443] x13: 656c626174756365 x12: 7865206b63617473
>> [ 48.559625] x11: 0000000000000003 x10: 0101010101010101
>> [ 48.559828] x9 : 0000ffff818afda8 x8 : 0000000000000081
>> [ 48.559973] x7 : 6174732065687420 x6 : 64657070616d6552
>> [ 48.560115] x5 : 000000000e0388bd x4 : 000000000040135d
>> [ 48.560270] x3 : 0000000000000000 x2 : 0000000000000001
>> [ 48.560412] x1 : 0000000000000003 x0 : 00000000004120b8
>> Segmentation fault
>> #
>>
>> So I think we need to keep the unmap hook. Maybe it should be handled by
>> the special_mapping stuff generically.
>
> I'll cook a patch for vm_special_mapping if you don't mind :-)

That would be great, thanks!

cheers

2020-09-21 11:28:34

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
> Dmitry Safonov <[email protected]> writes:
> > On Wed, 26 Aug 2020 at 15:39, Michael Ellerman <[email protected]> wrote:
> >> Christophe Leroy <[email protected]> writes:
> >> We added a test for vdso unmap recently because it happened to trigger a
> >> KAUP failure, and someone actually hit it & reported it.
> >
> > You right, CRIU cares much more about moving vDSO.
> > It's done for each restoree and as on most setups vDSO is premapped and
> > used by the application - it's actively tested.
> > Speaking about vDSO unmap - that's concerning only for heterogeneous C/R,
> > i.e when an application is migrated from a system that uses vDSO to the one
> > which doesn't - it's much rare scenario.
> > (for arm it's !CONFIG_VDSO, for x86 it's `vdso=0` boot parameter)
>
> Ah OK that explains it.
>
> The case we hit of VDSO unmapping was some strange "library OS" thing
> which had explicitly unmapped the VDSO, so also very rare.
>
> > Looking at the code, it seems quite easy to provide/maintain .close() for
> > vm_special_mapping. A bit harder to add a test from CRIU side
> > (as glibc won't know on restore that it can't use vdso anymore),
> > but totally not impossible.
> >
> >> Running that test on arm64 segfaults:
> >>
> >> # ./sigreturn_vdso
> >> VDSO is at 0xffff8191f000-0xffff8191ffff (4096 bytes)
> >> Signal delivered OK with VDSO mapped
> >> VDSO moved to 0xffff8191a000-0xffff8191afff (4096 bytes)
> >> Signal delivered OK with VDSO moved
> >> Unmapped VDSO
> >> Remapped the stack executable
> >> [ 48.556191] potentially unexpected fatal signal 11.
> >> [ 48.556752] CPU: 0 PID: 140 Comm: sigreturn_vdso Not tainted 5.9.0-rc2-00057-g2ac69819ba9e #190
> >> [ 48.556990] Hardware name: linux,dummy-virt (DT)
> >> [ 48.557336] pstate: 60001000 (nZCv daif -PAN -UAO BTYPE=--)
> >> [ 48.557475] pc : 0000ffff8191a7bc
> >> [ 48.557603] lr : 0000ffff8191a7bc
> >> [ 48.557697] sp : 0000ffffc13c9e90
> >> [ 48.557873] x29: 0000ffffc13cb0e0 x28: 0000000000000000
> >> [ 48.558201] x27: 0000000000000000 x26: 0000000000000000
> >> [ 48.558337] x25: 0000000000000000 x24: 0000000000000000
> >> [ 48.558754] x23: 0000000000000000 x22: 0000000000000000
> >> [ 48.558893] x21: 00000000004009b0 x20: 0000000000000000
> >> [ 48.559046] x19: 0000000000400ff0 x18: 0000000000000000
> >> [ 48.559180] x17: 0000ffff817da300 x16: 0000000000412010
> >> [ 48.559312] x15: 0000000000000000 x14: 000000000000001c
> >> [ 48.559443] x13: 656c626174756365 x12: 7865206b63617473
> >> [ 48.559625] x11: 0000000000000003 x10: 0101010101010101
> >> [ 48.559828] x9 : 0000ffff818afda8 x8 : 0000000000000081
> >> [ 48.559973] x7 : 6174732065687420 x6 : 64657070616d6552
> >> [ 48.560115] x5 : 000000000e0388bd x4 : 000000000040135d
> >> [ 48.560270] x3 : 0000000000000000 x2 : 0000000000000001
> >> [ 48.560412] x1 : 0000000000000003 x0 : 00000000004120b8
> >> Segmentation fault
> >> #
> >>
> >> So I think we need to keep the unmap hook. Maybe it should be handled by
> >> the special_mapping stuff generically.
> >
> > I'll cook a patch for vm_special_mapping if you don't mind :-)
>
> That would be great, thanks!

I lost track of this one. Is there a patch kicking around to resolve this,
or is the segfault expected behaviour?

Will

2020-09-27 07:45:10

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()



Le 21/09/2020 à 13:26, Will Deacon a écrit :
> On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
>> Dmitry Safonov <[email protected]> writes:
>>> On Wed, 26 Aug 2020 at 15:39, Michael Ellerman <[email protected]> wrote:
>>>> Christophe Leroy <[email protected]> writes:
>>>> We added a test for vdso unmap recently because it happened to trigger a
>>>> KAUP failure, and someone actually hit it & reported it.
>>>
>>> You right, CRIU cares much more about moving vDSO.
>>> It's done for each restoree and as on most setups vDSO is premapped and
>>> used by the application - it's actively tested.
>>> Speaking about vDSO unmap - that's concerning only for heterogeneous C/R,
>>> i.e when an application is migrated from a system that uses vDSO to the one
>>> which doesn't - it's much rare scenario.
>>> (for arm it's !CONFIG_VDSO, for x86 it's `vdso=0` boot parameter)
>>
>> Ah OK that explains it.
>>
>> The case we hit of VDSO unmapping was some strange "library OS" thing
>> which had explicitly unmapped the VDSO, so also very rare.
>>
>>> Looking at the code, it seems quite easy to provide/maintain .close() for
>>> vm_special_mapping. A bit harder to add a test from CRIU side
>>> (as glibc won't know on restore that it can't use vdso anymore),
>>> but totally not impossible.
>>>
>>>> Running that test on arm64 segfaults:
>>>>
>>>> # ./sigreturn_vdso
>>>> VDSO is at 0xffff8191f000-0xffff8191ffff (4096 bytes)
>>>> Signal delivered OK with VDSO mapped
>>>> VDSO moved to 0xffff8191a000-0xffff8191afff (4096 bytes)
>>>> Signal delivered OK with VDSO moved
>>>> Unmapped VDSO
>>>> Remapped the stack executable
>>>> [ 48.556191] potentially unexpected fatal signal 11.
>>>> [ 48.556752] CPU: 0 PID: 140 Comm: sigreturn_vdso Not tainted 5.9.0-rc2-00057-g2ac69819ba9e #190
>>>> [ 48.556990] Hardware name: linux,dummy-virt (DT)
>>>> [ 48.557336] pstate: 60001000 (nZCv daif -PAN -UAO BTYPE=--)
>>>> [ 48.557475] pc : 0000ffff8191a7bc
>>>> [ 48.557603] lr : 0000ffff8191a7bc
>>>> [ 48.557697] sp : 0000ffffc13c9e90
>>>> [ 48.557873] x29: 0000ffffc13cb0e0 x28: 0000000000000000
>>>> [ 48.558201] x27: 0000000000000000 x26: 0000000000000000
>>>> [ 48.558337] x25: 0000000000000000 x24: 0000000000000000
>>>> [ 48.558754] x23: 0000000000000000 x22: 0000000000000000
>>>> [ 48.558893] x21: 00000000004009b0 x20: 0000000000000000
>>>> [ 48.559046] x19: 0000000000400ff0 x18: 0000000000000000
>>>> [ 48.559180] x17: 0000ffff817da300 x16: 0000000000412010
>>>> [ 48.559312] x15: 0000000000000000 x14: 000000000000001c
>>>> [ 48.559443] x13: 656c626174756365 x12: 7865206b63617473
>>>> [ 48.559625] x11: 0000000000000003 x10: 0101010101010101
>>>> [ 48.559828] x9 : 0000ffff818afda8 x8 : 0000000000000081
>>>> [ 48.559973] x7 : 6174732065687420 x6 : 64657070616d6552
>>>> [ 48.560115] x5 : 000000000e0388bd x4 : 000000000040135d
>>>> [ 48.560270] x3 : 0000000000000000 x2 : 0000000000000001
>>>> [ 48.560412] x1 : 0000000000000003 x0 : 00000000004120b8
>>>> Segmentation fault
>>>> #
>>>>
>>>> So I think we need to keep the unmap hook. Maybe it should be handled by
>>>> the special_mapping stuff generically.
>>>
>>> I'll cook a patch for vm_special_mapping if you don't mind :-)
>>
>> That would be great, thanks!
>
> I lost track of this one. Is there a patch kicking around to resolve this,
> or is the segfault expected behaviour?
>

IIUC dmitry said he will cook a patch. I have not seen any patch yet.

AFAIKS, among the architectures having VDSO sigreturn trampolines, only SH, X86 and POWERPC provide
alternative trampoline on stack when VDSO is not there.

All other architectures just having a VDSO don't expect VDSO to not be mapped.

As far as nowadays stacks are mapped non-executable, getting a segfaut is expected behaviour.
However, I think we should really make it cleaner. Today it segfaults because it is still pointing
to the VDSO trampoline that has been unmapped. But should the user map some other code at the same
address, we'll run in the weed on signal return instead of segfaulting.

So VDSO unmapping should really be properly managed, the reference should be properly cleared in
order to segfault in a controllable manner.

Only powerpc has a hook to properly clear the VDSO pointer when VDSO is unmapped.

Christophe

2020-09-28 15:10:18

by Dmitry Safonov

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

On 9/27/20 8:43 AM, Christophe Leroy wrote:
>
>
> Le 21/09/2020 à 13:26, Will Deacon a écrit :
>> On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
>>> Dmitry Safonov <[email protected]> writes:
[..]
>>>> I'll cook a patch for vm_special_mapping if you don't mind :-)
>>>
>>> That would be great, thanks!
>>
>> I lost track of this one. Is there a patch kicking around to resolve
>> this,
>> or is the segfault expected behaviour?
>>
>
> IIUC dmitry said he will cook a patch. I have not seen any patch yet.

Yes, sorry about the delay - I was a bit busy with xfrm patches.

I'll send patches for .close() this week, working on them now.

> AFAIKS, among the architectures having VDSO sigreturn trampolines, only
> SH, X86 and POWERPC provide alternative trampoline on stack when VDSO is
> not there.
>
> All other architectures just having a VDSO don't expect VDSO to not be
> mapped.
>
> As far as nowadays stacks are mapped non-executable, getting a segfaut
> is expected behaviour. However, I think we should really make it
> cleaner. Today it segfaults because it is still pointing to the VDSO
> trampoline that has been unmapped. But should the user map some other
> code at the same address, we'll run in the weed on signal return instead
> of segfaulting.

+1.

> So VDSO unmapping should really be properly managed, the reference
> should be properly cleared in order to segfault in a controllable manner.
>
> Only powerpc has a hook to properly clear the VDSO pointer when VDSO is
> unmapped.

Thanks,
Dmitry

2020-10-23 11:24:05

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

Hi Dmitry,

Le 28/09/2020 à 17:08, Dmitry Safonov a écrit :
> On 9/27/20 8:43 AM, Christophe Leroy wrote:
>>
>>
>> Le 21/09/2020 à 13:26, Will Deacon a écrit :
>>> On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
>>>> Dmitry Safonov <[email protected]> writes:
> [..]
>>>>> I'll cook a patch for vm_special_mapping if you don't mind :-)
>>>>
>>>> That would be great, thanks!
>>>
>>> I lost track of this one. Is there a patch kicking around to resolve
>>> this,
>>> or is the segfault expected behaviour?
>>>
>>
>> IIUC dmitry said he will cook a patch. I have not seen any patch yet.
>
> Yes, sorry about the delay - I was a bit busy with xfrm patches.
>
> I'll send patches for .close() this week, working on them now.

I haven't seen the patches, did you sent them out finally ?

Christophe

2020-10-23 15:37:44

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

On Fri, Oct 23, 2020 at 01:22:04PM +0200, Christophe Leroy wrote:
> Hi Dmitry,
>
> Le 28/09/2020 ? 17:08, Dmitry Safonov a ?crit?:
> > On 9/27/20 8:43 AM, Christophe Leroy wrote:
> > >
> > >
> > > Le 21/09/2020 ? 13:26, Will Deacon a ?crit?:
> > > > On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
> > > > > Dmitry Safonov <[email protected]> writes:
> > [..]
> > > > > > I'll cook a patch for vm_special_mapping if you don't mind :-)
> > > > >
> > > > > That would be great, thanks!
> > > >
> > > > I lost track of this one. Is there a patch kicking around to resolve
> > > > this,
> > > > or is the segfault expected behaviour?
> > > >
> > >
> > > IIUC dmitry said he will cook a patch. I have not seen any patch yet.
> >
> > Yes, sorry about the delay - I was a bit busy with xfrm patches.
> >
> > I'll send patches for .close() this week, working on them now.
>
> I haven't seen the patches, did you sent them out finally ?

I think it's this series:

https://lore.kernel.org/r/[email protected]

but they look really invasive to me, so I may cook a small hack for arm64
in the meantine / for stable.

Will

2020-10-23 16:00:29

by Dmitry Safonov

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()

Hi Christophe, Will,

On 10/23/20 12:57 PM, Christophe Leroy wrote:
>
>
> Le 23/10/2020 à 13:25, Will Deacon a écrit :
>> On Fri, Oct 23, 2020 at 01:22:04PM +0200, Christophe Leroy wrote:
>>> Hi Dmitry,
[..]
>>> I haven't seen the patches, did you sent them out finally ?

I was working on .close() hook, but while cooking it, I thought it may
be better to make tracking of user landing generic. Note that the vdso
base address is mostly needed by kernel as an address to land in
userspace after processing a signal.

I have some raw patches that add
+#ifdef CONFIG_ARCH_HAS_USER_LANDING
+ struct vm_area_struct *user_landing;
+#endif
inside mm_struct and I plan to finish them after rc1 gets released.

While working on that, I noticed that arm32 and some other architectures
track vdso position in mm.context with the only reason to add
AT_SYSINFO_EHDR in the elf header that's being loaded. That's quite
overkill to have a pointer in mm.context that rather can be a local
variable in elf binfmt loader. Also, I found some issues with mremap
code. The patches series mentioned are at the base of the branch with
generic user landing. I have sent only those patches not the full branch
as I remember there was a policy that during merge window one should
send only fixes, rather than refactoring/new code.

>> I think it's this series:
>>
>> https://lore.kernel.org/r/[email protected]
>>
>> but they look really invasive to me, so I may cook a small hack for arm64
>> in the meantine / for stable.

I don't mind small hacks, but I'm concerned that the suggested fix which
sets `mm->context.vdso_base = 0` on munmap() may have it's issue: that
won't work if a user for whatever-broken-reason will mremap() vdso on 0
address. As the fix supposes to fix an issue that hasn't fired for
anyone yet, it probably shouldn't introduce another. That's why I've
used vm_area_struct to track vdso position in the patches set.
Probably, temporary, you could use something like:
#define BAD_VDSO_ADDRESS (-1)UL
Or non-page-aligned address.
But the signal code that checks if it can land on vdso/sigpage should be
also aligned with the new definition.

> Not sure we are talking about the same thing.
>
> I can't see any new .close function added to vm_special_mapping in order
> to replace arch_unmap() hook.
Thanks,
Dmitry

2020-10-23 18:13:03

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()



Le 23/10/2020 à 13:25, Will Deacon a écrit :
> On Fri, Oct 23, 2020 at 01:22:04PM +0200, Christophe Leroy wrote:
>> Hi Dmitry,
>>
>> Le 28/09/2020 à 17:08, Dmitry Safonov a écrit :
>>> On 9/27/20 8:43 AM, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 21/09/2020 à 13:26, Will Deacon a écrit :
>>>>> On Fri, Aug 28, 2020 at 12:14:28PM +1000, Michael Ellerman wrote:
>>>>>> Dmitry Safonov <[email protected]> writes:
>>> [..]
>>>>>>> I'll cook a patch for vm_special_mapping if you don't mind :-)
>>>>>>
>>>>>> That would be great, thanks!
>>>>>
>>>>> I lost track of this one. Is there a patch kicking around to resolve
>>>>> this,
>>>>> or is the segfault expected behaviour?
>>>>>
>>>>
>>>> IIUC dmitry said he will cook a patch. I have not seen any patch yet.
>>>
>>> Yes, sorry about the delay - I was a bit busy with xfrm patches.
>>>
>>> I'll send patches for .close() this week, working on them now.
>>
>> I haven't seen the patches, did you sent them out finally ?
>
> I think it's this series:
>
> https://lore.kernel.org/r/[email protected]
>
> but they look really invasive to me, so I may cook a small hack for arm64
> in the meantine / for stable.
>

Not sure we are talking about the same thing.

I can't see any new .close function added to vm_special_mapping in order to replace arch_unmap() hook.

Christophe