[edk2-devel] [PATCH 6/6] UefiCpuPkg/LocalApicTimerDxe: Passing fixed timer period is not a bug.

["Ni, Ray" <ray.ni@intel.com>]

The patch adds detailed comments to explain that passing fixed timer period
to mTimerNotifyFunction() is not a bug.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
index 14ab8fd03d..51c5d39b29 100644
--- a/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
+++ b/UefiCpuPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
@@ -66,7 +66,17 @@ TimerInterruptHandler (
 
   if (mTimerNotifyFunction != NULL) {
     //
-    // @bug : This does not handle missed timer interrupts
+    // When mTimerPeriod is 10ms but 1st call of mTimerNotifyFunction() takes 1 hour, the timer
+    // interrupt will be queued and TimerInterruptHandler() will be called immediately after the
+    // interrupt is re-enabled with gBS->RestoreTPL(). The most precise implementation should pass
+    // 1 hour as the time period to 2nd call of mTimerNotifyFunction().
+    // Always passing 10ms to mTimerNotifyFunction() might cause some timer event callbacks to be
+    // invoked later than expected.
+    //
+    // But in reality, DxeCore implements the mTimerNotifyFunction() as a simple function which
+    // runs at TPL_HIGH_LEVEL. Considering almost all UEFI services require to run at TPL lower
+    // than TPL_HIGH_LEVEL, the mTimerNotifyFunction() does not have much heavy work to do. It
+    // should not take longer.
     //
     mTimerNotifyFunction (mTimerPeriod);
   }
@@ -361,9 +371,6 @@ TimerDriverGenerateSoftInterrupt (
     OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
 
     if (mTimerNotifyFunction != NULL) {
-      //
-      // @bug : This does not handle missed timer interrupts
-      //
       mTimerNotifyFunction (mTimerPeriod);
     }
 
-- 
2.39.1.windows.1



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