Add the ability to perform self-tests on the first few invocations of
NestedInterruptRestoreTPL(), to verify that:

- the timer interrupt handler correctly rearms the timer interrupt
  before calling NestedInterruptRestoreTPL(), and

- the architecture-specific DisableInterruptsOnIret() implementation
  correctly causes interrupts to be disabled after the IRET or
  equivalent instruction.

Any test failure will be treated as fatal and will halt the system
with an appropriate diagnostic message.

Each test invocation adds approximately one timer tick of delay to the
overall system startup time.

Only one test is performed by default (to avoid unnecessary system
startup delay).  The number of tests performed can be controlled via
PcdNestedInterruptNumberOfSelfTests at build time.

Signed-off-by: Michael Brown <mc...@ipxe.org>
---
 MdeModulePkg/MdeModulePkg.dec                 |   4 +
 .../NestedInterruptTplLib.inf                 |   3 +
 .../Include/Library/NestedInterruptTplLib.h   |   4 +
 .../Library/NestedInterruptTplLib/Tpl.c       | 129 ++++++++++++++++++
 4 files changed, 140 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index d6fb729af5a7..efd32c197b18 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1142,6 +1142,10 @@ [PcdsFixedAtBuild]
   # @Prompt Enable large address image loading.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdImageLargeAddressLoad|TRUE|BOOLEAN|0x30001059
 
+  ## Number of NestedInterruptTplLib self-tests to perform at startup.
+  # @Prompt Number of NestedInterruptTplLib self-tests.
+  
gEfiMdeModulePkgTokenSpaceGuid.PcdNestedInterruptNumberOfSelfTests|1|UINT32|0x30001060
+
 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Dynamic type PCD can be registered callback function for Pcd setting 
action.
   #  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of 
