From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id CE1AD7803D0 for ; Tue, 23 Jan 2024 15:31:21 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=pbBI1+Qv7xem21vdu0jQ2zBgSInQsbFiBHjwqyzkFdY=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:MIME-Version:Feedback-ID:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1706023880; v=1; b=Dwdpv6OWxrhG5bRinuaQinPPT2J99fLxKL3JoY/Z5+bEGiJZjHiElfFCinrHnRJdOLVEeW1S 7UVAO7RROh3kmPSyhUDghsuQCJ3zcM5r3yI+jpH/+HxDhshp60llN/zGjVJobyYUZeSVetzvhc5 +3XBlcGYSLmDM7lhL3uqWFKM= X-Received: by 127.0.0.2 with SMTP id wZXjYY7687511xX4pvIw577R; Tue, 23 Jan 2024 07:31:20 -0800 X-Received: from a7-20.smtp-out.eu-west-1.amazonses.com (a7-20.smtp-out.eu-west-1.amazonses.com [54.240.7.20]) by mx.groups.io with SMTP id smtpd.web11.15650.1706023871929543121 for ; Tue, 23 Jan 2024 07:31:20 -0800 From: "Michael Brown" To: devel@edk2.groups.io Cc: Ray Ni , Gerd Hoffmann , Laszlo Ersek , Michael Brown Subject: [edk2-devel] [PATCH v3 4/5] MdeModulePkg: Add self-tests for NestedInterruptTplLib Date: Tue, 23 Jan 2024 15:31:19 +0000 Message-ID: <0102018d36f28154-cb2f6e0a-93ea-490d-96bd-5c804c984e2a-000000@eu-west-1.amazonses.com> In-Reply-To: <20240123153104.2451759-1-mcb30@ipxe.org> References: <17ACFF3FDD20CD9A.13754@groups.io> <20240123153104.2451759-1-mcb30@ipxe.org> MIME-Version: 1.0 X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00, URIBL_DBL_BLOCKED_OPENDNS,URIBL_ZEN_BLOCKED_OPENDNS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on blyat.fensystems.co.uk Feedback-ID: 1.eu-west-1.fspj4M/5bzJ9NLRzJP0PaxRwxrpZqiDQJ1IF94CF2TA=:AmazonSES X-SES-Outgoing: 2024.01.23-54.240.7.20 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mcb30@ipxe.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: nJvfaRzWsIDfoixVNOrN86vXx7686176AA= Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=Dwdpv6OW; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io 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 --- 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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-