Thread (29 messages) 29 messages, 5 authors, 2025-01-09

Re: [PATCH 12/17] powerpc/vdso: Switch to generic storage implementation

From: Christophe Leroy <hidden>
Date: 2024-12-18 07:20:59
Also in: linux-arch, linux-arm-kernel, linux-mips, linux-riscv, linux-s390, lkml, loongarch


Le 16/12/2024 à 15:10, Thomas Weißschuh a écrit :
quoted hunk ↗ jump to hunk
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 <redacted>
Signed-off-by: Nam Cao <redacted>
Signed-off-by: Thomas Weißschuh <redacted>
---
  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@vnet.ibm.com>, IBM
+ * Copyright (C) 2005 Benjamin Herrenschmidy <benh@kernel.crashing.org>,
+ * 		      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;
  }
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help