Sender: rich AT phekda DOT freeserve DOT co DOT uk
Message-ID: <3EAE64CC.DBA5C7D7@phekda.freeserve.co.uk>
Date: Tue, 29 Apr 2003 12:41:00 +0100
From: Richard Dawe <rich AT phekda DOT freeserve DOT co DOT uk>
X-Mailer: Mozilla 4.77 [en] (X11; U; Linux 2.2.23 i586)
X-Accept-Language: de,fr
MIME-Version: 1.0
To: djgpp-workers AT delorie DOT com
Subject: Re: uclock proposed patch
References: <10304290440 DOT AA26174 AT clio DOT rice DOT edu>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Reply-To: djgpp-workers AT delorie DOT com

Hello.

Charles Sandmann wrote:
> For Windows 2000 and XP; I'll try a test on NT4 and on a 486 running NT 3.1
> if I get a chance.  Autocalibrates as needed to always keep the tic
> counter aligned.  This might cause some jitter due to the inaccuracy of
> the NT tic counter, but since it's not a real time OS and multitasking
> who knows what really happened.  Installs signal handler, should never
> been seen in real life (no one is running Windows NT 3.5 or earlier
> which were the last versions to support 486s which didn't have rdtsc).
> If they are, it falls back to tic accuracy.
> 
> Comments?
> 
> --- uclock.c_   Tue Dec 11 21:28:06 2001
> +++ uclock.c    Mon Apr 28 23:25:18 2003
> @@ -11,4 +11,21 @@
>  #include <dpmi.h>
>  #include <libc/bss.h>
> +#include <dos.h>
> +#include <signal.h>
> +#include <setjmp.h>
> +#include <sys/exceptn.h>
> +
> +/* Catch rdtsc exception and always return 0LL */
> +static void catch_rdtsc(int val)
> +{
> +  short *eip = (short *)__djgpp_exception_state->__eip;
> +  if(*eip == 0x310f) {

Presumably this is the opcode for the RDTSC instruction?

> +    __djgpp_exception_state->__eip += 2;
> +    __djgpp_exception_state->__edx = 0;
> +    longjmp(__djgpp_exception_state, 0);
> +  }
> +  return;
> +}
> +
> 
>  static int uclock_bss = -1;
> @@ -30,4 +47,32 @@
>    uclock_t rv;
> 
> +  _farsetsel(_dos_ds);
> +
> +  if(_os_trueversion == 0x532) {       /* Windows NT, 2000, XP */
> +    static double multiplier;
> +    static unsigned long btics;
> +    uclock_t rval;
> +
> +    if (uclock_bss != __bss_count) {
> +      signal(SIGILL, catch_rdtsc);

Perhaps we could try executing _rdtsc after calling signal, to see if we
actually need the signal handler? If not, restore the previous handler.

I don't like the idea of installing the signal handler, if we don't need it. I
suppose this would have the disadvantage that the code will behave differently
on systems with and without RDTSC. That may be a problem, if no-one tests it
on a system without RDTSC. I certainly don't have any systems without RDTSC.
So maybe it would be best to always install it.

What happens if we get SIGILL, but the instruction was not RDTSC?

I agree with Gisle that we should save and chain any previous SIGILL handler
here.

Do you plan to update the documentation? If so, maybe we should mention the
fact we add a handler for SIGILL. A user program will need to chain SIGILL, if
they install a SIGILL handler after calling uclock.

> +      tics = _farnspeekl(0x46c);
> +      while ( (btics = _farnspeekl(0x46c)) == tics);
> +      base = _rdtsc();

Where is th the _rdtsc function/macro defined?

> +      if (base == 0) base = -1LL;
> +      while ( (tics = _farnspeekl(0x46c)) == btics);
> +      if (tics < btics) tics = btics + 1;              /* Midnight */
> +      multiplier = ((tics - btics)*65536) / (double)(_rdtsc() - base);
> +      uclock_bss = __bss_count;
> +    }
> +    rval = (_rdtsc() - base) * multiplier;
> +    tics = _farnspeekl(0x46c) - btics;
> +    while (tics <= 0) tics += 0x1800b0;                        /* Midnight */
> +    if( (unsigned long)(rval >> 16) != tics) {         /* Recalibrate */
> +      rval = ((uclock_t)tics) << 16;
> +      multiplier = (tics*65536) / (double)(_rdtsc() - base);
> +    }
> +    return rval;
> +  }
> +
>    if (uclock_bss != __bss_count)
>    {
> @@ -52,5 +97,4 @@
>         if the timer worked in mode 3.  So we simply wait for one clock
>         tick when we run on Windows.  */
> -    _farsetsel(_dos_ds);
>      otics = _farnspeekl(0x46c);
>      do {
> @@ -63,9 +107,9 @@
>    /* Make sure the numbers we get are consistent */
>    do {
> -    otics = _farpeekl(_dos_ds, 0x46c);
> +    otics = _farnspeekl(0x46c);
>      outportb(0x43, 0x00);
>      lsb = inportb(0x40);
>      msb = inportb(0x40);
> -    tics = _farpeekl(_dos_ds, 0x46c);
> +    tics = _farnspeekl(0x46c);
>    } while (otics != tics);
> 

Apart from that, I'm happy with the patch!

Thanks, bye, Rich =]

-- 
Richard Dawe [ http://www.phekda.freeserve.co.uk/richdawe/ ]