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 43A8DD81163 for ; Mon, 18 Dec 2023 09:23:28 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=jV3DUIaV5Ls+SuP1FmEmCO+2/JVyUqVX6Jlbm/xYLfQ=; 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=1702891406; v=1; b=FbrylqNTinpat3z6RY/MVY3VkIaw3nMBr0FzvTpPkZ1BX7W040uZvUSGhGbG+NoaoPs0i5VP WTyr/fJEf91iheOISTgkOYPSVY8MDFKS9KFahEDirb9UM4NaUXiGYjdw26PFMiUp6u5JAb/KuiN rSKUaTiSbjeHFiCynLgu+Hfs= X-Received: by 127.0.0.2 with SMTP id qkhSYY7687511x894xpzaebt; Mon, 18 Dec 2023 01:23:26 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.93]) by mx.groups.io with SMTP id smtpd.web10.40279.1702891406323030289 for ; Mon, 18 Dec 2023 01:23:26 -0800 X-IronPort-AV: E=McAfee;i="6600,9927,10927"; a="392649136" X-IronPort-AV: E=Sophos;i="6.04,285,1695711600"; d="scan'208";a="392649136" X-Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Dec 2023 01:23:25 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10927"; a="919198945" X-IronPort-AV: E=Sophos;i="6.04,285,1695711600"; d="scan'208";a="919198945" X-Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmsmga001.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 18 Dec 2023 01:23:25 -0800 X-Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Mon, 18 Dec 2023 01:23:25 -0800 X-Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Mon, 18 Dec 2023 01:23:25 -0800 X-Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.40) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Mon, 18 Dec 2023 01:23:24 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=H6f4F8oIbAmPN+nbikV02TegV0ttPJTPfHsaN3VQ2tXmELVQM6SuIq0gMcGqItGhC9KSw+AhI4emQMOBbMbB+hFN3bMXzxaSz2gQl9DfYeeanXpwood1yN013oly8QP/vg6viIViyTkOEnhWRNNwYbrVKTxwIYXZg3+It+ugxI4x3oJ4uy697LI4EOj4HPCANTxf95wrqOP0Rz4aOohPiwUiUPr4kspqEr8JsYHqjGL8GQAh1ZsjpmIKGTIzFjY7JdgX2RC4aCibNEOjb4+oECtj97nvw97RdzbmmUOB3WWIPvTeoGxzSGeK2EGvi4kNlFOsktM1t1bw7R3clFhkzQ== 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=Nqt5ly8gO051Am8/PTnRa1CAbdz27MqpNoyoWD3hCZw=; b=Zb2UF0W+PrkOunQKxDaqmfSMiFEcCBRvbifpgVc8ftoHFo0UpxnE2I5Fp1DcCBMQwqDNMjLKYqwl4SVF+rNeSI2URWtSELXf0v8v12+Q5tfmP5Pbdj+WHhTqiWWzCzpjppCgXTiGZaEUG4eR7LlTAxIdSD6tXqX80H5cD8AYJoXQ/AGaBP4BOXU+x3kkQW4xzsLxKhDesiCu0Uwy+ww6epMRFZ2a5CYi0cLowHi9nrMK9OQQxXKUcbwC3HQPcEfkveMEBD0a1EfTXMgC8jv63kPdcJwiCdODyfZufza6u8dK6uIyGcE2W6oKi/hfk3YYoNyAvjhca4GVaIN4BScBPw== 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 DS0PR11MB8162.namprd11.prod.outlook.com (2603:10b6:8:166::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7091.38; Mon, 18 Dec 2023 09:23:22 +0000 X-Received: from MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::b614:1f5e:8b0c:9858]) by MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::b614:1f5e:8b0c:9858%4]) with mapi id 15.20.7091.034; Mon, 18 Dec 2023 09:23:21 +0000 From: "Ni, Ray" To: "Wu, Jiaxin" , "devel@edk2.groups.io" CC: Laszlo Ersek , "Dong, Eric" , "Zeng, Star" , Gerd Hoffmann , "Kumar, Rahul R" Subject: Re: [edk2-devel] [PATCH v4 4/8] UefiCpuPkg: Implements SmmCpuSyncLib library instance Thread-Topic: [PATCH v4 4/8] UefiCpuPkg: Implements SmmCpuSyncLib library instance Thread-Index: AQHaLzzbnSBsrNIwkE6IIje5dI7GG7CugsPA Date: Mon, 18 Dec 2023 09:23:21 +0000 Message-ID: References: <20231215095515.9484-1-jiaxin.wu@intel.com> <20231215095515.9484-5-jiaxin.wu@intel.com> In-Reply-To: <20231215095515.9484-5-jiaxin.wu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MN6PR11MB8244:EE_|DS0PR11MB8162:EE_ x-ms-office365-filtering-correlation-id: 3eecb3dc-3e1d-4cd5-11a3-08dbffaafa44 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: xSb1/j4hbZrJjoVZtZ+j1ohyBhzFxjUicORIjlGXXzGW8VBJnhKIuc4qALNMSh5fgnCZsrQ5OQ+sOfZfFVNmSl5IP4l4/lYkjwNGft6dRhm6bsWQtO1Zj7suqTYzMWZwSyirL7D9lOA2M0Wby0LBnU22H1WnhR4t40KTH7ner7BrNPqy/rAhtk3XxY75yC89oztudtrSR93TD/n156j5Hh1m8JkpTCx7iIl+HmThmO8iv6LkBOvtX9A3C03l2cSVOuNrm4zdaVJM0731eohBOR7vuG7LWqj1FKTu8WsWgftNC/VildyEjhebk/DHnUG7EHFYCcWsjBe31LUmUBARUTQOGtOYAaHyuPeRHd+04UzI5sApA3fb/BuEQrYmlejMBa/WDfPoiglZlhAQ83HG3DpxpltIp9g+1tAnTOnhIszrBlImhCcF30zH8OYFXaTrMrSQG6YS6tshQ20kovumpxh5db3jYWLYMaqutgsfyxetkL2paoH6bNd/ImKf6Pfse2ArY66xPv/yuxlJbDhT0dR8TFnp4QdTdybUNXg5JcRJyXCl5xhMfC3DznL03BYgCoohFeLTeUrku68CLsLbq8vPXlkpw/VatLwYhiA9Z3wXVup1H9DSeqEko9N4TEk7 x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?HDo8a6GHkCf/pvjK38MXOTSIv35h3UHWHJOs53nFckiMtDxJ+YZREWW4YFuC?= =?us-ascii?Q?NcNlUBN41HQWbh8X7aplDYg2h/WR/C61Qasph+C3b/fCdjtG3s89LYkLFe2a?= =?us-ascii?Q?XW3CfEEJSZ3VYm6Rr3DUzuuoDaaYMXD3dWTjwcEQxWi/1Vn8NzwYFEYSd+yw?= =?us-ascii?Q?07V9lvmrhifm3+e3mOuS/6mR5oW2ZxBfSf1Ks/Rf8BuePfZ2QznSBayrGM58?= =?us-ascii?Q?06cslX8POOCGOecXFY1KdBdodvG511IH9ILbgJA95m+8Myrrfmkpvwzkffdr?= =?us-ascii?Q?jAH9Ut07t4uh0nIzrYIKdIy6NIDTBiEKYvyK3NDEHdkWm/YFEh+PG/403wk+?= =?us-ascii?Q?ACJIzPK12P+xModEasSLXSc17sR2FXFQBeo5YCrbqi59mx8EJiWbd89AANr5?= =?us-ascii?Q?0VzqPASAGeMNYzkx/LgpUi9gpus+W3GYw+5hujjvnxxfxxHKdf7MFsU0xHcb?= =?us-ascii?Q?LAwCXRhwx5vHK9G1PXCg/pRpKIWvFSEhEgfuloz+FyGwfZKhitLmDAvbr2tu?= =?us-ascii?Q?sPQZGdXyqgzwScOhb9UHQcrQkBnww4Ony7o/eQAskdjAE1UohSvbOK/s7fai?= =?us-ascii?Q?ejegSyQhmR+zNhg3BsjnXXvkGrkzM8a1RR/gzKOdE2G6wf1pm/SgPjBZgLyI?= =?us-ascii?Q?sY6ao6g7SlOUnpDymgRZ5io8ofJBjqfF+Txb/1DDGz6eUGQ4j/WWxCd/mWez?= =?us-ascii?Q?nzWk852eEk/rEe14HxhMTSks2bzoChoOP5GX/Mtna4OzDveHj8j0EnAXsre7?= =?us-ascii?Q?sneZ60eX1zGcQ1iQFFc/Kv0QxtUoRCUvOUnvtdUv1to3uxoNUoF/Kj22CbBV?= =?us-ascii?Q?aS4YRESSp7jNqoe/n2G2Yff2rOJK7XuY8J50L1xeSscGcAgn3qvrL5yd29p4?= =?us-ascii?Q?OOqWsHTzZPOY5txJZuEnYHq/3XuB383qi2UHEL6RooQaXds4QnNOUW5kjW1d?= =?us-ascii?Q?LkmfZ7O2e4uJqb589qrfM8GNiWx21XOESQJco7RAYJs9cCqH1cOgkPtRu8MV?= =?us-ascii?Q?n8ZtRlKEd8WuyPLtHyASzhfhqzNONoG44xQLfcX3bFql5lHUz7gfTB2Bsf4C?= =?us-ascii?Q?z1oQR8zyE1uuSiEHy0LTU9xVsIiRsZpCVqy+UCxu/wOSHE0J4tECva8xxYK7?= =?us-ascii?Q?zwg9naUCy8y1Xzxz6fG6QnCG6We/94cbl4VWi0xoR/IyP/lTPRa1Z0CeY9yr?= =?us-ascii?Q?SbTB0YwXUfgA0dxRxNBk5X6qMkLlY1QvtIIg3AtR2eDt0PcbXXvsgIxyKCEC?= =?us-ascii?Q?vDNbD6qIxflTzc6A1oBWQLsl1eExThDpOGQA+7INmiPllJKYNULb8QpqZ+Aq?= =?us-ascii?Q?QCNpdZIMGXw01GJWYv/p4JrhNkuu0PRPnNV1fcTIzvR91l8YGMwIpxNbiY75?= =?us-ascii?Q?HGrtEdMKDE1hm+rrwmRMhRxwpHwmR0blxZ/5cFwB5M66YSwFMZsmgxphMlhV?= =?us-ascii?Q?UVGpU8MpL9xDvaQitotvBGmzPC59WLfteZYNaauWROdZSwvuhC5Pwiygo9A6?= =?us-ascii?Q?hOEt9D7uE8jQJHDqCDSGxfMpH7sRyVQnJEn1lO7zsrIWntMcRZR7JC1snwq9?= =?us-ascii?Q?9qreiFxBLutXZadjfbs=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: 3eecb3dc-3e1d-4cd5-11a3-08dbffaafa44 X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Dec 2023 09:23:21.1332 (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: ym3n4SReTQENkqPQNcu0Oik/taOWo13pUez0yoP1pH1qPQfmOv6yE9auFgfssbTq6WG5BltmIbojWMNmg5UK1w== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8162 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: n8Z6LgQmQfhpaz9SZwi1ihGdx7686176AA= 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=FbrylqNT; 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}") > + /// > + /// Indicate CPUs entered SMM after lock door. > + /// > + UINTN LockedCpuCount; 1. It's not "LockedCpuCount". It's "ArrivedCpuCountUponLock". Comments can be: Before the door is locked, CpuCount stores the arrived CPU count. After the door is locked, CpuCount is set to -1 indicating the door is = locked. ArrivedCpuCpuntUponLock stores the arrived CPU count then. > +/** > + Performs an atomic compare exchange operation to get semaphore. > + The compare exchange operation must be performed using MP safe > + mechanisms. > + > + @param[in,out] Sem IN: 32-bit unsigned integer > + OUT: original integer - 1 if Sem is not locked. > + OUT: original integer if Sem is locked > (MAX_UINT32). > + > + @retval Original integer - 1 if Sem is not locked. > + Original integer if Sem is locked (MAX_UINT32). 2. Can just say "MAX_UINT32 if Sem is locked". > + // > + // Calculate total semaphore size > + // > + CacheLineSize =3D GetSpinLockProperties (); > + OneSemSize =3D ALIGN_VALUE (sizeof (SMM_CPU_SYNC_SEMAPHORE), > CacheLineSize); 3. I prefer: OneSemSize =3D GetSpinLockProperties (); ASSERT (sizeof (SMM_CPU_SYNC_SEMAPHORE) <=3D OneSemSize); > + > + Status =3D SafeUintnAdd (1, NumberOfCpus, &NumSem); 4. ok:) you are checking if NumberOfCpus + 1 could exceed MAX_UINTN. Fine t= o me. > + > + // > + // Assign CPU Semaphore pointer > + // > + CpuSem =3D (*Context)->CpuSem; > + for (CpuIndex =3D 0; CpuIndex < NumberOfCpus; CpuIndex++) { > + CpuSem->Run =3D (SMM_CPU_SYNC_SEMAPHORE *)SemAddr; > + *CpuSem->Run =3D 0; > + > + CpuSem++; > + SemAddr +=3D OneSemSize; 5. SafeIntLib was used earlier to make sure no integer overflow. But "SemAddr +=3D OneSemSize" is simply ignoring the danger of integer ov= erflow. I agree (NumberOfCpus + 1) * OneSemSize shouldn't cause integer overflow = when code runs to here. But initial value of SemAddr is not zero. It's still possible the SemAddr= + (NumberOfCpus+1)*OneSemSize causes integer overflow. I am ok if you don't fix it as I don't believe the integer overflow could= happen in 5 years. -=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 (#112638): https://edk2.groups.io/g/devel/message/112638 Mute This Topic: https://groups.io/mt/103187894/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-