Re: [PATCH 12/17] powerpc/vdso: Switch to generic storage implementation
From: Christophe Leroy
Date: Wed Dec 18 2024 - 02:21:16 EST
Le 16/12/2024 à 15:10, Thomas Weißschuh a écrit :
The generic storage implementation provides the same features as the
custom one. However it can be shared between architectures, making
maintenance easier.
Co-developed-by: Nam Cao <namcao@xxxxxxxxxxxxx>
Signed-off-by: Nam Cao <namcao@xxxxxxxxxxxxx>
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
---
arch/powerpc/Kconfig | 2 +
arch/powerpc/include/asm/vdso.h | 1 +
arch/powerpc/include/asm/vdso/arch_data.h | 37 +++++++++
arch/powerpc/include/asm/vdso/getrandom.h | 11 +--
arch/powerpc/include/asm/vdso/gettimeofday.h | 36 +++++----
arch/powerpc/include/asm/vdso/vsyscall.h | 13 ---
arch/powerpc/include/asm/vdso_datapage.h | 44 +---------
arch/powerpc/kernel/asm-offsets.c | 1 -
arch/powerpc/kernel/time.c | 2 +-
arch/powerpc/kernel/vdso.c | 115 +++------------------------
arch/powerpc/kernel/vdso/cacheflush.S | 2 +-
arch/powerpc/kernel/vdso/datapage.S | 4 +-
arch/powerpc/kernel/vdso/gettimeofday.S | 4 +-
arch/powerpc/kernel/vdso/vdso32.lds.S | 4 +-
arch/powerpc/kernel/vdso/vdso64.lds.S | 4 +-
arch/powerpc/kernel/vdso/vgettimeofday.c | 14 ++--
16 files changed, 101 insertions(+), 193 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a0ce777f97063bf858942c60654d8411bcf2a3dc..600fa3b917ee902d016f2a04376950a9dc49074f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -156,6 +156,7 @@ config PPC
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UACCESS_FLUSHCACHE
select ARCH_HAS_UBSAN
+ select ARCH_HAS_VDSO_ARCH_DATA
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select ARCH_HAVE_EXTRA_ELF_NOTES if SPU_BASE
select ARCH_KEEP_MEMBLOCK
@@ -206,6 +207,7 @@ config PPC
select GENERIC_PTDUMP
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL
+ select GENERIC_VDSO_DATA_STORE
select GENERIC_VDSO_TIME_NS
select HAS_IOPORT if PCI
select HAVE_ARCH_AUDITSYSCALL
diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h
index 8d972bc98b55fe916f23488ca9e2a5918046b9aa..1ca23fbfe087ae90b90c4286335f86d9f8121078 100644
--- a/arch/powerpc/include/asm/vdso.h
+++ b/arch/powerpc/include/asm/vdso.h
@@ -3,6 +3,7 @@
#define _ASM_POWERPC_VDSO_H
#define VDSO_VERSION_STRING LINUX_2.6.15
+#define __VDSO_PAGES 4
#ifndef __ASSEMBLY__
diff --git a/arch/powerpc/include/asm/vdso/arch_data.h b/arch/powerpc/include/asm/vdso/arch_data.h
new file mode 100644
index 0000000000000000000000000000000000000000..c240a6b875181ac4159f2e80b11f9bf214e22808
--- /dev/null
+++ b/arch/powerpc/include/asm/vdso/arch_data.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2002 Peter Bergner <bergner@xxxxxxxxxxxx>, IBM
+ * Copyright (C) 2005 Benjamin Herrenschmidy <benh@xxxxxxxxxxxxxxxxxxx>,
+ * IBM Corp.
+ */
+#ifndef _ASM_POWERPC_VDSO_ARCH_DATA_H
+#define _ASM_POWERPC_VDSO_ARCH_DATA_H
+
+#include <linux/unistd.h>
+#include <linux/types.h>
+
+#define SYSCALL_MAP_SIZE ((NR_syscalls + 31) / 32)
+
+#ifdef CONFIG_PPC64
+
+struct vdso_arch_data {
+ __u64 tb_ticks_per_sec; /* Timebase tics / sec */
+ __u32 dcache_block_size; /* L1 d-cache block size */
+ __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 syscall_map[SYSCALL_MAP_SIZE]; /* Map of syscalls */
+ __u32 compat_syscall_map[SYSCALL_MAP_SIZE]; /* Map of compat syscalls */
+};
+
+#else /* CONFIG_PPC64 */
+
+struct vdso_arch_data {
+ __u64 tb_ticks_per_sec; /* Timebase tics / sec */
+ __u32 syscall_map[SYSCALL_MAP_SIZE]; /* Map of syscalls */
+ __u32 compat_syscall_map[0]; /* No compat syscalls on PPC32 */
+};
+
+#endif /* CONFIG_PPC64 */
+
+#endif /* _ASM_POWERPC_VDSO_ARCH_DATA_H */
diff --git a/arch/powerpc/include/asm/vdso/getrandom.h b/arch/powerpc/include/asm/vdso/getrandom.h
index 80ce0709725eb89c1f3b69e0733038b458fbf24f..c82eb0d8237681a7396abfe7d161292636b8cce4 100644
--- a/arch/powerpc/include/asm/vdso/getrandom.h
+++ b/arch/powerpc/include/asm/vdso/getrandom.h
@@ -43,20 +43,21 @@ static __always_inline ssize_t getrandom_syscall(void *buffer, size_t len, unsig
(unsigned long)len, (unsigned long)flags);
}
-static __always_inline struct vdso_rng_data *__arch_get_vdso_rng_data(void)
+static __always_inline const struct vdso_rng_data *__ppc_get_vdso_u_rng_data(void)
{
- struct vdso_arch_data *data;
+ struct vdso_rng_data *data;
asm (
" bcl 20, 31, .+4 ;"
"0: mflr %0 ;"
- " addis %0, %0, (_vdso_datapage - 0b)@ha ;"
- " addi %0, %0, (_vdso_datapage - 0b)@l ;"
+ " addis %0, %0, (vdso_u_rng_data - 0b)@ha ;"
+ " addi %0, %0, (vdso_u_rng_data - 0b)@l ;"
: "=r" (data) : : "lr"
);
- return &data->rng_data;
+ return data;
}
+#define __arch_get_vdso_u_rng_data __ppc_get_vdso_u_rng_data
ssize_t __c_kernel_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state,
size_t opaque_len);
diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h b/arch/powerpc/include/asm/vdso/gettimeofday.h
index c6390890a60c2fdcb608bf321b2945c3fb372f54..bddd9cde97db197d0d3daba6c2289cb29e6b5a75 100644
--- a/arch/powerpc/include/asm/vdso/gettimeofday.h
+++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
@@ -94,22 +94,29 @@ int clock_getres32_fallback(clockid_t _clkid, struct old_timespec32 *_ts)
#endif
static __always_inline u64 __arch_get_hw_counter(s32 clock_mode,
- const struct vdso_data *vd)
+ const struct vdso_time_data *vd)
{
return get_tb();
}
-const struct vdso_data *__arch_get_vdso_data(void);
-
#ifdef CONFIG_TIME_NS
-static __always_inline
-const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
+static __always_inline const struct vdso_time_data *__ppc_get_vdso_u_timens_data(void)
{
- return (void *)vd + (1U << CONFIG_PAGE_SHIFT);
+ struct vdso_time_data *time_data;
+
+ asm(
+ " bcl 20, 31, .+4\n"
+ "0: mflr %0\n"
+ " addis %0, %0, (vdso_u_timens_data - 0b)@ha\n"
+ " addi %0, %0, (vdso_u_timens_data - 0b)@l\n"
+ : "=r" (time_data) :: "lr");
+
+ return time_data;
Please don't do that, it kills optimisation efforts done when
implementing VDSO time. Commit ce7d8056e38b ("powerpc/vdso: Prepare for
switching VDSO to generic C implementation.") explains why.
For time data, the bcl/mflr dance is done by get_datapage macro called
by cvdso_call macro in gettimeofday.S, and given to
__cvdso_clock_gettime_data() by __c_kernel_clock_gettime() in
vgettimeofday.c . Use that information and don't redo the bcl/mflr sequence.
See for instance function __c_kernel_clock_getres():
Before your series it is 30 instructions.
After your series it is 59 instructions.
Before:
000010f8 <__c_kernel_clock_getres>:
10f8: 28 03 00 0f cmplwi r3,15
10fc: 41 81 00 60 bgt 115c <__c_kernel_clock_getres+0x64>
1100: 81 45 00 04 lwz r10,4(r5)
1104: 3d 20 7f ff lis r9,32767
1108: 61 29 ff ff ori r9,r9,65535
110c: 7c 0a 48 00 cmpw r10,r9
1110: 40 a2 00 08 bne 1118 <__c_kernel_clock_getres+0x20>
1114: 38 a5 40 00 addi r5,r5,16384
1118: 39 20 00 01 li r9,1
111c: 7d 29 18 30 slw r9,r9,r3
1120: 71 2a 08 93 andi. r10,r9,2195
1124: 40 82 00 30 bne 1154 <__c_kernel_clock_getres+0x5c>
1128: 71 29 00 60 andi. r9,r9,96
112c: 41 82 00 30 beq 115c <__c_kernel_clock_getres+0x64>
1130: 3d 20 00 98 lis r9,152
1134: 61 29 96 80 ori r9,r9,38528
1138: 2c 04 00 00 cmpwi r4,0
113c: 41 82 00 10 beq 114c <__c_kernel_clock_getres+0x54>
1140: 39 40 00 00 li r10,0
1144: 91 24 00 04 stw r9,4(r4)
1148: 91 44 00 00 stw r10,0(r4)
114c: 38 60 00 00 li r3,0
1150: 4e 80 00 20 blr
1154: 81 25 00 e8 lwz r9,232(r5)
1158: 4b ff ff e0 b 1138 <__c_kernel_clock_getres+0x40>
115c: 38 00 00 f7 li r0,247
1160: 44 00 00 02 sc
1164: 40 a3 00 08 bns 116c <__c_kernel_clock_getres+0x74>
1168: 7c 63 00 d0 neg r3,r3
116c: 4e 80 00 20 blr
After:
000011ac <__c_kernel_clock_getres>:
11ac: 28 03 00 0f cmplwi r3,15
11b0: 41 81 00 c0 bgt 1270 <__c_kernel_clock_getres+0xc4>
11b4: 81 45 00 04 lwz r10,4(r5)
11b8: 3d 20 7f ff lis r9,32767
11bc: 61 29 ff ff ori r9,r9,65535
11c0: 7c 0a 48 00 cmpw r10,r9
11c4: 41 82 00 48 beq 120c <__c_kernel_clock_getres+0x60>
11c8: 39 20 00 01 li r9,1
11cc: 7d 29 18 30 slw r9,r9,r3
11d0: 71 2a 08 93 andi. r10,r9,2195
11d4: 40 82 00 30 bne 1204 <__c_kernel_clock_getres+0x58>
11d8: 71 29 00 60 andi. r9,r9,96
11dc: 41 82 00 94 beq 1270 <__c_kernel_clock_getres+0xc4>
11e0: 3d 20 00 98 lis r9,152
11e4: 61 29 96 80 ori r9,r9,38528
11e8: 2c 04 00 00 cmpwi r4,0
11ec: 41 82 00 10 beq 11fc <__c_kernel_clock_getres+0x50>
11f0: 39 40 00 00 li r10,0
11f4: 91 24 00 04 stw r9,4(r4)
11f8: 91 44 00 00 stw r10,0(r4)
11fc: 38 60 00 00 li r3,0
1200: 4e 80 00 20 blr
1204: 81 25 00 e8 lwz r9,232(r5)
1208: 4b ff ff e0 b 11e8 <__c_kernel_clock_getres+0x3c>
120c: 39 20 00 01 li r9,1
1210: 7d 29 18 30 slw r9,r9,r3
1214: 71 2a 08 93 andi. r10,r9,2195
1218: 7c 08 02 a6 mflr r0
121c: 90 01 00 04 stw r0,4(r1)
1220: 42 9f 00 05 bcl 20,4*cr7+so,1224
<__c_kernel_clock_getres+0x78>
1224: 7c a8 02 a6 mflr r5
1228: 3c a5 ff ff addis r5,r5,-1
122c: 38 a5 2d dc addi r5,r5,11740
1230: 40 82 00 38 bne 1268 <__c_kernel_clock_getres+0xbc>
1234: 71 29 00 60 andi. r9,r9,96
1238: 41 82 00 4c beq 1284 <__c_kernel_clock_getres+0xd8>
123c: 3d 20 00 98 lis r9,152
1240: 61 29 96 80 ori r9,r9,38528
1244: 2c 04 00 00 cmpwi r4,0
1248: 41 82 00 10 beq 1258 <__c_kernel_clock_getres+0xac>
124c: 39 40 00 00 li r10,0
1250: 91 24 00 04 stw r9,4(r4)
1254: 91 44 00 00 stw r10,0(r4)
1258: 38 60 00 00 li r3,0
125c: 80 01 00 04 lwz r0,4(r1)
1260: 7c 08 03 a6 mtlr r0
1264: 4e 80 00 20 blr
1268: 81 25 00 e8 lwz r9,232(r5)
126c: 4b ff ff d8 b 1244 <__c_kernel_clock_getres+0x98>
1270: 38 00 00 f7 li r0,247
1274: 44 00 00 02 sc
1278: 40 a3 00 08 bns 1280 <__c_kernel_clock_getres+0xd4>
127c: 7c 63 00 d0 neg r3,r3
1280: 4e 80 00 20 blr
1284: 38 00 00 f7 li r0,247
1288: 44 00 00 02 sc
128c: 40 a3 00 08 bns 1294 <__c_kernel_clock_getres+0xe8>
1290: 7c 63 00 d0 neg r3,r3
1294: 4b ff ff c8 b 125c <__c_kernel_clock_getres+0xb0>
}
+#define __arch_get_vdso_u_timens_data __ppc_get_vdso_u_timens_data
There is not #ifdef __arch_get_vdso_u_timens_data anywhere, this #define
is not needed, the function should be called
__arch_get_vdso_u_timens_data() directly as before, unnecessary
indirections reduce readability.
#endif
-static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
+static inline bool vdso_clocksource_ok(const struct vdso_time_data *vd)
{
return true;
}