On Wed, Jul 08, 2020 at 01:35:30AM +0200, Mark Kettenis wrote:
> > Date: Tue, 7 Jul 2020 23:46:00 +0200 (CEST)
> > From: Mark Kettenis <mark.kette...@xs4all.nl>
> >
> > > Date: Tue, 7 Jul 2020 23:13:06 +0200
> > > From: Christian Weisgerber <na...@mips.inka.de>
> > >
> > > Mark Kettenis:
> > >
> > > > > --- lib/libc/arch/alpha/gen/usertc.c 6 Jul 2020 13:33:05 -0000
> > > > > 1.1
> > > > > +++ lib/libc/arch/alpha/gen/usertc.c 7 Jul 2020 20:40:37 -0000
> > >
> > > > > +int
> > > > > +tc_get_timecount(struct timekeep *tk, u_int *tc)
> > > >
> > > > Need to make this function static to avoid namespace pollution.
> > >
> > > Then this needs to be fixed in amd64/gen/usertc.c pronto, before
> > > everybody copies it from there.
> >
> > Yeah, maybe people should hold off a bit. I have a few cleanups to
> > that code so it is best not to copy what is there now.
>
> So here is a diff that cleans things up and implements sparc64
> support. Showing both together since some of the amd64 changes were
> inspired by the sparc64 code.
>
> * TC_LAST can be removed; it really doesn't serve any purpose
>
> * the functions in usertc.c need to be static to avoid namespace
> pollution in libc.a.
>
> * I use a switch statement to simplify tc_get_timecount().
> Architectures with only one supported timecounter could use an if
> statement like naddy@ did for alpha. That would be fine with me as
> well.
>
> ok?
>
> Index: sys/arch/sparc64/sparc64/clock.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sparc64/sparc64/clock.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 clock.c
> --- sys/arch/sparc64/sparc64/clock.c 6 Jul 2020 13:33:08 -0000 1.62
> +++ sys/arch/sparc64/sparc64/clock.c 7 Jul 2020 23:29:48 -0000
> @@ -109,13 +109,14 @@ struct cfdriver clock_cd = {
> u_int tick_get_timecount(struct timecounter *);
>
> struct timecounter tick_timecounter = {
> - tick_get_timecount, NULL, ~0u, 0, "tick", 0, NULL, 0
> + tick_get_timecount, NULL, ~0u, 0, "tick", 0, NULL, TC_TICK
> };
>
> u_int sys_tick_get_timecount(struct timecounter *);
>
> struct timecounter sys_tick_timecounter = {
> - sys_tick_get_timecount, NULL, ~0u, 0, "sys_tick", 1000, NULL, 0
> + sys_tick_get_timecount, NULL, ~0u, 0, "sys_tick", 1000, NULL,
> + TC_SYS_TICK
> };
>
> /*
> @@ -940,7 +941,7 @@ tick_get_timecount(struct timecounter *t
> {
> u_int64_t tick;
>
> - __asm volatile("rd %%tick, %0" : "=r" (tick) :);
> + __asm volatile("rd %%tick, %0" : "=r" (tick));
>
> return (tick & ~0u);
> }
> @@ -950,7 +951,7 @@ sys_tick_get_timecount(struct timecounte
> {
> u_int64_t tick;
>
> - __asm volatile("rd %%sys_tick, %0" : "=r" (tick) :);
> + __asm volatile("rd %%sys_tick, %0" : "=r" (tick));
>
> return (tick & ~0u);
> }
The only thing that gives me pause is the inline assembly.
I know it isn't a lot of assembly, but would it be a layer violation to
put, e.g.
uint64_t
rd_systick(void)
{
uint64_t val;
__asm volatile("rd %%sys_tick, %0" : "=r" (val));
return val;
}
into something like sys/arch/sparc64/include/cpufunc.h and use it
in both the kernel and libc?
In general isn't it nicer to write it once?
We could do the same thing on other platforms too for select
instructions that are useful in both libc and the kernel.
