[edk2-devel] [Patch 1/1] PcAtChipsetPkg/HpetTimerDxe: Fix nested interrupt time accuracy

[edk2-devel] [Patch 1/1] PcAtChipsetPkg/HpetTimerDxe: Fix nested interrupt time accuracy

[Michael D Kinney]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4659

When HPET timer is used as the timer interrupt and nested
interrupts into the HPET timer interrupt handler occur, the
elapsed time passed into the DXE Core is sometime too large
and this causes the DXE Core internal system time to run too
fast. Fix the logic so the previous main counter value stored
in the module global variable mPreviousMainCounter is always
captured before the timer notification function is called.

Without this change, mPreviousMainCounter is updated after
the timer notification function is called and when nesting
occurs, it updates with the value from the first level of
nesting which is further back in time than the interrupt from
the deepest level of nesting.  This causes the next two timer
interrupts to compute a TimerPeriod that is twice the actual
time period since the last interrupt and this causes the DXE
Core internal time to run faster than expected.

Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c b/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
index 427572c2ff6d..e74777fbc6f4 100644
--- a/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
+++ b/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
@@ -387,11 +387,18 @@ TimerInterruptHandler (
                     100000000
                     );
 
+    //
+    // Save main counter value before calling notification function
+    // that may enable interrupts and allow interrupt nesting.
+    //
+    mPreviousMainCounter = MainCounter;
+
     //
     // Call registered notification function passing in the time since the last
     // interrupt in 100 ns units.
     //
     mTimerNotifyFunction (TimerPeriod);
+    return;
   }
 
   //
-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114648): https://edk2.groups.io/g/devel/message/114648
Mute This Topic: https://groups.io/mt/103992760/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch 1/1] PcAtChipsetPkg/HpetTimerDxe: Fix nested interrupt time accuracy

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Saturday, January 27, 2024 3:56 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: [Patch 1/1] PcAtChipsetPkg/HpetTimerDxe: Fix nested interrupt time
> accuracy
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4659
> 
> When HPET timer is used as the timer interrupt and nested
> interrupts into the HPET timer interrupt handler occur, the
> elapsed time passed into the DXE Core is sometime too large
> and this causes the DXE Core internal system time to run too
> fast. Fix the logic so the previous main counter value stored
> in the module global variable mPreviousMainCounter is always
> captured before the timer notification function is called.
> 
> Without this change, mPreviousMainCounter is updated after
> the timer notification function is called and when nesting
> occurs, it updates with the value from the first level of
> nesting which is further back in time than the interrupt from
> the deepest level of nesting.  This causes the next two timer
> interrupts to compute a TimerPeriod that is twice the actual
> time period since the last interrupt and this causes the DXE
> Core internal time to run faster than expected.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
> b/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
> index 427572c2ff6d..e74777fbc6f4 100644
> --- a/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
> +++ b/PcAtChipsetPkg/HpetTimerDxe/HpetTimer.c
> @@ -387,11 +387,18 @@ TimerInterruptHandler (
>                      100000000
>                      );
> 
> +    //
> +    // Save main counter value before calling notification function
> +    // that may enable interrupts and allow interrupt nesting.
> +    //
> +    mPreviousMainCounter = MainCounter;
> +
>      //
>      // Call registered notification function passing in the time since the last
>      // interrupt in 100 ns units.
>      //
>      mTimerNotifyFunction (TimerPeriod);
> +    return;
>    }
> 
>    //
> --
> 2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114649): https://edk2.groups.io/g/devel/message/114649
Mute This Topic: https://groups.io/mt/103992760/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-