> Date: Sun, 12 Jul 2020 00:06:21 +0200
> From: Christian Weisgerber <na...@mips.inka.de>
>
> 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.
I tried to make it explicit that the result is truncated to 32 bits.
But indeed this isn't necessary. The compiler knows and optimizes it
away completely. I can leave it out if you think that's better.
>
> > }
> >
> > 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
>
