From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from a7-11.smtp-out.eu-west-1.amazonses.com (a7-11.smtp-out.eu-west-1.amazonses.com [54.240.7.11]) by mx.groups.io with SMTP id smtpd.web11.30245.1683634172468888716 for ; Tue, 09 May 2023 05:09:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@ipxe.org header.s=cphpx6z2rfcgehlykjjh3gknqe3hsoe2 header.b=fgtaXiYi; spf=pass (domain: eu-west-1.amazonses.com, ip: 54.240.7.11, mailfrom: 01020188006aaa5a-e517ec29-3a95-4b01-9b30-1c1b609cf95e-000000@eu-west-1.amazonses.com) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=cphpx6z2rfcgehlykjjh3gknqe3hsoe2; d=ipxe.org; t=1683634170; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References:MIME-Version:Content-Transfer-Encoding; bh=48YiRF62XA2hziecwiXAgAJoZ7HfxCRz4PRgOH0uX7s=; b=fgtaXiYiDW+J+gTpxzMx4NkPpyabJ1zrDMhyoTbnrWU9iXnbiWDImE+Zr2FCT+V8 vt4THwV8AntTGYQtcnTY9whQFo80PSMp3hJ8Vb/rufCW8vzsKj+D7gDDMIEeVGi1UJY VfcYf4CoqdQRfVY0pBZ0j8qm3qLg+lDFbToKsWD5KK6L0wgSjm5DErWw2a91njeeReR vUApsCT7u6983dW17HRHx+5FVCwbJ0FCcY2NbZt3bspZ3L3KIyD5isNGHrKicgbtgK4 g8pHEEt7X6/+KKVvUQz+X1XlXpOpSWU+Zw505ytkwV670U1q8GCneRjGsyDA591tC0O J9OV/CwrMg== DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=shh3fegwg5fppqsuzphvschd53n6ihuv; d=amazonses.com; t=1683634170; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References:MIME-Version:Content-Transfer-Encoding:Feedback-ID; bh=48YiRF62XA2hziecwiXAgAJoZ7HfxCRz4PRgOH0uX7s=; b=GjLUytRoBM33oX5FHUOQ+AT9i6qrY1uMENZZVaiiDnk/Dj7chhlaNSby9bN+4lJn +agNR8hFPX80gSDy1vrDh/pPxaNOINdklm8esK4xDz/XBFOfmTeENNIt/YUPZe8EOsH yivrLoWGBUxyfuYO3IUHRYlZ6bhgAqtQo4E+Zo3g= From: "Michael Brown" To: devel@edk2.groups.io Cc: lersek@redhat.com, Michael Brown Subject: [PATCH v2 1/2] OvmfPkg: Clarify invariants for NestedInterruptTplLib Date: Tue, 9 May 2023 12:09:30 +0000 Message-ID: <01020188006aaa5a-e517ec29-3a95-4b01-9b30-1c1b609cf95e-000000@eu-west-1.amazonses.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230509120909.3970177-1-mcb30@ipxe.org> References: <175D7813A02A6FCA.31428@groups.io> <20230509120909.3970177-1-mcb30@ipxe.org> MIME-Version: 1.0 X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00, URIBL_BLOCKED,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: 2023.05.09-54.240.7.11 Content-Transfer-Encoding: 8bit NestedInterruptTplLib relies on CPU interrupts being disabled to guarantee exclusive (and hence atomic) access to the shared state in IsrState. Nothing in the calling interrupt handler should have re-enabled interrupts before calling NestedInterruptRestoreTPL(), and the loop in NestedInterruptRestoreTPL() itself maintains the invariant that interrupts are disabled at the start of each iteration. Add assertions to clarify this invariant, and expand the comments around the calls to RestoreTPL() and DisableInterrupts() to clarify the expectations around enabling and disabling interrupts. Signed-off-by: Michael Brown --- OvmfPkg/Library/NestedInterruptTplLib/Tpl.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c index e19d98878eb7..e921a09c5599 100644 --- a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c +++ b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c @@ -104,6 +104,7 @@ NestedInterruptRestoreTPL ( // defer our call to RestoreTPL() to the in-progress outer instance // of the same interrupt handler. // + ASSERT (GetInterruptState () == FALSE); if (InterruptedTPL == IsrState->InProgressRestoreTPL) { // // Trigger outer instance of this interrupt handler to perform the @@ -153,6 +154,7 @@ NestedInterruptRestoreTPL ( // // Check shared state loop invariants. // + ASSERT (GetInterruptState () == FALSE); ASSERT (IsrState->InProgressRestoreTPL < InterruptedTPL); ASSERT (IsrState->DeferredRestoreTPL == FALSE); @@ -167,13 +169,17 @@ NestedInterruptRestoreTPL ( // // Call RestoreTPL() to allow event notifications to be - // dispatched. This will implicitly re-enable interrupts. + // dispatched. This will implicitly re-enable interrupts (if + // InterruptedTPL is below TPL_HIGH_LEVEL), even though we are + // still inside the interrupt handler. // gBS->RestoreTPL (InterruptedTPL); // // Re-disable interrupts after the call to RestoreTPL() to ensure - // that we have exclusive access to the shared state. + // that we have exclusive access to the shared state. Interrupts + // will be re-enabled by the IRET or equivalent instruction when + // we subsequently return from the interrupt handler. // DisableInterrupts (); -- 2.39.0