callback function
diff --git 
a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf 
b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
index f130d6dcd213..e67d899b9446 100644
--- a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
+++ b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
@@ -34,3 +34,6 @@ [LibraryClasses]
 
 [Depex.common.DXE_DRIVER]
   TRUE
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNestedInterruptNumberOfSelfTests
diff --git a/MdeModulePkg/Include/Library/NestedInterruptTplLib.h 
b/MdeModulePkg/Include/Library/NestedInterruptTplLib.h
index 0ead6e4b346a..7dd934577e99 100644
--- a/MdeModulePkg/Include/Library/NestedInterruptTplLib.h
+++ b/MdeModulePkg/Include/Library/NestedInterruptTplLib.h
@@ -32,6 +32,10 @@ typedef struct {
   /// interrupt handler.
   ///
   BOOLEAN    DeferredRestoreTPL;
+  ///
+  /// Number of self-tests performed.
+  ///
+  UINTN      SelfTestCount;
 } NESTED_INTERRUPT_STATE;
 
 /**
diff --git a/MdeModulePkg/Library/NestedInterruptTplLib/Tpl.c 
b/MdeModulePkg/Library/NestedInterruptTplLib/Tpl.c
index 99af553ab189..dfe22331204f 100644
--- a/MdeModulePkg/Library/NestedInterruptTplLib/Tpl.c
+++ b/MdeModulePkg/Library/NestedInterruptTplLib/Tpl.c
@@ -17,6 +17,18 @@
 
 #include "Iret.h"
 
+//
+// Number of self-tests to perform.
+//
+#define NUMBER_OF_SELF_TESTS \
+  (FixedPcdGet32 (PcdNestedInterruptNumberOfSelfTests))
+
+STATIC
+VOID
+NestedInterruptSelfTest (
+  IN NESTED_INTERRUPT_STATE  *IsrState
+  );
+
 /**
   Raise the task priority level to TPL_HIGH_LEVEL.
 
@@ -211,6 +223,16 @@ NestedInterruptRestoreTPL (
     //
     DisableInterrupts ();
 
+    ///
+    /// Perform a limited number of self-tests on the first few
+    /// invocations.
+    ///
+    if ((IsrState->DeferredRestoreTPL == FALSE) &&
+       (IsrState->SelfTestCount < NUMBER_OF_SELF_TESTS)) {
+      IsrState->SelfTestCount++;
+      NestedInterruptSelfTest (IsrState);
+    }
+
     //
     // DEFERRAL RETURN POINT
     //
@@ -248,3 +270,110 @@ NestedInterruptRestoreTPL (
     }
   }
 }
+
+/**
+  Perform internal self-test.
+
+  Induce a delay to force a nested timer interrupt to take place, and
+  verify that the nested interrupt behaves as required.
+
+  @param IsrState              A pointer to the state shared between all
+                               invocations of the nested interrupt handler.
+**/
+VOID
+NestedInterruptSelfTest (
+  IN NESTED_INTERRUPT_STATE  *IsrState
+  )
+{
+  UINTN SelfTestCount;
+  UINTN TimeOut;
+
+  //
+  // Record number of this self-test for debug messages.
+  //
+  SelfTestCount = IsrState->SelfTestCount;
+
+  //
+  // Re-enable interrupts and stall for up to one second to induce at
+  // least one more timer interrupt.
+  //
+  // This mimics the effect of an interrupt having occurred precisely
+  // at the end of our call to RestoreTPL(), with interrupts having
+  // been re-enabled by RestoreTPL() and with the interrupt happening
+  // to occur after the TPL has already been lowered back down to
+  // InterruptedTPL.  (This is the scenario that can lead to stack
+  // exhaustion, as described above.)
+  //
+  ASSERT (GetInterruptState () == FALSE);
+  ASSERT (IsrState->DeferredRestoreTPL == FALSE);
+  EnableInterrupts();
+  for (TimeOut = 0; TimeOut < 1000; TimeOut++) {
+    //
+    // Stall for 1ms
+    //
+    gBS->Stall (1000);
+
+    //
+    // If we observe that interrupts have been spontaneously disabled,
+    // then this must be because the induced interrupt handler's call
+    // to NestedInterruptRestoreTPL() correctly chose to defer the
+    // RestoreTPL() call to the outer handler (i.e. to us).
+    //
+    if (GetInterruptState() == FALSE) {
+      ASSERT (IsrState->DeferredRestoreTPL == TRUE);
+      DEBUG ((
+        DEBUG_INFO,
+        "Nested interrupt self-test %u/%u passed\n",
+        SelfTestCount,
+        NUMBER_OF_SELF_TESTS
+       ));
+      return;
+    }
+  }
+
+  //
+  // The test has failed and we will halt the system.  Disable
+  // interrupts now so that any test-induced interrupt storm does not
+  // prevent the fatal error messages from being displayed correctly.
+  //
+  DisableInterrupts();
+
+  //
+  // If we observe that DeferredRestoreTPL is TRUE then this indicates
+  // that an interrupt did occur and NestedInterruptRestoreTPL() chose
+  // to defer the RestoreTPL() call to the outer handler, but that
+  // DisableInterruptsOnIret() failed to cause interrupts to be
+  // disabled after the IRET or equivalent instruction.
+  //
+  // This error represents a bug in the architecture-specific
+  // implementation of DisableInterruptsOnIret().
+  //
+  if (IsrState->DeferredRestoreTPL == TRUE) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Nested interrupt self-test %u/%u failed: interrupts still enabled\n",
+      SelfTestCount,
+      NUMBER_OF_SELF_TESTS
+      ));
+    ASSERT (FALSE);
+  }
+
+  //
+  // If no timer interrupt occurred then this indicates that the timer
+  // interrupt handler failed to rearm the timer before calling
+  // NestedInterruptRestoreTPL().  This would prevent nested
+  // interrupts from occurring at all, which would break
+  // e.g. callbacks at TPL_CALLBACK that themselves wait on further
+  // timer events.
+  //
+  // This error represents a bug in the platform-specific timer
+  // interrupt handler.
+  //
+  DEBUG ((
+    DEBUG_ERROR,
+    "Nested interrupt self-test %u/%u failed: no nested interrupt\n",
+    SelfTestCount,
+    NUMBER_OF_SELF_TESTS
+    ));
+  ASSERT (FALSE);
+}
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114210): https://edk2.groups.io/g/devel/message/114210
Mute This Topic: https://groups.io/mt/103911608/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to