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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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




[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux