> Date: Thu, 11 Jun 2020 19:38:48 +0200
> From: Christian Weisgerber <na...@mips.inka.de>
>
> Theo de Raadt:
>
> > The diff is growing complexity to support a future which wouldn't
> > exist if attempts at *supporting all* architectures received priority.
>
> Adding support for more archs is very simple, since you just need
> to copy the corresponding get_timecounter function from the kernel.
>
> Here's arm64. I'm running a kernel and libc with this.
>
> I can also provide alpha, powerpc, and sparc64, but I don't have
> such machines.
Hope you didn't spend too much time on that, because I already
mentioned that I had arm64 working earlier in the thread.
I've just fired up one of my sparc64 machines such that I can check
how well the approach works for an architecture with two exported
timecounters.
As for decreasing the complexity of the diff, I think naddy's
suggestion to have an MD header file actually helps. With that file
we can easily avoid the function pointer and it becomes easier adding
new timecounters in the kernel (which happens from time to time).
Further opportunities to decrease the complexity:
1. Drop support for CLOCK_UPTIME and CLOCK_BOOTTIME.
2. Avoid using bintime. That would decouple the libc code more from
the kernel timecounter implementation details. The downside of
this is that we can't just copy and paste kernel code like we do
now so this may not be a genuine benefit.
I consider that mostly polishing though, although the polishing has to
be done before this hits the tree.
The real question that still need to be answered are:
1. Are we happy with an implementation that can only support
clock/architectures that have a clock that can be accessed through
a register or special instruction?
2. What are we going to do with the TSC on amd64 with regards to them
not being synched up properly.
For me the answers are:
1. Yes.
2. Not expose the TSC to userland if there is a significant skew.
Cheers,
Mark
> --- ./lib/libc/arch/aarch64/gen/usertc.c.orig Thu Jun 11 19:07:39 2020
> +++ ./lib/libc/arch/aarch64/gen/usertc.c Thu Jun 11 19:08:01 2020
> @@ -18,4 +18,32 @@
> #include <sys/types.h>
> #include <sys/timetc.h>
>
> -int (*const _tc_get_timecount)(struct timekeep *, uint64_t *) = NULL;
> +static uint64_t
> +readcnt64(void)
> +{
> + uint64_t val0, val1;
> +
> + /*
> + * Work around Cortex-A73 errata 858921, where there is a
> + * one-cycle window where the read might return the old value
> + * for the low 32 bits and the new value for the high 32 bits
> + * upon roll-over of the low 32 bits.
> + */
> + __asm volatile("isb" : : : "memory");
> + __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val0));
> + __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val1));
> + return ((val0 ^ val1) & 0x100000000ULL) ? val0 : val1;
> +}
> +
> +int
> +tc_get_timecount(struct timekeep *tk, uint64_t *tc)
> +{
> + if (tc == NULL || tk->tk_user != 1)
> + return -1;
> +
> + *tc = readcnt64();
> + return 0;
> +}
> +
> +int (*const _tc_get_timecount)(struct timekeep *, uint64_t *)
> + = tc_get_timecount;
> --- ./sys/arch/arm64/arm64/machdep.c.orig Thu Jun 11 17:46:54 2020
> +++ ./sys/arch/arm64/arm64/machdep.c Thu Jun 11 17:46:59 2020
> @@ -91,7 +91,7 @@
> char machine[] = MACHINE; /* from <machine/param.h> */
>
> /* timekeep number of user accesible clocks */
> -int tk_nclocks = 0;
> +int tk_nclocks = 1;
>
> int safepri = 0;
>
> --- ./sys/arch/arm64/dev/agtimer.c.orig Thu Jun 11 17:47:23 2020
> +++ ./sys/arch/arm64/dev/agtimer.c Thu Jun 11 17:47:27 2020
> @@ -43,7 +43,7 @@
> u_int agtimer_get_timecount(struct timecounter *);
>
> static struct timecounter agtimer_timecounter = {
> - agtimer_get_timecount, NULL, 0x7fffffff, 0, "agtimer", 0, NULL, 0
> + agtimer_get_timecount, NULL, 0x7fffffff, 0, "agtimer", 0, NULL, 1
> };
>
> struct agtimer_pcpu_softc {
> --
> Christian "naddy" Weisgerber na...@mips.inka.de
>
>
