Mark Kettenis:
> Nevertheless, here is a different take on the problem. Since the
> timecounter only uses the low 32 bits we don't need the double read.
> This version also changes the timecounter mask from 0x7fffffff to
> 0xffffffff. That must be ok, since the counter has 64 bits and we are
> already using 0xffffffff as a mask on amd64 and sparc64.
>
> ok?
Yes, but don't forget the part in sys/arch/arm64/include/timetc.h.
Also, if I may ask, ...
> Index: sys/arch/arm64/dev/agtimer.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm64/dev/agtimer.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 agtimer.c
> --- sys/arch/arm64/dev/agtimer.c 11 Jul 2020 15:22:44 -0000 1.14
> +++ sys/arch/arm64/dev/agtimer.c 11 Jul 2020 18:35:12 -0000
> @@ -43,7 +43,8 @@ int32_t agtimer_frequency = TIMER_FREQUE
> 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, 0xffffffff, 0, "agtimer", 0, NULL,
> + TC_AGTIMER
> };
>
> struct agtimer_pcpu_softc {
> @@ -191,7 +192,15 @@ agtimer_attach(struct device *parent, st
> u_int
> agtimer_get_timecount(struct timecounter *tc)
> {
> - return agtimer_readcnt64();
> + uint64_t val;
> +
> + /*
> + * No need to work around Cortex-A73 errata 858921 since we
> + * only look at the low 32 bits here.
> + */
> + __asm volatile("isb" ::: "memory");
> + __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val));
> + return (val & 0xffffffff);
Is there any point, stylistically I mean, to explicitly masking
this over the truncation implicit in the types? I'm pretty sure
we do, say, "intvar = int64var" all the time.
> }
>
> int
> Index: lib/libc/arch/aarch64/gen/usertc.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/arch/aarch64/gen/usertc.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 usertc.c
> --- lib/libc/arch/aarch64/gen/usertc.c 6 Jul 2020 13:33:05 -0000
> 1.1
> +++ lib/libc/arch/aarch64/gen/usertc.c 11 Jul 2020 18:35:12 -0000
> @@ -1,6 +1,6 @@
> -/* $OpenBSD: usertc.c,v 1.1 2020/07/06 13:33:05 pirofti Exp $ */
> +/* $OpenBSD$ */
> /*
> - * Copyright (c) 2020 Paul Irofti <p...@irofti.net>
> + * Copyright (c) 2020 Mark Kettenis <kette...@openbsd.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 +18,30 @@
> #include <sys/types.h>
> #include <sys/timetc.h>
>
> -int (*const _tc_get_timecount)(struct timekeep *, u_int *) = NULL;
> +static inline u_int
> +agtimer_get_timecount(struct timecounter *tc)
> +{
> + uint64_t val;
> +
> + /*
> + * No need to work around Cortex-A73 errata 858921 since we
> + * only look at the low 32 bits here.
> + */
> + __asm volatile("isb" ::: "memory");
> + __asm volatile("mrs %x0, CNTVCT_EL0" : "=r" (val));
> + return (val & 0xffffffff);
> +}
> +
> +static int
> +tc_get_timecount(struct timekeep *tk, u_int *tc)
> +{
> + switch (tk->tk_user) {
> + case TC_AGTIMER:
> + *tc = agtimer_get_timecount(NULL);
> + return 0;
> + }
> +
> + return -1;
> +}
> +
> +int (*const _tc_get_timecount)(struct timekeep *, u_int *) =
> tc_get_timecount;
>
--
Christian "naddy" Weisgerber na...@mips.inka.de
