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 856EED804B8 for ; Wed, 6 Sep 2023 05:19:13 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=wuWCA7iYpMvH/EQ4Woh5DlbIbtXhd/Fo6IrHkSKZVD8=; 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:msip_labels: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; s=20140610; t=1693977552; v=1; b=YVH0LsF0tbaPs2gMzosNKSI6iNRBFVTAIoqgu6lDYh+jpVk+lDpQP6IqKdMMYLyzdPQomlOc WfHZysiw5qqSTNOF0zIvk0HUYeD+fbgtmW3FTMFhW0jMMA5CeIRlVw7f+z/YYq2Qu8kJR8d0MAT CYx7yd3i5aMkyyoEazhhF2hU= X-Received: by 127.0.0.2 with SMTP id mALoYY7687511xQn4koiGF1V; Tue, 05 Sep 2023 22:19:12 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web10.2027.1693977551478003857 for ; Tue, 05 Sep 2023 22:19:11 -0700 X-IronPort-AV: E=McAfee;i="6600,9927,10824"; a="463356023" X-IronPort-AV: E=Sophos;i="6.02,231,1688454000"; d="scan'208,217";a="463356023" X-Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Sep 2023 22:19:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10824"; a="770625467" X-IronPort-AV: E=Sophos;i="6.02,231,1688454000"; d="scan'208,217";a="770625467" X-Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga008.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 05 Sep 2023 22:19:10 -0700 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.32; Tue, 5 Sep 2023 22:19:09 -0700 X-Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) 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.27; Tue, 5 Sep 2023 22:19:09 -0700 X-Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) 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.27 via Frontend Transport; Tue, 5 Sep 2023 22:19:09 -0700 X-Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.102) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.27; Tue, 5 Sep 2023 22:19:08 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DGF49pdofwbBVhNYBmBtfYRgF0ZTP5J08xV3j50ywj/WxyQkg1eLc+SWqDgzdmKZOSlguP3LBVfnPnyI+rUB0CdAejZhurDB97zf1P0ohCUX+5LciRLgRKsqDXAMR4xBh2GsG/ftEQ1XhMBn7oFLkHXsY+j4MwWiW/Iy8WpyAEmzClJTBmdbEDDpQII8KVex3TBohWx1Nf/xqWxIQHUTN73ooDtZra8xwaEnIy365mVUsCr2BWvfWHODLQ2MT8iIaJnm403nxr7hSZMbshB9AXRhEZkPI1FZ4IKNtyOUHCvIU65127Vy/kr0K3zEJuj55tf27lay4gcW41G9nEIO+w== 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=Wscgkht835RJ7ZvAOqGy31BFoA99QcvxV4zdg8xi/dY=; b=bXtoORtdT79EH940YJqtvkclCdyUboFnMASlkGlVJKkaptYmEIotoUHww9RioFFeV0fQOObCJ6pO5iVy+1o3Fekde2cYaPHW0YJHpK8L+sJ9G+z37v2W0vKPxFX1deB88hC8EYJKvlpISyrwShRjxleS4G4W8QlOqxcDraQ3Y+eEXLRvYnQI6Nj+zq4QKaUbWh5782sUqf5jMFIKiV/HC/MmdKiy/oUYmt7h5qPr68NfeSQy8cLksFzceiNPuO77DUjMkFP7WGcVKW3b9Uj12dfMd5H+1oXnAl4Dds2tJW4ksU5ZQbtiBJ1b4cgp6sJbzf/I2ggNUd1jkDdiEtNJTw== 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 CY5PR11MB6281.namprd11.prod.outlook.com (2603:10b6:930:23::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6745.34; Wed, 6 Sep 2023 05:19:04 +0000 X-Received: from MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::bf9a:54ca:d270:59b]) by MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::bf9a:54ca:d270:59b%5]) with mapi id 15.20.6745.030; Wed, 6 Sep 2023 05:19:04 +0000 From: "Ni, Ray" To: "Tan, Dun" , "devel@edk2.groups.io" CC: "Dong, Eric" , "Kumar, Rahul R" Subject: Re: [edk2-devel] [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c Thread-Topic: [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c Thread-Index: AQHZ2xR9RHPYZEH+z0uPIp54MXr8CbANTRX/ Date: Wed, 6 Sep 2023 05:19:03 +0000 Message-ID: References: <20230830073418.586-1-dun.tan@intel.com> <20230830073418.586-2-dun.tan@intel.com> In-Reply-To: <20230830073418.586-2-dun.tan@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MN6PR11MB8244:EE_|CY5PR11MB6281:EE_ x-ms-office365-filtering-correlation-id: 18fca5a7-5f51-4305-3116-08dbae98c963 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: +KVEOnCJCLooHQFlnQ6Mc0eZgkFeufimeDg2wd6hmHjOnBJ6tug42WlygxRqj7B1sNjKq6rZ/DKi4BdOE6cz2itgri34r9I00GVLmP3YFCt/qswD/Bn4UyZwTRrIT9VJdHamDgxvUj8ieA5ud+5ApLblk8KrNOLfqKMRcQCa62H0rPVqLxESDUhifhgqLWwSYqa24TAqcc4hp5aUiBgWOkbF1HoSOyJpnZCVyGJOmWAcrHsaWksUPBD8OKIwq5dAzcrovIr/YwaBo98kIoVNo0Dcus+e9Jq7Wtz0ND0PTZX4MwtDsw0/fxIQ82SjAJKe+YpLN4fSC8IGNJzeJtl3h8dqHJsuPljOYbCucopWADWuPWx6mZT/9bfqtcalpXIgAfeWJHUhkfrkkH+goa617ZVVu52Ks+u+GNfV6Lu8Jq+E9TpR1u3hsyvGVVYz5MeYQhDfN4QjTjJYGc6/rZgP/VBjMb6I1d6iRr5nJNwSLpUAp2Ip2Xv24fWvwSOhppwT48DXiKyOwLSJ9gZShHSaMBJo9n5nYxfmZz7acqqZLsUjLfFASmVuhbZ4BlgORc/sAXEKosvQrdIytumt4qCPNwrju302C2iL1N70mFbUUj0/y4/ZdCXZL7lNcgeNUVTCpoxsBBDLYRtmQ9sCapyZUg== x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?yXFC+9dMgGrDHRrhxg4iUfZEZzTRFLNru44bPXkdUwLsAui4Cz5Rqjy1oqfO?= =?us-ascii?Q?eZU6gSNsnPO2KXRPRnPUZIKCXpgJawNIx2HPk0iK6d05JKhqI+/4ZRpJvlnv?= =?us-ascii?Q?ZYxee9mOywoOKWpenAenpwzAVl8EZX39nxvTzAQgMlHc93sBFOIgmJiBiHS9?= =?us-ascii?Q?7B0Dku/QtkPPYuxxDko6H7BcIlAlxaw5uu/k3l83DEZOU7jF3ogrm60t2JdT?= =?us-ascii?Q?8l1032tsQNYKkXF1F/FlHSUysUYGNfNd14Pftq/18M64VFwrgkXZxBqRZc0A?= =?us-ascii?Q?4M2efvtSSFWkTEB8ujd68pLKnxEQAioQR/k6lhqwsMptcFslm9yYYqu/Qdw2?= =?us-ascii?Q?9SXvvQbKBuL0i2QllfJv5PmnYkA+NZ7QH7yR4nonULJY9P6/tlRKoT4EIpCW?= =?us-ascii?Q?qc6EcF83VijMPLaUZXKmgxU2+gVH4FQfa1Q0gZE7sUyGRTuAwdLkBvmUa2L3?= =?us-ascii?Q?DZANWyY4MO1LfekTh74vXIf1uPM3x4X6GNmZfPi6AXHb3HRuSIztspXpGqKR?= =?us-ascii?Q?VLIY01RDNifnpbkyZxLaTBMG46nU2Jgd/Lh7l7r0/dxAiMs/PHgT+K94NKv9?= =?us-ascii?Q?vQy/00VzBc1G2zxl3hsgBS0nto3hJszS3o/dS3Us+wF51cyyQiXgMRr+vSq1?= =?us-ascii?Q?P5/wERT4W+oDi2Pfmouh1s7J1Ma/HgDVKCyAymacXL9LbNqj99czphOT3o+z?= =?us-ascii?Q?QTvQt//A384y7PC5IrZdQowAwUcyVBEQw12LmqNf6i50bQMbr/p/B7qVOKDJ?= =?us-ascii?Q?PG9P4Iqx7fnLy/OCvYJlp7XVlyjABYuupZbjTh4dZgM74DGaCW5V3G8DbYfW?= =?us-ascii?Q?VGHlnqSxN8t4/ypkhWXB9HD8ybnDF9sBm2bPDcERRDoCfVU5TSisXyg8vjZD?= =?us-ascii?Q?vBuwnZWEoMPOg96PJS9ksgU5zy4iHHC2dhC2BQErks6ujDp4F7GiKsdqk9SH?= =?us-ascii?Q?HHSRFC1La1CGTNv54A8QKDgDuB0/+fodKv1Bzjo7hYtt8znhotSQec1Bln6r?= =?us-ascii?Q?G7zOHJCj4efn/Bs3TJbF3PkB0LIBcc7Qa/fndoNUUcLm20ByJ+XcPwM41t/I?= =?us-ascii?Q?9E5JgfWi75wkQtfOsEhRXt1wwz0qHH3Qd9sCaNsejLkMd/z+0G553G+npR3S?= =?us-ascii?Q?gyAIpt8Y3t4bmQfpSWjtGF3in+i5d+moGuO6qMLW8w9RpMZWHs+upqdGhyoO?= =?us-ascii?Q?9BpW9QLDxgHIDA0f8twnUIC1WrHwe4UeaCx1rWUv5+V1nVR0di2Ioh63XE4D?= =?us-ascii?Q?EsvyrHjAMGcasOH3C0AJaILT/zCveHNT8/YuDacWveS2ba1fkKvu6IOCL8Z0?= =?us-ascii?Q?ewLSnsTgOgUY/1UKcY0Xg9KOjT/a83qiFMJGc26wnhIheFdLID3DWsdwZBOq?= =?us-ascii?Q?00/WksnhcBoYhC/bCeg2LHnD4BpKJP5jgc3z984Oc1WfyDFRJxkgcF+1W9G9?= =?us-ascii?Q?5bBfSBXaG3bSW/NEmeMAUr5iuPWjuBSZ3qtN8IoZVev2iXF+IyB1H2acpg0I?= =?us-ascii?Q?MyKsdzX8l59x87xma6HEbnUby3VyYAzguMpx/k4S18rvyiRzr8VdngKogt9b?= =?us-ascii?Q?tJLdRFXLTnfOY7Oyn2A=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: 18fca5a7-5f51-4305-3116-08dbae98c963 X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Sep 2023 05:19:03.9970 (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: a6rlA4ZSB+gD8PI3+1BlOtSxU614M4rT+u0DS+tXQCCT5zDtASbZ6QQ7GbLhNs2wTOrcdHKuoFQEqMbWuoIXhg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6281 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: Urm0hPBUfalwQwsceW1oJnpzx7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MN6PR11MB82440049143C1D07150EFE958CEFAMN6PR11MB8244namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=YVH0LsF0; 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}") --_000_MN6PR11MB82440049143C1D07150EFE958CEFAMN6PR11MB8244namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ray Ni Thanks, Ray ________________________________ From: Tan, Dun Sent: Wednesday, August 30, 2023 3:34 PM To: devel@edk2.groups.io Cc: Dong, Eric ; Ni, Ray ; Kumar, Ra= hul R Subject: [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for CpuS3.c This commit is code logic refinement for s3 boot flow in CpuS3.c. It doesn't change any code functionality. This commit implementes InitializeAp and InitializeBsp as a single function since they are doing almost the same thing. Then both BSP and AP will execute the same function InitializeCpuProcedure to do CPU initialization. This can make the code logic easier to understand. Signed-off-by: Dun Tan Cc: Eric Dong Cc: Ray Ni Cc: Rahul Kumar --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 266 ++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++----------------------------------------------= ---------------------------------------------------------------------------= ------- 1 file changed, 138 insertions(+), 128 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/= CpuS3.c index 0f7ee0372d..b9dbeaef87 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -539,44 +539,149 @@ SetRegister ( } /** - AP initialization before then after SMBASE relocation in the S3 boot pat= h. + The function is invoked before SMBASE relocation in S3 path to restores = CPU status. + + The function is invoked before SMBASE relocation in S3 path. It does fir= st time microcode load + and restores MTRRs for both BSP and APs. + + @param IsBsp The CPU this function executes on is BSP or not. + **/ VOID -InitializeAp ( - VOID +InitializeCpuBeforeRebase ( + IN BOOLEAN IsBsp ) { - UINTN TopOfStack; - UINT8 Stack[128]; - LoadMtrrData (mAcpiCpuData.MtrrTable); SetRegister (TRUE); + ProgramVirtualWireMode (); + if (!IsBsp) { + DisableLvtInterrupts (); + } + // // Count down the number with lock mechanism. // InterlockedDecrement (&mNumberToFinish); + if (IsBsp) { + // + // Bsp wait here till all AP finish the initialization before rebase + // + while (mNumberToFinish > 0) { + CpuPause (); + } + } +} + +/** + The function is invoked after SMBASE relocation in S3 path to restores C= PU status. + + The function is invoked after SMBASE relocation in S3 path. It restores = configuration according to + data saved by normal boot path for both BSP and APs. + + @param IsBsp The CPU this function executes on is BSP or not. + +**/ +VOID +InitializeCpuAfterRebase ( + IN BOOLEAN IsBsp + ) +{ + UINTN TopOfStack; + UINT8 Stack[128]; + + SetRegister (FALSE); + if (IsBsp) { + while (mNumberToFinish > 0) { + CpuPause (); + } + } else { + // + // Place AP into the safe code, count down the number with lock mechan= ism in the safe code. + // + TopOfStack =3D (UINTN)Stack + sizeof (Stack); + TopOfStack &=3D ~(UINTN)(CPU_STACK_ALIGNMENT - 1); + CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof= (mApHltLoopCodeTemplate)); + TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNum= berToFinish); + } +} + +/** + Cpu initialization procedure. + + @param[in,out] Buffer The pointer to private data buffer. + +**/ +VOID +EFIAPI +InitializeCpuProcedure ( + IN OUT VOID *Buffer + ) +{ + BOOLEAN IsBsp; + + IsBsp =3D (BOOLEAN)(mBspApicId =3D=3D GetApicId ()); + // - // Wait for BSP to signal SMM Base relocation done. + // Skip initialization if mAcpiCpuData is not valid // - while (!mInitApsAfterSmmBaseReloc) { - CpuPause (); + if (mAcpiCpuData.NumberOfCpus > 0) { + // + // First time microcode load and restore MTRRs + // + InitializeCpuBeforeRebase (IsBsp); } - ProgramVirtualWireMode (); - DisableLvtInterrupts (); + if (IsBsp) { + DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n", mSmmReloca= ted)); - SetRegister (FALSE); + // + // Check whether Smm Relocation is done or not. + // If not, will do the SmmBases Relocation here!!! + // + if (!mSmmRelocated) { + // + // Restore SMBASE for BSP and all APs + // + SmmRelocateBases (); + } else { + // + // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to exec= ute first SMI init. + // + ExecuteFirstSmiInit (); + } + } // - // Place AP into the safe code, count down the number with lock mechanis= m in the safe code. + // Skip initialization if mAcpiCpuData is not valid // - TopOfStack =3D (UINTN)Stack + sizeof (Stack); - TopOfStack &=3D ~(UINTN)(CPU_STACK_ALIGNMENT - 1); - CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (= mApHltLoopCodeTemplate)); - TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&mNumbe= rToFinish); + if (mAcpiCpuData.NumberOfCpus > 0) { + if (IsBsp) { + // + // mNumberToFinish should be set before AP executes InitializeCpuAft= erRebase() + // + mNumberToFinish =3D (UINT32)(mNumberOfCpus - 1); + // + // Signal that SMM base relocation is complete and to continue initi= alization for all APs. + // + mInitApsAfterSmmBaseReloc =3D TRUE; + } else { + // + // AP Wait for BSP to signal SMM Base relocation done. + // + while (!mInitApsAfterSmmBaseReloc) { + CpuPause (); + } + } + + // + // Restore MSRs for BSP and all APs + // + InitializeCpuAfterRebase (IsBsp); + } } /** @@ -627,91 +732,7 @@ PrepareApStartupVector ( mExchangeInfo->BufferStart =3D (UINT32)StartupVe= ctor; mExchangeInfo->Cr3 =3D (UINT32)(AsmReadC= r3 ()); mExchangeInfo->InitializeFloatingPointUnitsAddress =3D (UINTN)Initialize= FloatingPointUnits; -} - -/** - The function is invoked before SMBASE relocation in S3 path to restores = CPU status. - - The function is invoked before SMBASE relocation in S3 path. It does fir= st time microcode load - and restores MTRRs for both BSP and APs. - -**/ -VOID -InitializeCpuBeforeRebase ( - VOID - ) -{ - LoadMtrrData (mAcpiCpuData.MtrrTable); - - SetRegister (TRUE); - - ProgramVirtualWireMode (); - - PrepareApStartupVector (mAcpiCpuData.StartupVector); - - if (FeaturePcdGet (PcdCpuHotPlugSupport)) { - ASSERT (mNumberOfCpus <=3D mAcpiCpuData.NumberOfCpus); - } else { - ASSERT (mNumberOfCpus =3D=3D mAcpiCpuData.NumberOfCpus); - } - - mNumberToFinish =3D (UINT32)(mNumberOfCpus - 1); - mExchangeInfo->ApFunction =3D (VOID *)(UINTN)InitializeAp; - - // - // Execute code for before SmmBaseReloc. Note: This flag is maintained a= cross S3 boots. - // - mInitApsAfterSmmBaseReloc =3D FALSE; - - // - // Send INIT IPI - SIPI to all APs - // - SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); - - while (mNumberToFinish > 0) { - CpuPause (); - } -} - -/** - The function is invoked after SMBASE relocation in S3 path to restores C= PU status. - - The function is invoked after SMBASE relocation in S3 path. It restores = configuration according to - data saved by normal boot path for both BSP and APs. - -**/ -VOID -InitializeCpuAfterRebase ( - VOID - ) -{ - if (FeaturePcdGet (PcdCpuHotPlugSupport)) { - ASSERT (mNumberOfCpus <=3D mAcpiCpuData.NumberOfCpus); - } else { - ASSERT (mNumberOfCpus =3D=3D mAcpiCpuData.NumberOfCpus); - } - - mNumberToFinish =3D (UINT32)(mNumberOfCpus - 1); - - // - // Signal that SMM base relocation is complete and to continue initializ= ation for all APs. - // - mInitApsAfterSmmBaseReloc =3D TRUE; - - // - // Must begin set register after all APs have continue their initializat= ion. - // This is a requirement to support semaphore mechanism in register tabl= e. - // Because if semaphore's dependence type is package type, semaphore wil= l wait - // for all Aps in one package finishing their tasks before set next regi= ster - // for all APs. If the Aps not begin its task during BSP doing its task,= the - // BSP thread will hang because it is waiting for other Aps in the same - // package finishing their task. - // - SetRegister (FALSE); - - while (mNumberToFinish > 0) { - CpuPause (); - } + mExchangeInfo->ApFunction =3D (VOID *)(UINTN)In= itializeCpuProcedure; } /** @@ -813,44 +834,33 @@ SmmRestoreCpu ( InitializeDebugAgent (DEBUG_AGENT_INIT_THUNK_PEI_IA32TOX64, (VOID *)&I= a32Idtr, NULL); } + mBspApicId =3D GetApicId (); // - // Skip initialization if mAcpiCpuData is not valid + // Skip AP initialization if mAcpiCpuData is not valid // if (mAcpiCpuData.NumberOfCpus > 0) { - // - // First time microcode load and restore MTRRs - // - InitializeCpuBeforeRebase (); - } + if (FeaturePcdGet (PcdCpuHotPlugSupport)) { + ASSERT (mNumberOfCpus <=3D mAcpiCpuData.NumberOfCpus); + } else { + ASSERT (mNumberOfCpus =3D=3D mAcpiCpuData.NumberOfCpus); + } - DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n", mSmmRelocate= d)); + mNumberToFinish =3D (UINT32)mNumberOfCpus; - // - // Check whether Smm Relocation is done or not. - // If not, will do the SmmBases Relocation here!!! - // - if (!mSmmRelocated) { // - // Restore SMBASE for BSP and all APs + // Execute code for before SmmBaseReloc. Note: This flag is maintained= across S3 boots. // - SmmRelocateBases (); - } else { - // - // Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) to execut= e first SMI init. - // - ExecuteFirstSmiInit (); - } + mInitApsAfterSmmBaseReloc =3D FALSE; - // - // Skip initialization if mAcpiCpuData is not valid - // - if (mAcpiCpuData.NumberOfCpus > 0) { + PrepareApStartupVector (mAcpiCpuData.StartupVector); // - // Restore MSRs for BSP and all APs + // Send INIT IPI - SIPI to all APs // - InitializeCpuAfterRebase (); + SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVector); } + InitializeCpuProcedure (NULL); + // // Set a flag to restore SMM configuration in S3 path. // -- 2.31.1.windows.1 -=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 (#108303): https://edk2.groups.io/g/devel/message/108303 Mute This Topic: https://groups.io/mt/101047981/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- --_000_MN6PR11MB82440049143C1D07150EFE958CEFAMN6PR11MB8244namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
Reviewed= -by: Ray Ni <ray.ni@intel.com>



Thanks,
Ray
From: Tan, Dun <dun.tan@= intel.com>
Sent: Wednesday, August 30, 2023 3:34 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel= .com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
Subject: [Patch V3 4/5] UefiCpuPkg/PiSmmCpuDxe: code refinement for = CpuS3.c
 
This commit is code logic refinement for s3 boot f= low
in CpuS3.c. It doesn't change any code functionality.
This commit implementes InitializeAp and InitializeBsp
as a single function since they are doing almost the
same thing. Then both BSP and AP will execute the same
function InitializeCpuProcedure to do CPU initialization.
This can make the code logic easier to understand.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 266 +++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++++++++++++++++++++++++++++-----------------------------------------= ---------------------------------------------------------------------------= ------------
 1 file changed, 138 insertions(+), 128 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/= CpuS3.c
index 0f7ee0372d..b9dbeaef87 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -539,44 +539,149 @@ SetRegister (
 }
 
 /**
-  AP initialization before then after SMBASE relocation in the S3 boo= t path.
+  The function is invoked before SMBASE relocation in S3 path to rest= ores CPU status.
+
+  The function is invoked before SMBASE relocation in S3 path. It doe= s first time microcode load
+  and restores MTRRs for both BSP and APs.
+
+  @param   IsBsp   The CPU this function executes= on is BSP or not.
+
 **/
 VOID
-InitializeAp (
-  VOID
+InitializeCpuBeforeRebase (
+  IN BOOLEAN  IsBsp
   )
 {
-  UINTN  TopOfStack;
-  UINT8  Stack[128];
-
   LoadMtrrData (mAcpiCpuData.MtrrTable);
 
   SetRegister (TRUE);
 
+  ProgramVirtualWireMode ();
+  if (!IsBsp) {
+    DisableLvtInterrupts ();
+  }
+
   //
   // Count down the number with lock mechanism.
   //
   InterlockedDecrement (&mNumberToFinish);
 
+  if (IsBsp) {
+    //
+    // Bsp wait here till all AP finish the initialization = before rebase
+    //
+    while (mNumberToFinish > 0) {
+      CpuPause ();
+    }
+  }
+}
+
+/**
+  The function is invoked after SMBASE relocation in S3 path to resto= res CPU status.
+
+  The function is invoked after SMBASE relocation in S3 path. It rest= ores configuration according to
+  data saved by normal boot path for both BSP and APs.
+
+  @param   IsBsp   The CPU this function executes= on is BSP or not.
+
+**/
+VOID
+InitializeCpuAfterRebase (
+  IN BOOLEAN  IsBsp
+  )
+{
+  UINTN  TopOfStack;
+  UINT8  Stack[128];
+
+  SetRegister (FALSE);
+  if (IsBsp) {
+    while (mNumberToFinish > 0) {
+      CpuPause ();
+    }
+  } else {
+    //
+    // Place AP into the safe code, count down the number w= ith lock mechanism in the safe code.
+    //
+    TopOfStack  =3D (UINTN)Stack + sizeof (Stack);
+    TopOfStack &=3D ~(UINTN)(CPU_STACK_ALIGNMENT - 1);<= br> +    CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeT= emplate, sizeof (mApHltLoopCodeTemplate));
+    TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStac= k, (UINTN)&mNumberToFinish);
+  }
+}
+
+/**
+  Cpu initialization procedure.
+
+  @param[in,out] Buffer  The pointer to private data buffer.
+
+**/
+VOID
+EFIAPI
+InitializeCpuProcedure (
+  IN OUT VOID  *Buffer
+  )
+{
+  BOOLEAN  IsBsp;
+
+  IsBsp =3D  (BOOLEAN)(mBspApicId =3D=3D GetApicId ());
+
   //
-  // Wait for BSP to signal SMM Base relocation done.
+  // Skip initialization if mAcpiCpuData is not valid
   //
-  while (!mInitApsAfterSmmBaseReloc) {
-    CpuPause ();
+  if (mAcpiCpuData.NumberOfCpus > 0) {
+    //
+    // First time microcode load and restore MTRRs
+    //
+    InitializeCpuBeforeRebase (IsBsp);
   }
 
-  ProgramVirtualWireMode ();
-  DisableLvtInterrupts ();
+  if (IsBsp) {
+    DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated = is %d\n", mSmmRelocated));
 
-  SetRegister (FALSE);
+    //
+    // Check whether Smm Relocation is done or not.
+    // If not, will do the SmmBases Relocation here!!!
+    //
+    if (!mSmmRelocated) {
+      //
+      // Restore SMBASE for BSP and all APs
+      //
+      SmmRelocateBases ();
+    } else {
+      //
+      // Issue SMI IPI (All Excluding  Self = SMM IPI + BSP SMM IPI) to execute first SMI init.
+      //
+      ExecuteFirstSmiInit ();
+    }
+  }
 
   //
-  // Place AP into the safe code, count down the number with lock mec= hanism in the safe code.
+  // Skip initialization if mAcpiCpuData is not valid
   //
-  TopOfStack  =3D (UINTN)Stack + sizeof (Stack);
-  TopOfStack &=3D ~(UINTN)(CPU_STACK_ALIGNMENT - 1);
-  CopyMem ((VOID *)(UINTN)mApHltLoopCode, mApHltLoopCodeTemplate, siz= eof (mApHltLoopCodeTemplate));
-  TransferApToSafeState ((UINTN)mApHltLoopCode, TopOfStack, (UINTN)&a= mp;mNumberToFinish);
+  if (mAcpiCpuData.NumberOfCpus > 0) {
+    if (IsBsp) {
+      //
+      // mNumberToFinish should be set before AP = executes InitializeCpuAfterRebase()
+      //
+      mNumberToFinish =3D (UINT32)(mNumberOfCpus = - 1);
+      //
+      // Signal that SMM base relocation is compl= ete and to continue initialization for all APs.
+      //
+      mInitApsAfterSmmBaseReloc =3D TRUE;
+    } else {
+      //
+      // AP Wait for BSP to signal SMM Base reloc= ation done.
+      //
+      while (!mInitApsAfterSmmBaseReloc) {
+        CpuPause ();
+      }
+    }
+
+    //
+    // Restore MSRs for BSP and all APs
+    //
+    InitializeCpuAfterRebase (IsBsp);
+  }
 }
 
 /**
@@ -627,91 +732,7 @@ PrepareApStartupVector (
   mExchangeInfo->BufferStart     &nb= sp;            =        =3D (UINT32)StartupVector;
   mExchangeInfo->Cr3       = ;            &n= bsp;            = ; =3D (UINT32)(AsmReadCr3 ());
   mExchangeInfo->InitializeFloatingPointUnitsAddress =3D (UIN= TN)InitializeFloatingPointUnits;
-}
-
-/**
-  The function is invoked before SMBASE relocation in S3 path to rest= ores CPU status.
-
-  The function is invoked before SMBASE relocation in S3 path. It doe= s first time microcode load
-  and restores MTRRs for both BSP and APs.
-
-**/
-VOID
-InitializeCpuBeforeRebase (
-  VOID
-  )
-{
-  LoadMtrrData (mAcpiCpuData.MtrrTable);
-
-  SetRegister (TRUE);
-
-  ProgramVirtualWireMode ();
-
-  PrepareApStartupVector (mAcpiCpuData.StartupVector);
-
-  if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
-    ASSERT (mNumberOfCpus <=3D mAcpiCpuData.NumberOfCpus= );
-  } else {
-    ASSERT (mNumberOfCpus =3D=3D mAcpiCpuData.NumberOfCpus)= ;
-  }
-
-  mNumberToFinish        &nbs= p;  =3D (UINT32)(mNumberOfCpus - 1);
-  mExchangeInfo->ApFunction =3D (VOID *)(UINTN)InitializeAp;
-
-  //
-  // Execute code for before SmmBaseReloc. Note: This flag is maintai= ned across S3 boots.
-  //
-  mInitApsAfterSmmBaseReloc =3D FALSE;
-
-  //
-  // Send INIT IPI - SIPI to all APs
-  //
-  SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.StartupVecto= r);
-
-  while (mNumberToFinish > 0) {
-    CpuPause ();
-  }
-}
-
-/**
-  The function is invoked after SMBASE relocation in S3 path to resto= res CPU status.
-
-  The function is invoked after SMBASE relocation in S3 path. It rest= ores configuration according to
-  data saved by normal boot path for both BSP and APs.
-
-**/
-VOID
-InitializeCpuAfterRebase (
-  VOID
-  )
-{
-  if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
-    ASSERT (mNumberOfCpus <=3D mAcpiCpuData.NumberOfCpus= );
-  } else {
-    ASSERT (mNumberOfCpus =3D=3D mAcpiCpuData.NumberOfCpus)= ;
-  }
-
-  mNumberToFinish =3D (UINT32)(mNumberOfCpus - 1);
-
-  //
-  // Signal that SMM base relocation is complete and to continue init= ialization for all APs.
-  //
-  mInitApsAfterSmmBaseReloc =3D TRUE;
-
-  //
-  // Must begin set register after all APs have continue their initia= lization.
-  // This is a requirement to support semaphore mechanism in register= table.
-  // Because if semaphore's dependence type is package type, semaphor= e will wait
-  // for all Aps in one package finishing their tasks before set next= register
-  // for all APs. If the Aps not begin its task during BSP doing its = task, the
-  // BSP thread will hang because it is waiting for other Aps in the = same
-  // package finishing their task.
-  //
-  SetRegister (FALSE);
-
-  while (mNumberToFinish > 0) {
-    CpuPause ();
-  }
+  mExchangeInfo->ApFunction      &nb= sp;            =        =3D (VOID *)(UINTN)InitializeCpuProced= ure;
 }
 
 /**
@@ -813,44 +834,33 @@ SmmRestoreCpu (
     InitializeDebugAgent (DEBUG_AGENT_INIT_THUNK_PEI_I= A32TOX64, (VOID *)&Ia32Idtr, NULL);
   }
 
+  mBspApicId =3D GetApicId ();
   //
-  // Skip initialization if mAcpiCpuData is not valid
+  // Skip AP initialization if mAcpiCpuData is not valid
   //
   if (mAcpiCpuData.NumberOfCpus > 0) {
-    //
-    // First time microcode load and restore MTRRs
-    //
-    InitializeCpuBeforeRebase ();
-  }
+    if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
+      ASSERT (mNumberOfCpus <=3D mAcpiCpuData.= NumberOfCpus);
+    } else {
+      ASSERT (mNumberOfCpus =3D=3D mAcpiCpuData.N= umberOfCpus);
+    }
 
-  DEBUG ((DEBUG_INFO, "SmmRestoreCpu: mSmmRelocated is %d\n"= ;, mSmmRelocated));
+    mNumberToFinish =3D (UINT32)mNumberOfCpus;
 
-  //
-  // Check whether Smm Relocation is done or not.
-  // If not, will do the SmmBases Relocation here!!!
-  //
-  if (!mSmmRelocated) {
     //
-    // Restore SMBASE for BSP and all APs
+    // Execute code for before SmmBaseReloc. Note: This fla= g is maintained across S3 boots.
     //
-    SmmRelocateBases ();
-  } else {
-    //
-    // Issue SMI IPI (All Excluding  Self SMM IPI + BS= P SMM IPI) to execute first SMI init.
-    //
-    ExecuteFirstSmiInit ();
-  }
+    mInitApsAfterSmmBaseReloc =3D FALSE;
 
-  //
-  // Skip initialization if mAcpiCpuData is not valid
-  //
-  if (mAcpiCpuData.NumberOfCpus > 0) {
+    PrepareApStartupVector (mAcpiCpuData.StartupVector);      //
-    // Restore MSRs for BSP and all APs
+    // Send INIT IPI - SIPI to all APs
     //
-    InitializeCpuAfterRebase ();
+    SendInitSipiSipiAllExcludingSelf ((UINT32)mAcpiCpuData.= StartupVector);
   }
 
+  InitializeCpuProcedure (NULL);
+
   //
   // Set a flag to restore SMM configuration in S3 path.
   //
--
2.31.1.windows.1

_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#108303) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--_000_MN6PR11MB82440049143C1D07150EFE958CEFAMN6PR11MB8244namp_--