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 647767803E1 for ; Wed, 24 Jan 2024 10:26:17 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=JAEDCyAWO3vOn8k2slykuNcJcXUJr76Mc7V5owwTT2I=; 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=1706091976; v=1; b=IOMlOs+t7GgMDEKlyCcg+VbQGkWvrem5BlgzVDXVPm+MDfGe1MuToryMgfJY2cC1ANtwcnoL yKp07XwZqu8ynabXPh97b4+erNBBHPUZCUC3RLuNK+TAjBAlaFTz2GG4u26BM3zEd5qvDFfFTxu lVY3YyvFyGc9vpsX0V58qucQ= X-Received: by 127.0.0.2 with SMTP id 7xfbYY7687511xh6Rhu0aMGv; Wed, 24 Jan 2024 02:26:16 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by mx.groups.io with SMTP id smtpd.web10.19474.1706091975385463402 for ; Wed, 24 Jan 2024 02:26:15 -0800 X-IronPort-AV: E=McAfee;i="6600,9927,10962"; a="9186016" X-IronPort-AV: E=Sophos;i="6.05,216,1701158400"; d="scan'208";a="9186016" X-Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jan 2024 02:26:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10962"; a="929632750" X-IronPort-AV: E=Sophos;i="6.05,216,1701158400"; d="scan'208";a="929632750" X-Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by fmsmga001.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 24 Jan 2024 02:26:14 -0800 X-Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by ORSMSX603.amr.corp.intel.com (10.22.229.16) 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:26:14 -0800 X-Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX611.amr.corp.intel.com (10.22.229.24) 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:26:13 -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:26:13 -0800 X-Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.41) 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:26:13 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=idfrWbXvglvD+Q0u3zSgr6Ab+px8qCSkECKD9slPsfYsLmitmfOtdWs7vCqZW1eFALnhdAg9hzZBjYaMzECcw9Abu5d7SX+UM9uKKsfWz5D3rACkI22UK3a3BN6vi/xEx3ODBrcjT2zB5LzrE/qZQ45rllP6N/Uxe4qjIvgQOq7tpLBjLGV9m8gaI9rqZGuh25xVkAXUyZNYFc/fYNdi/7Dpxp0CF1Az4gKBNlvRx9wuYKFlKesyZwoBUHow4XMV6oLIHv55DuOkKP6GMfJsfm6gfv02sCYneTdLcdHDVnurl+mdkq1vNBXBio3FG5WXWW4XHDv7afa7jiCi8Kqgfg== 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=K6sx6vTk7kVtw6VHb8JcLaB7j6PfWXB4VyATg9AdAc8=; b=gaJ4wZSdQmKH/dLwM86QsQTET9p1nd4OgxmTHMXTrIf9QDnXmZhd84O4HtEZ3AeBg8EGR1oQNz89ArO85x+pP3pZRnCQ19h3SvEfurUMtsaeBA8zpniak/rwQ/iEfl//DiBtwg1V7FhjpHjv+tbYpFK6K3aJjSrx7x4o7CHMVQkrO1Arsmf9HI6hQa0r6R6DrbX4xmizZQkSSpA7+mB+3tuJhppkT2T3kvw06mZb4qDC5bklqoBGQz1OWhrnjXKkJCdCrPN9Zs0SxNCbwGnQ5EFhGC55WtghhLov8QwSpEUNuwFSipi+d81hiKjUN5VG9M6taWIfR/8VCfuv8namXw== 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 SA2PR11MB4985.namprd11.prod.outlook.com (2603:10b6:806:111::8) 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:26:11 +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:26:11 +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/LRcW1Xn3kOJg0YZhHf0wrDovymAgAAD1XA= Date: Wed, 24 Jan 2024 10:26:11 +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: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MN6PR11MB8244:EE_|SA2PR11MB4985:EE_ x-ms-office365-filtering-correlation-id: 7db589c1-b3c3-4139-6730-08dc1cc6e304 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: bFdByeM3QynueHIEwAnIFZ2ZnFmRagaE0A9Q0Kz0BGFMYfF7kMW5w7VnMJE18anvCzKcrYo5MIL6NU8feLb5eQDx7GrZVIX7WJfb4mdaC3+zzhckYybvMWdCzN3O+vZJduRYAZoJte/omLIfeCRWUBAFT3ANmK9u0GrcAZHrFlx49g4V6AaUYIQoZGcZlKHH7FMs/hgSSk/z56BwBr+1bH2nSH7eohnbBjAwbZyh0r+XSVplMfKcBhyrgCZ9cDNxaqdSHI9+AsrtNTdcNhTfMluqIGjnwT+U9YOV/4iddMUkfVJyv3hxvQBHuhmFWcalsWDD4lGPUdU61YuJlT9BjVP5b7p5G6Q8oZrnpJOioli2VA92qPLY9A58zkPbvnjMyQ2OOtRj6TZ4n847Lzl2cPemjawDxFo0+1sdaw/G7c9q3WUgbB+5aNkM7krDne/yB1V3BL+97BpfBy3HmKIbqjqRBqwPSLOEwslqGzBd2ND4BMNn1bTu9phPuqzaPumOgkWbz5bhqCiMhhS370K9JHpCltuedc/hot/SsqIiG3Xf4TO5g+VT5s/m2PPyKxS3fMS9I5Xzdc7A6Ev5Yd6+cFUYAQZTzYJiPE2y8lApqlr8JqrU+QzRZpMn1JttFuxN x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?+yqJH28jJv48cgHoS9rpRrlhmN/UUDZ5RPChfMse+8dIRC865jxpgmW0RxM1?= =?us-ascii?Q?idje8AjF3NvzRy0dy6rNa/cqj6g1NYiN4SzWM+d6z5o1Jg+iblwLNjcDBexx?= =?us-ascii?Q?tiTvD4SNPYQ1D97DigNbTOTXXnoWCq4opeoKYLHhZZdsStR7C3GY78jME8Yu?= =?us-ascii?Q?PuNGYimBFF5mQwcxTNE2djcIWzuEQn7PxGhzckjOxVONQyYDl3bdIzyUW/fb?= =?us-ascii?Q?5z7jJz8jdqSmTslDKwyevUz1WI4x0FO6i8HBauuxN136kyYxyj0vihZYYEZb?= =?us-ascii?Q?Fa7AizHewrT0phfV3EwZAeftQCjHF2rbq3TBfH9hfIzpGkBfHkgwLE1ESGGP?= =?us-ascii?Q?2qJNa0pAVELXV+jSbEBbSHgmNUQC5IAI60HuVs1Csv11JmcuhCOPucaUsnPu?= =?us-ascii?Q?yFk7YoRBRIaYcJ/wz9sreer0YHel53gDaTBF7sgWtrBWWyVJmTj68qK7VsiR?= =?us-ascii?Q?42/JKUz4UV7aO8DVr/kfka1oPr/RO/E9BsQnHHbdTdgVQrqyPgqEp/KnFUe2?= =?us-ascii?Q?zKHeUpic3kJ6S/SDHe2FjaKXa8xIil9R0yv1KGwq6LLBa2hnwweRviAeuGg9?= =?us-ascii?Q?TDPtkF5HwWrDsuHRgHVPpB6+d8Ljgf4OhYkw+YqFWaezaRGVMCySMM37IGGT?= =?us-ascii?Q?FohY/KqAsF82EFrsDomrx1n/jocolWV0m9ijaRD86MkeBklZ0+sc+PWxuBtJ?= =?us-ascii?Q?MGuBVu6gJW3Ed2RCRj+HBb/rNX22GldlHK8dQDS9z6KmcxVyRExlPBQ2cOyp?= =?us-ascii?Q?ZDan/ZtckuiywyU/bKgjUqNR+9YDo3c60m8ZbPwK+0p4FczzjwYjAT8ffg9J?= =?us-ascii?Q?iRbPsPJSA6ZN43VGdmViOh8S7yaHageIU6QxeyzKr1bCI3d5gsGPmQUF3VXl?= =?us-ascii?Q?x4fhmJcJohP6Q9603EMI2veqRAD8DbD9HEr+ht51v9B0PjzgRYpemIxUqK9w?= =?us-ascii?Q?LhLhkS4tkv9k8/ZeyP6bdQM40mZ2/HDoFkX2qC+tJxxNLk2AiO5kiHw3wEX0?= =?us-ascii?Q?STGpb2ES4ycJPi91uwDAm84CHUYXD6nAGtI4eC9ni9VXMNf8u7zdQG0FuATQ?= =?us-ascii?Q?i1AK2AzMarZf02n0KMPtpDUzrD1GcdXauU8BxsCinUJJnEPrmTL37VgXRn9+?= =?us-ascii?Q?gQdRndFRRJV/yPP9af2VANNpnvP3FQi1culq5qyR2qcLxDm2EqRgSCX63vkQ?= =?us-ascii?Q?gPZ8SyTGQwRlXL3djyR28+L7HGJ8s9wTfLjhk5GfhemsNDP9/SNW0V6vE1M9?= =?us-ascii?Q?jLK18hgC3z7gc/V5rPs/Z4dOW5oPwN4r4GJFcZVq8wIW8MVd2+gaVnDwKyGo?= =?us-ascii?Q?Iq7UfDvwdgOSQfDpSCSuaREAqOcvMvKyryPwxWZiYjgimG1xTFYq87LIeSfh?= =?us-ascii?Q?AB+thukk+YJMt6Jnw6unZvjKV4ZyGbnPQToBrvXdN8Abk4QuDpd6QsUEkLcS?= =?us-ascii?Q?4J6v88jovZIuyNBGtHUyqZYbuL3rBXbNalKGCR+F/memj/ZAsqWi3Ccx7jQi?= =?us-ascii?Q?08g4EzdL/GYcsKEl/smv3W7FPvOrvKWAOgQpBAotYL2W2QYX1WcAq1bF4g9L?= =?us-ascii?Q?gA9WwxVCibAn6X5uDoI=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: 7db589c1-b3c3-4139-6730-08dc1cc6e304 X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Jan 2024 10:26:11.7261 (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: gFVkA+4pYbLhw8Mg+BbTNcxjD3FyLUDNmDrAIo5WoX5zmvxA8pujRlEfko+e9muPgSKmV/15ruJf3mLCOVC9Nw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4985 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: aWQGa4rIceACVXxB0PQUB0Kix7686176AA= 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=IOMlOs+t; arc=reject ("signature check failed: fail, {[1] = sig:microsoft.com:reject}"); dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=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 Thank you for adding the self-test code! Thanks, Ray > -----Original Message----- > From: Ni, Ray > Sent: Wednesday, January 24, 2024 6:25 PM > To: Michael Brown ; devel@edk2.groups.io > Cc: Gerd Hoffmann ; Laszlo Ersek > Subject: RE: [PATCH v3 4/5] MdeModulePkg: Add self-tests for > NestedInterruptTplLib >=20 > > +// > > +// Number of self-tests to perform. > > +// > > +#define NUMBER_OF_SELF_TESTS \ > > + (FixedPcdGet32 (PcdNestedInterruptNumberOfSelfTests)) >=20 > 1. can we avoid defining a PCD? I do not see a need that each platform ne= eds > to use a different count. How about just define it as 1? >=20 > > + > > +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. >=20 > 2. the comment could mention that the self-test only is performed when > no nested interrupt happens in gBS->RestoreTpl() call in above. >=20 > > + /// > > + if ((IsrState->DeferredRestoreTPL =3D=3D FALSE) && > > + (IsrState->SelfTestCount < NUMBER_OF_SELF_TESTS)) { >=20 > 3. might have coding style issue. >=20 > > + // > > + // 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. > > + // >=20 > 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 int= errupt > happens till now. > In that case, IsrState->DeferredRestoreTPL is FALSE. That's not an issue = IMO. >=20 > Or, how about we stall for ever to wait for the timer interrupt instead o= f > waiting just 1 second. > We could wait for the IsrState->DeferredRestoreTPL is TRUE. >=20 > > + 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. >=20 > 5. The comment "failed to cause interrupts to be disabled after IRET" can= be > inside the if-condition. >=20 > > + // > > + // 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); > > + } > > + >=20 >=20 > > + // > > + // 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); >=20 > 6. If we change the above code to wait forever until the DeferredRestoreT= PL > is TRUE, > the above debug message and ASSERT() can be removed. Agree? >=20 > > +} > > -- > > 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 (#114273): https://edk2.groups.io/g/devel/message/114273 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-