Re: [ros-dev] [ros-diffs] [tkreuzer] 53496: [HAL] We cannot make any assumptions about the latency whith which the timer interrupt fires after a rollover, since VBox (other VMs probably as well) doesn't always meet this. Ad...

[Alex Ionescu]

Funny how Windows works without these hacks.

Let's hide more bugs :D

Best regards,
Alex Ionescu


On Tue, Aug 30, 2011 at 12:01 PM, <tkreuzer(a)svn.reactos.org> wrote:

>   Author: tkreuzer
>  Date: Tue Aug 30 12:01:01 2011
>  New Revision: 53496
>
>  URL: http://svn.reactos.org/svn/reactos?rev=53496&view=rev
>  Log:
>  [HAL]
>  We cannot make any assumptions about the latency whith which the timer
>  interrupt fires after a rollover, since VBox (other VMs probably as well)
>  doesn't always meet this. Add another check to KeQueryPerformanceCounter
>  that gracefully handles missing interrupts. Also raise to DISPATCH_LEVEL,
>  since the function is not reentrant.
>
>  Modified:
>     trunk/reactos/hal/halx86/generic/timer.c
>
>  Modified: trunk/reactos/hal/halx86/generic/timer.c
>  URL:
>
> http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/generic/timer.c…
>
>  ==============================================================================
>  --- trunk/reactos/hal/halx86/generic/timer.c [iso-8859-1] (original)
>  +++ trunk/reactos/hal/halx86/generic/timer.c [iso-8859-1] Tue Aug 30
>  12:01:01 2011
>  @@ -253,6 +253,7 @@
>   {
>      LARGE_INTEGER CurrentPerfCounter;
>      ULONG CounterValue, ClockDelta;
>  +    KIRQL OldIrql;
>
>      /* If caller wants performance frequency, return hardcoded value */
>      if (PerformanceFrequency) PerformanceFrequency->QuadPart =
>  PIT_FREQUENCY;
>  @@ -262,6 +263,10 @@
>
>      /* Check if interrupts are disabled */
>      if(!(__readeflags() & EFLAGS_INTERRUPT_MASK)) return HalpPerfCounter;
>  +
>  +    /* Raise irql to DISPATCH_LEVEL */
>  +    OldIrql = KeGetCurrentIrql();
>  +    if (OldIrql < DISPATCH_LEVEL) KfRaiseIrql(DISPATCH_LEVEL);
>
>      do
>      {
>  @@ -287,13 +292,21 @@
>      /* Add the clock delta */
>      CurrentPerfCounter.QuadPart += ClockDelta;
>
>  -    /* This must be true unless HalpPerfCounter has changed sign,
>  -       which takes approximately 245,118 years */
>  -    ASSERT(CurrentPerfCounter.QuadPart >= HalpLastPerfCounter.QuadPart);
>  +    /* Check if the value is smaller then before, this means, we somehow
>  +       missed an interrupt. This is a sign that the timer interrupt
>  +       is very inaccurate. Probably a virtual machine. */
>  +    if (CurrentPerfCounter.QuadPart < HalpLastPerfCounter.QuadPart)
>  +    {
>  +        /* We missed an interrupt. Assume we will receive it later */
>  +        CurrentPerfCounter.QuadPart += HalpCurrentRollOver;
>  +    }
>
>      /* Update the last counter value */
>      HalpLastPerfCounter = CurrentPerfCounter;
>
>  +    /* Restore previous irql */
>  +    if (OldIrql < DISPATCH_LEVEL) KfLowerIrql(OldIrql);
>  +
>      /* Return the result */
>      return CurrentPerfCounter;
>   }
>
>
>

[Aleksey Bragin]

More like :-{ for me...

From: Alex Ionescu 
Sent: Tuesday, August 30, 2011 10:52 PM
To: ros-dev(a)reactos.org 
Cc: ros-diffs(a)reactos.org 
Subject: Re: [ros-dev] [ros-diffs] [tkreuzer] 53496: [HAL] We cannot make any assumptions
about the latency whith which the timer interrupt fires after a rollover, since VBox
(other VMs probably as well) doesn't always meet this. Ad...

Funny how Windows works without these hacks. 

Let's hide more bugs :D

Best regards,
Alex Ionescu



On Tue, Aug 30, 2011 at 12:01 PM, <tkreuzer(a)svn.reactos.org> wrote:

  Author: tkreuzer
  Date: Tue Aug 30 12:01:01 2011
  New Revision: 53496

  URL: http://svn.reactos.org/svn/reactos?rev=53496&view=rev
  Log:
  [HAL]
  We cannot make any assumptions about the latency whith which the timer interrupt fires
after a rollover, since VBox (other VMs probably as well) doesn't always meet this.
Add another check to KeQueryPerformanceCounter that gracefully handles missing interrupts.
Also raise to DISPATCH_LEVEL, since the function is not reentrant.