On Sat, Jul 24 2021, Mark Kettenis <mark.kette...@xs4all.nl> wrote:
>> From: Jeremie Courreges-Anglas <j...@wxcvbn.org>
>> Date: Sat, 24 Jul 2021 21:22:23 +0200
>>
>> hifive /usr/src/regress/sys/kern/gettimeofday$ doas -u build time
>> obj/gettimeofday
>> 6.64 real 6.63 user 0.02 sys
>> hifive /usr/src/regress/sys/kern/gettimeofday$ doas -u build env
>> LIBC_NOUSERTC=1 time obj/gettimeofday
>> 6.48 real 0.60 user 5.42 sys
>>
>> Initially I thought that a more descriptive name than TC_TB could be
>> helpful (TC_TIMEBASE?). But since powerpc also uses TC_TB I think it's
>> fine as a first step. We can change it later easily, it's just a define
>> name.
>>
>> I haven't even built a release with this, not sure it's worth it.
>> If you have cpu cycles to spare, please say so.
>>
>> ok?
>
> Two small nits below. With that fixed, ok kettenis@
[...]
>> Index: lib/libc/arch/riscv64/gen/usertc.c
>> ===================================================================
>> RCS file: /cvs/src/lib/libc/arch/riscv64/gen/usertc.c,v
>> retrieving revision 1.1
>> diff -u -p -r1.1 usertc.c
>> --- lib/libc/arch/riscv64/gen/usertc.c 29 Apr 2021 18:33:36 -0000
>> 1.1
>> +++ lib/libc/arch/riscv64/gen/usertc.c 24 Jul 2021 17:07:01 -0000
>> @@ -1,6 +1,7 @@
>> /* $OpenBSD: usertc.c,v 1.1 2021/04/29 18:33:36 drahn Exp $ */
>> /*
>> * Copyright (c) 2020 Paul Irofti <p...@irofti.net>
>> + * Copyright (c) 2021 Jeremie Courreges-Anglas <j...@wxcvbn.org>
>> *
>> * Permission to use, copy, modify, and distribute this software for any
>> * purpose with or without fee is hereby granted, provided that the above
>> @@ -18,4 +19,24 @@
>> #include <sys/types.h>
>> #include <sys/timetc.h>
>>
>> -int (*const _tc_get_timecount)(struct timekeep *, u_int *) = NULL;
>> +static inline u_int
>> +rdtime(void)
>> +{
>> + uint64_t ret;
>> + asm volatile("rdtime %0" : "=r"(ret));
>
> Can you make that __asm vol[a]tile?
Done. I copied that from amd64 usertc.c.
>> + return ret & 0xffffffff;
>
> The & 0xffffffff isn't really necessary here and the kernel doesn't do
> it. So I'd drop that bit and simply return ret.
I thought I would make it explicit to the reader that we only cared
about the low 32 bits, rather then rely on the implicit truncation.
Your nit was about consistency, what about trying to make other
implementations consistent? Or should we make it explicit on other
archs?
Two changes, only compile-tested on amd64 and sparc64:
- asm/__asm__ -> __asm
- val & mask -> val
I can also drop this diff, consistency is good but so is time on our
hands.
Index: lib/libc/arch/aarch64/gen/usertc.c
===================================================================
RCS file: /d/cvs/src/lib/libc/arch/aarch64/gen/usertc.c,v
retrieving revision 1.2
diff -u -p -r1.2 usertc.c
--- lib/libc/arch/aarch64/gen/usertc.c 15 Jul 2020 22:58:33 -0000 1.2
+++ lib/libc/arch/aarch64/gen/usertc.c 24 Jul 2021 23:45:52 -0000
@@ -29,7 +29,7 @@ agtimer_get_timecount(struct timecounter
*/
__asm volatile("isb" ::: "memory");
__asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val));
- return (val & 0xffffffff);
+ return val;
}
static int
Index: lib/libc/arch/amd64/gen/usertc.c
===================================================================
RCS file: /d/cvs/src/lib/libc/arch/amd64/gen/usertc.c,v
retrieving revision 1.3
diff -u -p -r1.3 usertc.c
--- lib/libc/arch/amd64/gen/usertc.c 23 Aug 2020 21:38:47 -0000 1.3
+++ lib/libc/arch/amd64/gen/usertc.c 24 Jul 2021 23:45:52 -0000
@@ -22,7 +22,7 @@ static inline u_int
rdtsc_lfence(void)
{
uint32_t hi, lo;
- asm volatile("lfence; rdtsc" : "=a"(lo), "=d"(hi));
+ __asm volatile("lfence; rdtsc" : "=a"(lo), "=d"(hi));
return ((uint64_t)lo)|(((uint64_t)hi)<<32);
}
Index: lib/libc/arch/mips64/gen/usertc.c
===================================================================
RCS file: /d/cvs/src/lib/libc/arch/mips64/gen/usertc.c,v
retrieving revision 1.2
diff -u -p -r1.2 usertc.c
--- lib/libc/arch/mips64/gen/usertc.c 18 Jul 2020 08:37:43 -0000 1.2
+++ lib/libc/arch/mips64/gen/usertc.c 25 Jul 2021 02:16:21 -0000
@@ -23,7 +23,7 @@ get_cp0_count(void)
{
uint32_t count;
- __asm__ volatile (
+ __asm volatile (
" .set push\n"
" .set mips64r2\n"
" rdhwr %0, $2\n"
Index: lib/libc/arch/sparc64/gen/usertc.c
===================================================================
RCS file: /d/cvs/src/lib/libc/arch/sparc64/gen/usertc.c,v
retrieving revision 1.2
diff -u -p -r1.2 usertc.c
--- lib/libc/arch/sparc64/gen/usertc.c 8 Jul 2020 09:20:28 -0000 1.2
+++ lib/libc/arch/sparc64/gen/usertc.c 24 Jul 2021 23:45:52 -0000
@@ -25,7 +25,7 @@ tick_get_timecount(struct timecounter *t
__asm volatile("rd %%tick, %0" : "=r" (tick));
- return (tick & ~0u);
+ return tick;
}
static inline u_int
@@ -35,7 +35,7 @@ sys_tick_get_timecount(struct timecounte
__asm volatile("rd %%sys_tick, %0" : "=r" (tick));
- return (tick & ~0u);
+ return tick;
}
static int
Index: sys/arch/arm64/dev/agtimer.c
===================================================================
RCS file: /d/cvs/src/sys/arch/arm64/dev/agtimer.c,v
retrieving revision 1.18
diff -u -p -r1.18 agtimer.c
--- sys/arch/arm64/dev/agtimer.c 11 Mar 2021 11:16:56 -0000 1.18
+++ sys/arch/arm64/dev/agtimer.c 24 Jul 2021 23:45:52 -0000
@@ -206,7 +206,7 @@ agtimer_get_timecount(struct timecounter
*/
__asm volatile("isb" ::: "memory");
__asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val));
- return (val & 0xffffffff);
+ return val;
}
int
Index: sys/arch/sparc64/sparc64/clock.c
===================================================================
RCS file: /d/cvs/src/sys/arch/sparc64/sparc64/clock.c,v
retrieving revision 1.70
diff -u -p -r1.70 clock.c
--- sys/arch/sparc64/sparc64/clock.c 11 Mar 2021 11:17:00 -0000 1.70
+++ sys/arch/sparc64/sparc64/clock.c 24 Jul 2021 23:45:52 -0000
@@ -931,7 +931,7 @@ tick_get_timecount(struct timecounter *t
__asm volatile("rd %%tick, %0" : "=r" (tick));
- return (tick & ~0u);
+ return tick;
}
u_int
@@ -941,5 +941,5 @@ sys_tick_get_timecount(struct timecounte
__asm volatile("rd %%sys_tick, %0" : "=r" (tick));
- return (tick & ~0u);
+ return tick;
}
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
