From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mx.groups.io with SMTP id smtpd.web08.21951.1661419962714460997 for ; Thu, 25 Aug 2022 02:32:42 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=df6s3Z5i; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: ray.ni@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1661419962; x=1692955962; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=3kMpVFz7dZAp4ZUj4zjgvC0narFSkQ8K+tWnrEgjDZ0=; b=df6s3Z5igNmig+DtCvBk2US5qhb99yKMsMW6J0iDecd7VFzZVF7ofd7I wEzIZxHFt4mUohgD+LZjMVGvZMcvWyTh7UrOOziZw9QYq41/DL+yVPh79 jD6JuH7Ny+wFtZUSeLuB2AGFjmqbgXVEaIbHjeaS+TyhuM2Zm4WBst2pb ApVwwa4oXAtHNvxwjmF1HUBPUianuTiX5MWdEF6HV1LBd+VfzKgrYRX6C e8rdH8zUAljv5MsPXY0U3ytogmCs40p3wXktCDOm3xl0J6/M1d7GbBKla rOVhHLAfyHkuuNECUA941ZEidVlaEmkeNry+DEljT2j7bNbRhQuakJa2J Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10449"; a="320271123" X-IronPort-AV: E=Sophos;i="5.93,262,1654585200"; d="scan'208";a="320271123" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Aug 2022 02:32:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,262,1654585200"; d="scan'208";a="586805146" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by orsmga006.jf.intel.com with ESMTP; 25 Aug 2022 02:32:41 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) 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.2375.31; Thu, 25 Aug 2022 02:32:41 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31 via Frontend Transport; Thu, 25 Aug 2022 02:32:41 -0700 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (104.47.57.177) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2375.31; Thu, 25 Aug 2022 02:32:41 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dH4VB41tTMV+EeS8drok7YZb8HLO7IiV60FJ0Lnavy8A1C7TuaYC3K+uVUY1+jSWnXLOWWvdHpxaLO4VRmq92lol9UmMSuq5yY9PtI7SRQtQzLA9eHfOh/mh6UNsQAQ60dlGZhu09gRaeURKfYFV0jMF60OZF7c3IH5PnBwzUAGv2BIQSXjbziXzbe4Q8sc2+7KdHCfzWINSqYP/jbXah2+Jk9AfnwfNtYl1wdhIEhEEFs+kENUPgyk58kjN3ysRYs2eEf5C8Dkd50Fomcx3jcCbo7TUHkS/Ews7eHqd+DywKe9EvX7wC4KkI5lJBFTCWI+J7FM6ViKldpew1ZzJIQ== 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=484EbH2bcrjEi4g3sKxb85A9ndBoZOZOE2DKqZFIvAE=; b=HAXeUk3+KKKzbRvEbGW9w+1FGmGkofo5B647X6OOQqa6Su917N/qFK/gt7nhOUC4PmWbrr8hiEEWy2Z7pCr94f2McNwpghAZmUsvvSUzbka/dxboxaOGarMwN+eJPGGgQnDC3TXhDRJoRwGNV8mT0NIJGl07WxKBTDN50B6FPLXsvNB7gR46ysyipJGKnUqV4i9FuOwv5YOcSbs+9XsZHhO3LG9G8xwSrTFDk+K+nZ36mA9q+HAmsZB0KVbd22FErocV1QrSEpaXYhc8YssYiiQ5y0MTTx43xyViQFAYXYuSUvGqXsrjilycvBXZk9sJ8xckxRcgCS3f1FbnHgOUVA== 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 Received: from MWHPR11MB1631.namprd11.prod.outlook.com (2603:10b6:301:10::10) by DM5PR11MB1625.namprd11.prod.outlook.com (2603:10b6:4:b::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5566.15; Thu, 25 Aug 2022 09:32:39 +0000 Received: from MWHPR11MB1631.namprd11.prod.outlook.com ([fe80::958b:cc9d:ac3:288a]) by MWHPR11MB1631.namprd11.prod.outlook.com ([fe80::958b:cc9d:ac3:288a%10]) with mapi id 15.20.5566.015; Thu, 25 Aug 2022 09:32:39 +0000 From: "Ni, Ray" To: "devel@edk2.groups.io" , "Liu, Zhiguang" CC: "Dong, Eric" , "Kumar, Rahul R" Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Thread-Topic: [edk2-devel] [PATCH] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Thread-Index: AQHYuC4mJ+l+hrijbEK91SMg3CbnWK2/Vtkg Date: Thu, 25 Aug 2022 09:32:39 +0000 Message-ID: References: <20220825025506.2323-1-zhiguang.liu@intel.com> In-Reply-To: <20220825025506.2323-1-zhiguang.liu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 8c2a00ac-1091-496f-837b-08da867cc0b3 x-ms-traffictypediagnostic: DM5PR11MB1625:EE_ x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: MEkgevS92kNmFmugCDL7kh+Sj0Ruck5VQBgZ5fm+JFg+pCLMrEtuw0PqBYv6bMeNQEoBv5E9gP7znsg0yr0g05q3R9qJ5x6DG83jAzpexAgkyodP8nOWCIwGSNM2e3gqnuoXYRt0dgq4z8dx4AfZ5mK2hKd298Bi1CjTxpmPyvMJfvziRNJRr1Z3pAeLlGzSWEZeX+DIzu+w6zuykFAkiIgM+eBiPQsYubQR51hQcqCDHRNmuP+zsqXqTRaESQCtcRzAVs85OYxsjON+y4DRrkMaqR9+BLCG5Dcqsg3dWWjKQEEmrXNmqDvXbSusCvcKRBlDW2hj3SWVY7NmNQOdfEIUSTjvwrGBy9GWJhEsNUQNot8tXsqqCWAlBGrnkHG1rM/3Cy7aNkzSFORc+pbp/98a7xnTHgu4ieYdrRR8fqKwUCSn4k3OGy9i16n0b2LqoRz63CT3oh6XZsdPR2VeraLrEE1odORkxQrT6Gn/92Un8Ryu6Zth4O7Sbt+ETUnsAWYnqrRaRUI9ZIF8VcFMtfGnvLqfOsCn+C37e6r2ydh4+JcOpJFh0qNZQN7RpFvVm5+Asg8nsQC4WW8n/skvNq69BIyWUZrWswpX8idoF7Pjirn4pCin4Rb5tYX91FjJxmNr6+gg7Yzf5LHoUIkvyqgpiCnZg8sXitlHIgM3oR57y3AdBL5JxG6Jq2/VCVpbFbKKkuczzBWEi3ISUPGBTv44Vz6bYmbUnN+XoqFrMVXzcggWaWKL5aE4Pg9KO0z5cgxMtVZP5e/NU6cP/KnEVg== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MWHPR11MB1631.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230016)(6029001)(346002)(136003)(376002)(39860400002)(396003)(366004)(4326008)(316002)(8676002)(76116006)(66446008)(6636002)(54906003)(110136005)(55016003)(66946007)(66476007)(38100700002)(64756008)(122000001)(2906002)(5660300002)(8936002)(52536014)(33656002)(82960400001)(41300700001)(66556008)(6506007)(26005)(86362001)(7696005)(107886003)(38070700005)(71200400001)(9686003)(478600001)(83380400001)(186003);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?SFw2uhPNH8GjhI51O8HaSma7G3l2JdBg2FXrI8LxcH2BfLKjol4xXB1DU56J?= =?us-ascii?Q?RSLXk/fPbqO+N6cnMprpfsR64hnQdyOiXVzM+vs+YCB5YnZ/meKbtSB01cRJ?= =?us-ascii?Q?EuHktOua2QlSwrb5sgXKIHf4BI35ClKLKiqIbkqbKQQyBdlIIC6vqMFqRmwa?= =?us-ascii?Q?8fKwDGpRHXTc1ozxq9EMboi2Ma7uyU5PlMci+H3uJQpQ7sp/BwB7eLYnhNJW?= =?us-ascii?Q?1vdMURWCk7JZlKTjv4sY6R6O+10zXM5WgF36B9IBNxnN7kK3lwV41aaPrmGC?= =?us-ascii?Q?UEszHhEcjkTfIhjlgmIK/QbcLTzOhH5NMpUm69cA+qpmP7RotBMnD2Ti+5hN?= =?us-ascii?Q?DJR41RPAV6LdEfPplBs6W44V+ZeDnX/tihBPCFbLdesyJ2ECxWC12P2Yj04G?= =?us-ascii?Q?1VbkWQgkpviRsz6VnlKrHdBHIu72DRpFXp3WkbElbrf/Z9MacEtINiR4J8tk?= =?us-ascii?Q?J7ZxMEOZsx5TamIfDKgI8GMEQfzAKkvHniGCgsPMLEDaTLoqhtPTJR8XG0dE?= =?us-ascii?Q?HV9ZU24sN4alzlrdyLCfjA+kPsc4SYqoJvzizNL0rSpk6/MWCztLcq6S2h+e?= =?us-ascii?Q?SgK0ZwY4+4XOVDR8Og8zmWiDz8kI6ENGaQ5OaN9jqEIpsULMdAHewskDfQys?= =?us-ascii?Q?X46Pei1UQSqhuy6ejPOTZW8XzwN0/RKCs/l1iyVeRn0sflhQldpPbA2fm9wr?= =?us-ascii?Q?mG6xi7O5rWq0P7gckUrTrnJW8kLn+jyT67knWolt6wCObTLt1rugrQgWca77?= =?us-ascii?Q?Tt/gsOU1XN/EffWXtnZLqdRLwPnOPWzicrzHPjL2pYV1L4jinOx+MuV0HPQf?= =?us-ascii?Q?WT31N+I14f8IUW+o0SEtxd0j/wYjjA6B7tW7NuQRH2HAolP3fjo+K6Dir8Y5?= =?us-ascii?Q?ez7OzuaEjNDKdSh2x3o0KfvUv5q8k1eQ+eVzwZ6HUzA62RXXq48wnM06cOT4?= =?us-ascii?Q?jRi8ZOlRzt5C+C+7gbINx6hD3ZsgPK+GxsStNyBTkZMytpE6u4fgOvtPNJdt?= =?us-ascii?Q?dij6DbgSY7Jy3eSJlzgnmFeVbt0Avcnf0ILaYh09dlJyWcRQWsA7RyTPflUM?= =?us-ascii?Q?H2guVRdF+6kJNv30WRYMdgACYTrqGed+KE70ZYJofFdLlOTizWnTU8U2Q8FM?= =?us-ascii?Q?AUO/9znFY+wB5NYytSfVNvB1DncWBiX3VIiYhDGgP+GhczQ3GcPsUpX63VPV?= =?us-ascii?Q?7rx5l+eMcABG+IVaU6dTohDFYIH7mJvIqG1nzhTpOmu2MTKF01XfI46uVGdm?= =?us-ascii?Q?cDWwP2kRrJtXeem4CAdHgWiiutpFCJPluLrNWoDgQ6Y+zZwtTZbd2Po0bUwr?= =?us-ascii?Q?Od8bffgFej9AipTg0JNZQ6JE0Ih/wTQ+pkUDg3FRdux8RO12Cygej3IHUK+S?= =?us-ascii?Q?raDFL8vbo/leNJtZfURvBus7R4qRwdNPcbQmfqfk/6pCjXFh+rzm9b3yDhn1?= =?us-ascii?Q?+29PoKyTz5rYnVRzljXw5fZWTzoZfQtjATPy8ZQWvm2SqXHU++DmMHlVNP/J?= =?us-ascii?Q?/+zF8IgAJvOPT+anqJKW6iBaYnuDS5DPVRl5Is9RFFQ/NwFsvcX+KZetu5bC?= =?us-ascii?Q?iKI45lBmlR0UPz9xjnYdvtHZuETjowEfIocc/Ysx?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1631.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8c2a00ac-1091-496f-837b-08da867cc0b3 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Aug 2022 09:32:39.3247 (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: +iTUme3JYY5pcAgQyEnyW56nKuDm0ZYn5hYvrSe7hJynZxTPBSS3Z6iwGNNPl7xA6q+JTfmD4jsll9hP3LabKw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR11MB1625 Return-Path: ray.ni@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > + MpInitLibWhoAmI (&Index); > SwitchStackData =3D (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer; > - InitializeSeparateExceptionStacks (SwitchStackData->Buffer, > SwitchStackData->BufferSize); > + if ((SwitchStackData[Index].Status =3D=3D EFI_NOT_STARTED) || > (SwitchStackData[Index].Status =3D=3D EFI_BUFFER_TOO_SMALL)) { > + SwitchStackData[Index].Status =3D InitializeSeparateExceptionStacks > (SwitchStackData[Index].Buffer, &SwitchStackData[Index].BufferSize); > + } > } 1. Can you add comment to explain why BufferTooSmall is checked? I guess you want to handle the case when calls in some APs succeed while some calls in other APs get BufferTooSmall. So this check makes sure that the 2nd call happens only on those failed APs= . > + SwitchStackData =3D AllocateRuntimeZeroPool (mNumberOfProcessors * > sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)); 2. Why runtime type memory? > + if (SwitchStackData[Index].Status =3D=3D EFI_BUFFER_TOO_SMALL) { 3. Can you add assertion to make sure " SwitchStackData[Index].BufferSize != =3D 0"? > + SwitchStackData[Index].Buffer =3D (VOID *)(BufferSize + (UINTN)B= uffer); 4. Use "&Buffer[BufferSize]"; >=20 > - SwitchStackData.BufferSize =3D &BufferSize; > MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); > - MpInitLibWhoAmI (&Bsp); > + SwitchStackData =3D AllocateRuntimeZeroPool (NumberOfProcessors * > sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)); 5. No need for runtime memory. > + for (Index =3D 0; Index < NumberOfProcessors; ++Index) { > + if (SwitchStackData[Index].Status =3D=3D EFI_BUFFER_TOO_SMALL) { > + BufferSize +=3D SwitchStackData[Index].BufferSize; 6. add assertion to make sure ".BufferSize =3D=3D 0" when ".Status =3D=3D E= FI_SUCCESS". I think assertion here is better than my last comment which requests assert= ion in the other place. 7. Please use AllocatePages in PEI because in a many-core platform the pool= 64KB limitation may cause the code fail to allocate. Thanks, Ray