Hi Paul,
I'm just a bystander that likes to read diffs. I can't speak about how
the diff fits in the kernel, but I can do some basic checks of
correctness.
As a matter of syntax, there are quite some places with functions
without parameters defined as `f()` instead of `f(void)`.
Paul Irofti <p...@irofti.net> wrote:
> diff --git lib/libc/arch/amd64/gen/usertc.c lib/libc/arch/amd64/gen/usertc.c
> new file mode 100644
> index 00000000000..ee44d61de4b
> --- /dev/null
> +++ lib/libc/arch/amd64/gen/usertc.c
> @@ -0,0 +1,53 @@
> +/* $OpenBSD$ */
> +/*
> + * Copyright (c) 2020 Paul Irofti <p...@irofti.net>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/types.h>
> +#include <sys/timetc.h>
> +
> +static uint
> +rdtsc()
> +{
> + uint32_t hi, lo;
> + asm volatile("rdtsc" : "=a"(lo), "=d"(hi));
> + return ((uint64_t)lo)|(((uint64_t)hi)<<32);
> +}
> +
> +static uint
> +acpihpet()
> +{
> + return rdtsc(); /* JUST TO COMPILE */
> +}
> +
> +static uint (*const get_tc[])(void) =
> +{
> + [TC_TSC] = rdtsc,
> + [TC_HPET] = acpihpet,
> +};
> +
> +int
> +tc_get_timecount(struct timekeep *tk, uint *tc)
> +{
> + int tk_user = tk->tk_user;
> +
> + if (tc == NULL || tk_user < 1 || tk_user > TC_LAST)
Shouldn't you check for >= TC_LAST in here? Otherwise you'll be reading
and invoking dragons in the following lines.
*Unless*, the semantic meaning of TC_LAST is to indicate the last valid
index of get_tc[]. In that case, TC_LAST is defined to 3 in amd64,
instead of 2.
> + return -1;
> +
> + *tc = (*get_tc[tk_user])();
> + return 0;
> +}
> +int (*const _tc_get_timecount)(struct timekeep *tk, uint *tc)
> + = tc_get_timecount;
> diff --git sys/arch/amd64/include/timetc.h sys/arch/amd64/include/timetc.h
> new file mode 100644
> index 00000000000..72dfc969a76
> --- /dev/null
> +++ sys/arch/amd64/include/timetc.h
> @@ -0,0 +1,25 @@
> +/* $OpenBSD$ */
> +/*
> + * Copyright (c) 2020 Paul Irofti <p...@irofti.net>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#ifndef _MACHINE_TIMETC_H_
> +#define _MACHINE_TIMETC_H_
> +
> +#define TC_TSC 1
> +#define TC_HPET 2
> +#define TC_LAST 3
> +
> +#endif /* _MACHINE_TIMETC_H_ */
