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 2BAD5D81194 for ; Wed, 24 Jan 2024 10:24:50 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=VdK6AQO8AwjkuF9xFHKRIIJstDNRNSpy6c3kOX6fpW0=; c=relaxed/simple; d=groups.io; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:From:To:CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References:In-Reply-To:Accept-Language:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1706091888; v=1; b=rkpmoBL80+3+Pc9CO3netyqTPu8srmzFY6vNHSN4R/rZrTiQHIHvZUOhn+F0AM87uxqgL8WC iPD8/s7j8lOp4AUV2UOlkVpLkyo1PioCFqQqZSt2bLcPa+TDYuCUHZHWk24m2Y6OL5jjgUzaCGk eeKC7KD8//sCcR6g572KQ6Mw= X-Received: by 127.0.0.2 with SMTP id FBlMYY7687511xHtXy8owHTh; Wed, 24 Jan 2024 02:24:48 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.65]) by mx.groups.io with SMTP id smtpd.web11.19530.1706091887779002601 for ; Wed, 24 Jan 2024 02:24:48 -0800 X-IronPort-AV: E=McAfee;i="6600,9927,10962"; a="405553872" X-IronPort-AV: E=Sophos;i="6.05,216,1701158400"; d="scan'208";a="405553872" X-Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jan 2024 02:24:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10962"; a="876644896" X-IronPort-AV: E=Sophos;i="6.05,216,1701158400"; d="scan'208";a="876644896" X-Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by FMSMGA003.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 24 Jan 2024 02:24:46 -0800 X-Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Wed, 24 Jan 2024 02:24:45 -0800 X-Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Wed, 24 Jan 2024 02:24:45 -0800 X-Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.100) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Wed, 24 Jan 2024 02:24:45 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=l5DUU+VzkOIaP3xOezC5fNCF6hzlLIQ/5ge7WXBMptMZYLuvSwMqpoRSJXOk9xkA5xGkKRwziLFsos+P/ed9qwpTvz2V+PUVb+6+JA82oHGHZJjtZJTm9hMMMpDHOmA4RqE6AhOwZ9LJkLKO0e7Wcgs67NyvzczQeRwAyWmxwDme9ChWIvQC8ExnGLhZ/G52rSrVKPPF/KdT6sFytUVcxlebjLSZT060AQ7iJSMborEuIeJUai9eJeWGdc8wUoaP+qNJcELAUYbokIdbE2Ie9uVB8m1bjT4d0yhTmvfq2uhAFfLvMePR/+HHWzhhUOJp/OfWfb45DEX7Xmykj7Clyg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=cvqqL0fBzsnEu9IZGxBBuZThIhx1d3EixLuez0inCJE=; b=a5JmX/gsTVCCwp0S6d5TNaYvGEK6LpxO165fuzuDMdSR2J61fCOL57UAwYEbX5kLaKiGGmZMll7D3ZrujsSid4FrPNJozeE0ez+01eB3zegIl3J6JVKKcMhJSviCjuij0ja20SeiOnZE3fo5IW1r7UmAkC7grWXmLzoyanvfMDRskJMHAgEdzLz+3iOsF4QA+HvwEK4zuuH8kb36TtWqYDyKbrmIt5JqiEMhQwCMBb5ob39isxAmECIHXUesdMDy87q4hDBAp+Jy1nT0tP8NYchZBILBCu9NnRAl0k10uNBNbz+GsWCpzXxHml8nDwpSgPTASPxJska3lI5317DvBg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none X-Received: from MN6PR11MB8244.namprd11.prod.outlook.com (2603:10b6:208:470::14) by MN0PR11MB5988.namprd11.prod.outlook.com (2603:10b6:208:373::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7228.22; Wed, 24 Jan 2024 10:24:44 +0000 X-Received: from MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::fdd3:11d7:1c15:6c2d]) by MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::fdd3:11d7:1c15:6c2d%7]) with mapi id 15.20.7228.022; Wed, 24 Jan 2024 10:24:43 +0000 From: "Ni, Ray" To: Michael Brown , "devel@edk2.groups.io" CC: Gerd Hoffmann , Laszlo Ersek Subject: Re: [edk2-devel] [PATCH v3 4/5] MdeModulePkg: Add self-tests for NestedInterruptTplLib Thread-Topic: [PATCH v3 4/5] MdeModulePkg: Add self-tests for NestedInterruptTplLib Thread-Index: AQHaThFG/LRcW1Xn3kOJg0YZhHf0wrDovymA Date: Wed, 24 Jan 2024 10:24:43 +0000 Message-ID: References: <17ACFF3FDD20CD9A.13754@groups.io> <20240123153104.2451759-1-mcb30@ipxe.org> <0102018d36f28154-cb2f6e0a-93ea-490d-96bd-5c804c984e2a-000000@eu-west-1.amazonses.com> In-Reply-To: <0102018d36f28154-cb2f6e0a-93ea-490d-96bd-5c804c984e2a-000000@eu-west-1.amazonses.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MN6PR11MB8244:EE_|MN0PR11MB5988:EE_ x-ms-office365-filtering-correlation-id: 47b21149-9467-4c77-d6fc-08dc1cc6aea2 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: qGL+smdZLcpcKrFvVJx1gTWvKRkG9/5VtLSbksg3j2w+xqRpC+uaOV03MlDHk1aMUWtefLKakHS2Q0+Uf0v+dDa3ZTO7Et0HxfWqySn5YkcZeq2ENmc/EouFAdV+GaZVG/suQRiNEf6GvOKiaKBwf9axflZUr62FR2OB7oFwbgW2t9cgmvh4BWEkX4i8FIne7HJPp+dI6T/SsVGWOUL4qwo/d1qUGKwvsFK38Iaijkr12Kvk4yRlPYXwLhmPzsHbLKXnuK6/GYhKKeSFk4YDhjTotZ0rwCOnswk6domMtGIe2o6vpwHKVScCgPS0KyS0eD/Kk2z2niPlWj4VjXWrHYLlogsW4uDa1gP4f2XHpcMUCGP0mQsxGD4IX3962um5xYXQGB/euJo8YutqGkZ16roV14pxvjfsbKk0q2SAPcOV1nFZXHs1aAX34GBpHyZW8JddBGQdlb/Rz0IxvBzmopPbulzW3fLCZLigwQmA9aOftQIY9BozSfBOcsK75/6mpP08zRlNYyttxrZWSNarQRFSWkWRrfBp2vMCXmwldQOT8gWXDFVv+fxHn3Ta7plGnNdD/42BCFgJGkeHiK0a0lZ2NbAHCSaoYyUFpZs+jcCyFCk6VLVAF9kbiXXbhfl7 x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?AskVDUj9OWsNR8hkheP1HLHx/C1kupwA98bZHTruzo10n5Ck8VIrYXDQ3R3+?= =?us-ascii?Q?Xk2DggvH1WLNTT7cu8l8lC+d8hyo0IkoMp5YsuHXJ2OYz+4vA4xzi8xTIyKV?= =?us-ascii?Q?wdBr4UgMbQnWl9wbnL5xjV42o4hHJ0BhFZlTj32HeQvlopFXxBmbcnqhS3yP?= =?us-ascii?Q?UyI4e5NZaqzEQ+7DzBHuy91G9biaE7sEbsK2/iDjDShLj3eL8iK7XQTvwERv?= =?us-ascii?Q?7jj/ir6BCRkWejkukwz8H1NspGmPnX7IxYfv/snJVDlunvvFPxMiA7uRN8B9?= =?us-ascii?Q?NoIe88iRljKN9QX7g7m7O+DsnFzpd3Nwz7/G3QQZ67dkMSvoKNbdroMIepzQ?= =?us-ascii?Q?6JeSkPozy/Byny4GqR6JBje2sARefsTnepaLx/TPHwIyEWCjW+1Oar5/LFll?= =?us-ascii?Q?bl8WNe2jDgtWUgVPFK3C9bUmwuH9YW4lQkpjX5avzxOdpvm2Adsb7xcFu9Y5?= =?us-ascii?Q?P6LhWxEnHvCAzPd9l1h4ZQ5HgdE94z+WGgNXYGgmT9WVs5PstGM0qyIXyq9Y?= =?us-ascii?Q?TB0PP2mdHgc7XoyIaLYZvYU5PxzfXOwHEYEcnuebWSoE6EBTqgwW+NQTOOYH?= =?us-ascii?Q?PDYLyktJk1vJInEc7yW7DDfFd5qR1edSScKTQ3NCSlzLOp9vqaYWknIB0rPQ?= =?us-ascii?Q?FAn03T+qKsXWI967bnZ5QLkaM9dTq1TFh2rptlI6J6hackQVKn876oXCTSjg?= =?us-ascii?Q?QGGtsfxBv5u4ar9ifxBM1zWVVFg/9SbW9wVBZXDPyEqm97iOuk35Jp89ReQ1?= =?us-ascii?Q?4oHFK42zmAeFaxB9/sHsKW+1gMpCXqTObgvxfgDwSASA3tYg2lfXDLk7fSWP?= =?us-ascii?Q?GZNEyblTQkH9+16WgmmsHfZU7as/jpZpm099KDPhwsEeEeIbfXkJucbnpPid?= =?us-ascii?Q?0VLqMjKS8bPXEpDqC+GnkUlcmuxosgjhZzx2UgjCuAKbb2w+niQfdf11oemN?= =?us-ascii?Q?2AofG2hVkxwvYf8DTmsd/4y0vn8EPhza8ufKYc1VgWwgPHY6v0/4pD81wdbg?= =?us-ascii?Q?bl7azf2pUJT+YFPs96UhLt6LtIdM8hRufBZA55im/gKrGvIiaZc1dGI6uw9I?= =?us-ascii?Q?Sqpl2Z/R2ImEPb2YhL8qHB/8C0Z4Bs9BzPCexDoSJngX7JbjBU1dFy4ESeVo?= =?us-ascii?Q?2dKD/c0ZcbqH0eftlkSHtUX1mcbyNBMA+j0kDyKEVVmk4Svi2EK4r4VxpZcK?= =?us-ascii?Q?w1bSRe5exy3hpiDo1Zv6oijdd4/PBIslt5uxfrJuGYOHL5QxC4yFxkyh5FOn?= =?us-ascii?Q?RkRAZLEX020FOg4pCwS2IJEgYWdOxFifwsUjdRxqdby6jKU6CviIaDuJGWIt?= =?us-ascii?Q?BNxqz5HirSL6QNAKaOXywtCHJpR9qyZhTb2c2NCY7mtbWLBZI7lJQNBmyQzw?= =?us-ascii?Q?c4a53BbRdKaKyYHAosDyV5jrf0/xKxWw0fS8hzvKJ+rXBGEZy1RKg9HVFy0R?= =?us-ascii?Q?mJ2DRYCs7r4RAIdpeeoGxNYo7/P8KJIvJmMn/r1XM8gN4k/O6vOYgPPhj5Sz?= =?us-ascii?Q?ZUDIhYIv6JZpdfWK6oQbJQqXmDQgPAerJa7HPhRmnRKBolCoKEKVoew2hoSF?= =?us-ascii?Q?X9tqukH3Yi0HBnGV/nI=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8244.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 47b21149-9467-4c77-d6fc-08dc1cc6aea2 X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Jan 2024 10:24:43.8211 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: XUMSn4n7gqa4nxY6kw/lIwdLuF1HS9tHUhV7e04wZAHx7vdftCK4XRxStdQFlTY8jtNK/U/Qt745x1b/63TTHg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB5988 X-OriginatorOrg: intel.com 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,ray.ni@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: YRIjatwaD1f08haFpWHGtACQx7686176AA= Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=rkpmoBL8; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none); arc=reject ("signature check failed: fail, {[1] = sig:microsoft.com:reject}") > +// > +// Number of self-tests to perform. > +// > +#define NUMBER_OF_SELF_TESTS \ > + (FixedPcdGet32 (PcdNestedInterruptNumberOfSelfTests)) 1. can we avoid defining a PCD? I do not see a need that each platform need= s to use a different count. How about just define it as 1? > + > +STATIC > +VOID > +NestedInterruptSelfTest ( > + IN NESTED_INTERRUPT_STATE *IsrState > + ); > + > /** > Raise the task priority level to TPL_HIGH_LEVEL. >=20 > @@ -211,6 +223,16 @@ NestedInterruptRestoreTPL ( > // > DisableInterrupts (); >=20 > + /// > + /// Perform a limited number of self-tests on the first few > + /// invocations. 2. the comment could mention that the self-test only is performed when no nested interrupt happens in gBS->RestoreTpl() call in above. > + /// > + if ((IsrState->DeferredRestoreTPL =3D=3D FALSE) && > + (IsrState->SelfTestCount < NUMBER_OF_SELF_TESTS)) { 3. might have coding style issue. > + // > + // 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. > + // 4. The comment might be wrong. It does not indicate the test has failed. It's possible that the timer period is very long that causes no timer inter= rupt happens till now. In that case, IsrState->DeferredRestoreTPL is FALSE. That's not an issue IM= O. Or, how about we stall for ever to wait for the timer interrupt instead of = waiting just 1 second. We could wait for the IsrState->DeferredRestoreTPL is TRUE. > + 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. 5. The comment "failed to cause interrupts to be disabled after IRET" can b= e inside the if-condition. > + // > + // This error represents a bug in the architecture-specific > + // implementation of DisableInterruptsOnIret(). > + // > + if (IsrState->DeferredRestoreTPL =3D=3D 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); 6. If we change the above code to wait forever until the DeferredRestoreTPL= is TRUE, the above debug message and ASSERT() can be removed. Agree? > +} > -- > 2.43.0 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114272): https://edk2.groups.io/g/devel/message/114272 Mute This Topic: https://groups.io/mt/103911608/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-