On 2020-05-31 20:46, Mark Kettenis wrote:
From: Paul Irofti <p...@irofti.net>
Date: Sun, 31 May 2020 19:12:54 +0300

On 2020-05-31 18:25, Theo de Raadt wrote:
Mark Kettenis <mark.kette...@xs4all.nl> wrote:

I changed __amd64 to __amd64__ because I didn't find __powerpc.  I'm
not sure, but one might move the list of arches to dlfcn/Makefile.inc
and do -DTIMEKEEP, like how thread/Makefile.inc does -DFUTEX.  One
might drop the tc_get_timecount function pointer and just always call
the function #ifdef TIMEKEEP.

Yes, we prefer the __xxx__ variants in OpenBSD code; thanks for
catching that.  The benefit of the TIMEKEEP define would be that we
can eliminate the fallback code completely on architectures that don't
implement this functionality.

...

Yeah, I just followed the dlfcn/dlfcn_stubs.c example from libc. Which I
see now it is commented out...

--- lib/libc/dlfcn/init.c.before        Sat May 30 23:26:35 2020
+++ lib/libc/dlfcn/init.c       Sat May 30 18:00:45 2020
@@ -70,7 +70,7 @@
/* provide definitions for these */
   const dl_cb *_dl_cb __relro = NULL;
-#if defined(__amd64)
+#if defined(__amd64__) || defined(__powerpc__)
   uint64_t (*const tc_get_timecount)(void) = tc_get_timecount_md;
   #else
   uint64_t (*const tc_get_timecount)(void) = NULL;

1) I think adding _md to the name is superflous.  There will never
     be a MI version, so tc_get_timecount() is enough.

What about pvclock(4)?

What about it?  Seems to me what you're really thinking of here is how
to support more than just one timecounter for a specific architecture.
Your function pointer is not really going to help in that case.
You'll need to dispatch to the right function based on some sort of
machine-specific clock ID.

Oh and BTW, I don't think you're ever going to support pvclock(4).
Take a look at the code and think how you would do all that magic in
userland...

2) I hope we can get away from #ifdef __ arch__.
     Maybe this can be split into architectures which
        a) have a function called tc_get_timecount()
     or
        b) tc_get_timecount is #define'd to NULL, though I don't
           know which MD include file to do that in

If we go with something like this or with something like -DTIMEKEEP, how
do we handle the different PROTO_WRAP vs. PROTO_NORMAL declarations?
Split them in MD headers? But then we end up in the same place. Sort of.

Forget about all that for a moment.  Here is an alternative suggestion:

On sparc64 we need to support both tick_timecounter and
sys_tick_timecounter.  So we need some sort of clockid value to
distnguish between those two.  I already suggested to use the tc_user
field of the timecounter for that.  0 means that a timecounter is not
usable in userland, a (small) positive integer means a specific
timecounter type.  The code in libc will need to know whether a
particular timecounter type can be supported.  My proposal would be to
implement a function *on all architecture* that takes the clockid as
an argument and returns a pointer to the function that implements
support for that timecounter.  On architectures without support, ir
when called with a clockid that isn't supported, that function would
simply return NULL.



What if we declare in libc/arch/*/SYS.h tc_get_timecount to either be NULL or the prototype of a function. (I know SYS.c is a bit of a stretch, if not we can create a separate header usertc.h?) And then we use tc_user to be an ID for architectures such as sparc64 that have more than one clock and inside libc/*/gen/usertc.c we check which is it and call a local static function based on it?

Would that be OK?

Reply via email to