From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 8CCBF7803CC for ; Thu, 18 Apr 2024 07:40:12 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=zNQxtlEDoNEUKrS6hCTSf2OiMLEJabC5y+B9MkJ1XJw=; c=relaxed/simple; d=groups.io; h=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:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type; s=20240206; t=1713426011; v=1; b=CrUXuuNNx3R02T6IA8Yd7PsqNz/oWHHy0BRD3GEAznnmQemdZhiHdEzLiqSLioWaQag/Q05g zONi08PWe/trSMPVKX9wElBUxQabrL0bRXuCnvgFr9AeokNQMvgRwpLAxx3zZychUBHhaHZzgs0 CZNGSirU6Q0vPmuuywHpg3dSszGYf/b8QDc+y1UwrYZbsL3cn88aURSqBN8LSR2CGtveAG4RQSK DFl8w2rynqUV1a3XwWs/s3GPFJ+YgdWrKjweclm23z2XXD66NPtkTQv39IiCBu1PC/TTVUo9nhX zSBJFf9sEw+/wMoVfmEhXMWaQsHRU3KCBEfuLYef3v8hA== X-Received: by 127.0.0.2 with SMTP id vuHrYY7687511xEMuUAwt1Xo; Thu, 18 Apr 2024 00:40:11 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) by mx.groups.io with SMTP id smtpd.web11.7476.1713426010394191436 for ; Thu, 18 Apr 2024 00:40:10 -0700 X-CSE-ConnectionGUID: pg27ZEgkQnSayCF39P16BA== X-CSE-MsgGUID: 7rNBsAlrR8mTS9XpZjSCTw== X-IronPort-AV: E=McAfee;i="6600,9927,11047"; a="12738694" X-IronPort-AV: E=Sophos;i="6.07,211,1708416000"; d="scan'208,217";a="12738694" X-Received: from orviesa009.jf.intel.com ([10.64.159.149]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Apr 2024 00:40:10 -0700 X-CSE-ConnectionGUID: tCIiGX1kTCKyVHLPngbJIg== X-CSE-MsgGUID: 4ylHyA+vTzSghQvPvxrl1g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,211,1708416000"; d="scan'208,217";a="22892140" X-Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by orviesa009.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 18 Apr 2024 00:40:10 -0700 X-Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 18 Apr 2024 00:40:09 -0700 X-Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Thu, 18 Apr 2024 00:40:09 -0700 X-Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.40) 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.35; Thu, 18 Apr 2024 00:40:09 -0700 X-Received: from MN6PR11MB8244.namprd11.prod.outlook.com (2603:10b6:208:470::14) by SN7PR11MB7113.namprd11.prod.outlook.com (2603:10b6:806:298::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7472.31; Thu, 18 Apr 2024 07:40:06 +0000 X-Received: from MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::8774:81a7:c5b7:5c2c]) by MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::8774:81a7:c5b7:5c2c%7]) with mapi id 15.20.7472.037; Thu, 18 Apr 2024 07:40:06 +0000 From: "Ni, Ray" To: "Wu, Jiaxin" , "devel@edk2.groups.io" CC: "Zeng, Star" , Gerd Hoffmann , "Kumar, Rahul R" Subject: Re: [edk2-devel] [PATCH v3 04/13] UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation Thread-Topic: [PATCH v3 04/13] UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation Thread-Index: AQHakV2KkZzL1HA2p0eMceaag6kHNbFtpHCj Date: Thu, 18 Apr 2024 07:40:06 +0000 Message-ID: References: <20240418065556.5696-1-jiaxin.wu@intel.com> <20240418065556.5696-5-jiaxin.wu@intel.com> In-Reply-To: <20240418065556.5696-5-jiaxin.wu@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_|SN7PR11MB7113:EE_ x-ms-office365-filtering-correlation-id: b2feeacb-44ff-47a4-e861-08dc5f7ac47c x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: v1eiLwAJD/arJU69h2t8R7PtPWlSyXy6fJCCAZ0jmESkHWMdqG9xVOX0Qqx1i7xarlhLb22CtZAxjYuW2DMKrPtHUkdLYuB2I+C9FuY33RTRNED1zIHcdTyuFZZvdt2gT0AMLuukNT80sALvpKWN4vzNNcqQBPnW3ow46DtRCdyw4oZTGXuBvv7lKleuAT0pEREdsJB7MdFJWBHJIiI78eV9RoJaqwP4uNkNRrcQi+w1HIE5DOGG1skUcTw5Mo3dN0QBSZa7xsuvz/+xIv7WiqzjO/W6DDOOs+7OM9NMRtdSdzGiEMRaAqrCvtZNYaH6+auz+wwtR0DpVzy4gm3ADEW0V4QCDztLza5xrTSoJyWHb9DS8L0ixhYuvbBKz3gsrxYPKmmkIa1e/hOWBsx0ZhYTCs3mjiYNcru0Awk97d1OTWvR7m5Zf9tABfNL52dGfotxMuwIJtUDN4j72ng2Df7v/HW2euJCHMY+0VFE0uKcca1grcv7UOcIIHstsYM+DPVyP2I/zVFZHQdawIrIudX68msbp8U7XX7PUrogO2euvldu3thXqLETr3FxiJ3bSevQvIlCdxh4z5a8m7SIx+Zr63d1//oaxnkDbZIJgC6fNf3/U+XoGxDWJdHta+WZ179OkS+bFTuDRQdjfvC9GQOk7z1iFtFtKMv0XKtIo4LxMjvZdp2yLQ9siwWQYS6iEoCv3KVkK177ZNJ4S3av3w== x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?dcy3jfGzs5kXrk2oRUcb1yOy06cA+5OX6cP7OvSZJmIxWpWPRj3eMjN420oU?= =?us-ascii?Q?az4aC7w/u0FYnBTWrIxA12VohW5RsTCHIOOsvPzYjEHJq23lx5aDkBtXov1W?= =?us-ascii?Q?nSG+ST9Wbj+vYSUtfCS0ytOzDlXeEeQMSlWH7gCU1VZLJpIMj0Z814bJm4wQ?= =?us-ascii?Q?srpf5erfPDLwcGpB4CrPB+lTJm9Gkz7p3KToF2EKwWQUsSrLBeAszyfIlFmF?= =?us-ascii?Q?5Zo54wxCQzzwVO0AQUehn2pgIWCk5t015QPhBGYCdhNFua9Yn8qHevti2wsa?= =?us-ascii?Q?Q2FGJAPqOEUC/f4ZCQSGche1Y3/LR0UzMrIjbVpRb6CY8fnCstRZbyx+kGNH?= =?us-ascii?Q?lYeVZ3QF83UuCZtGQmDtwYuI98bpQUA/jpdMM9Cy1TBRxVPO15OKryIJT6+z?= =?us-ascii?Q?Rz/TcJVb4kTW8z87Jl98vYRcln/62SCSRL61B1yqoWqr0WLOcpvuoaYYaAW1?= =?us-ascii?Q?z3mz+d2ldGaN/Yu/OgvBCaJhajX/MYzAtPHHT1O2vdrMQgSy/y4aXqAXlyNt?= =?us-ascii?Q?07J3yV0DNOXSRrH2xaPee4+ul+OQckAZE97atOxRdQU65emd8vPwxGoXmz4U?= =?us-ascii?Q?6zNWX4RqFJClnQ5uaOEEgTEKlWvhLzEeFfbYbYjTbFlT+NTh6UcRNXWOVb4C?= =?us-ascii?Q?quHIiIxbCUocPcBPy/ap0XVSEZBSmDOdjSM83FqRVNl9X+s4G5W4HIpAGfbK?= =?us-ascii?Q?nhy5V3T8wsmhNaNjl2MxNk+QN9NY0d7nqQAZn0A39SzeIJQl0UE7Ew2d7ZFT?= =?us-ascii?Q?/0mWemyBlAQb/42DuGN3Ri9wN/d3J4OjIpfKi43pArCfSu2C8qifgvbgNRPh?= =?us-ascii?Q?p+u/513twCX5zPj3Zbo2TXJFD+bKDhwSn4fNue6oQxJHUVrJOYPIJGtSE1fc?= =?us-ascii?Q?QkXzgBAhV6KT06FwJGGfwHAqxcdbKE+WbY8MRySRUKpXwPoHbfQP2YgDKykR?= =?us-ascii?Q?iorbxxkMLY1nFhd88p7Y7wrudlKkAFGhjND8uS2zHHsOJzaFqJVYqQaP5W+G?= =?us-ascii?Q?Kl3d5mD08WX9srBsC9vGJox8OW3RJkJbF9yRZX1pX5RbHy1VtGP4buv+CmTl?= =?us-ascii?Q?OVF66e9LM06Xv9+V95buaFAwuLkmq8CAcePQ+yaAmpZgeaKcOeC/fnb7XKmp?= =?us-ascii?Q?9VXNJjH4QNDOOJnyIx6JNNQxP0KINHgRsQeMW6kwjvmLfznjkxXcpDbhYhik?= =?us-ascii?Q?3V1vkJbVSTKuMNmNdLgE3lQ8AVlcYZsYQsZVTAYBWZ6l9XpS5JVU6GxrMg9l?= =?us-ascii?Q?29uChMBer2ebff4FHrgyqkz/eQeB6DaRnRoDJbYXrtrdtZDzG/xA6NCHWaf+?= =?us-ascii?Q?w5Cr/fGVRTpZuYTD76/JNdwICQH+6crLzWuRq+1zhMKkRiaYh3V55yGcUGb6?= =?us-ascii?Q?3d29mz1lY8CRDfGNkBWrsH6XceVtsm4K5LjWhRF5YcAntuVsfY30t71mMzNB?= =?us-ascii?Q?XNaIm1mf7UJ/14jKPNzTeBtAhE4zrXJKptv5qDAbnOmiEis+MXLwhlrBm/zO?= =?us-ascii?Q?Pee0ViZayBJtN6HVEs9lb7gnjStRSn2cyYT7kwx/IyEF88bu7XlnsJFIvHnL?= =?us-ascii?Q?PsSME6TrNCwO+/WHGVI=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: b2feeacb-44ff-47a4-e861-08dc5f7ac47c X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2024 07:40:06.6708 (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: PimDz5a46fCUnO9gNX6CTHpd5po9ot5QZotznuzwqcJOxg7TCkWFb25KAXM/IEpwETjDeoPhLvO5BF/mJXVQdg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7113 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 Resent-Date: Thu, 18 Apr 2024 00:40:10 -0700 Resent-From: ray.ni@intel.com Reply-To: devel@edk2.groups.io,ray.ni@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: 9eXOowOJEaGjDWVo01VZ9G96x7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MN6PR11MB82448F44B42A7FAEA9ED71848C0E2MN6PR11MB8244namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=CrUXuuNN; 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 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io --_000_MN6PR11MB82448F44B42A7FAEA9ED71848C0E2MN6PR11MB8244namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ray Ni Thanks, Ray ________________________________ From: Wu, Jiaxin Sent: Thursday, April 18, 2024 14:55 To: devel@edk2.groups.io Cc: Ni, Ray ; Zeng, Star ; Gerd Hoff= mann ; Kumar, Rahul R Subject: [PATCH v3 04/13] UefiCpuPkg/SmmRelocationLib: Avoid unnecessary me= mory allocation Since SMM relocation is performed serially for each CPU, there is no need to allocate buffers for all CPUs to store the SmBase address in mSmBase and the Rebased flag in mRebased. A defined global variable is sufficient. This patch focuses on the mSmBase and mRebased global variables to prevent unnecessary memory allocation for these variables. Cc: Ray Ni Cc: Zeng Star Cc: Gerd Hoffmann Cc: Rahul Kumar Signed-off-by: Jiaxin Wu --- .../Library/SmmRelocationLib/SmmRelocationLib.c | 201 +++++++++--------= ---- 1 file changed, 90 insertions(+), 111 deletions(-) diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c b/UefiC= puPkg/Library/SmmRelocationLib/SmmRelocationLib.c index ca98f06a05..3694a07cbb 100644 --- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c +++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c @@ -25,31 +25,57 @@ EFI_PROCESSOR_INFORMATION *mProcessorInfo =3D NULL; // IDT used during SMM Init // IA32_DESCRIPTOR gcSmmInitIdtr; // -// Smbase for all CPUs +// Smbase for current CPU // -UINT64 *mSmBase =3D NULL; +UINT64 mSmBase; // -// SmBase Rebased flag for all CPUs +// SmBase Rebased flag for current CPU // -volatile BOOLEAN *mRebased; +volatile BOOLEAN mRebased; + +/** + This function will get the SmBase for CpuIndex. + + @param[in] CpuIndex The processor index. + @param[in] SmmRelocationStart The start address of Smm relocated memo= ry in SMRAM. + @param[in] TileSize The total size required for a CPU save = state, any + additional CPU-specific context and the= size of code + for the SMI entry point. + + @retval The value of SmBase for CpuIndex. + +**/ +UINTN +GetSmBase ( + IN UINTN CpuIndex, + IN EFI_PHYSICAL_ADDRESS SmmRelocationStart, + IN UINTN TileSize + ) +{ + return (UINTN)(SmmRelocationStart) + CpuIndex * TileSize - SMM_HANDLER_O= FFSET; +} /** This function will create SmBase for all CPUs. - @param[in] SmBase Pointer to SmBase for all CPUs. + @param[in] SmmRelocationStart The start address of Smm relocated memo= ry in SMRAM. + @param[in] TileSize The total size required for a CPU save = state, any + additional CPU-specific context and the= size of code + for the SMI entry point. @retval EFI_SUCCESS Create SmBase for all CPUs successfully. @retval Others Failed to create SmBase for all CPUs. **/ EFI_STATUS CreateSmmBaseHob ( - IN UINT64 *SmBase + IN EFI_PHYSICAL_ADDRESS SmmRelocationStart, + IN UINTN TileSize ) { UINTN Index; SMM_BASE_HOB_DATA *SmmBaseHobData; UINT32 CpuCount; @@ -90,11 +116,11 @@ CreateSmmBaseHob ( DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - SmmBaseHobData[%d]->NumberOfPr= ocessors: %d\n", HobCount, SmmBaseHobData->NumberOfProcessors)); for (Index =3D 0; Index < SmmBaseHobData->NumberOfProcessors; Index++)= { // // Calculate the new SMBASE address // - SmmBaseHobData->SmBase[Index] =3D SmBase[Index + CpuCount]; + SmmBaseHobData->SmBase[Index] =3D GetSmBase (Index + CpuCount, SmmRe= locationStart, TileSize); DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - SmmBaseHobData[%d]->SmBase[%= d]: 0x%08x\n", HobCount, Index, SmmBaseHobData->SmBase[Index])); } CpuCount +=3D NumberOfProcessorsInHob; HobCount++; @@ -126,18 +152,18 @@ SmmInitHandler ( for (Index =3D 0; Index < mNumberOfCpus; Index++) { if (ApicId =3D=3D (UINT32)mProcessorInfo[Index].ProcessorId) { // // Configure SmBase. // - ConfigureSmBase (mSmBase[Index]); + ConfigureSmBase (mSmBase); // // Hook return after RSM to set SMM re-based flag // SMM re-based flag can't be set before RSM, because SMM save state= context might be override // by next AP flow before it take effect. // - SemaphoreHook (Index, &mRebased[Index]); + SemaphoreHook (Index, &mRebased); return; } } ASSERT (FALSE); @@ -145,14 +171,22 @@ SmmInitHandler ( /** Relocate SmmBases for each processor. Execute on first boot and all S3 resumes + @param[in] MpServices2 Pointer to this instance of the MpServi= ces. + @param[in] SmmRelocationStart The start address of Smm relocated memo= ry in SMRAM. + @param[in] TileSize The total size required for a CPU save = state, any + additional CPU-specific context and the= size of code + for the SMI entry point. + **/ VOID SmmRelocateBases ( - VOID + IN EDKII_PEI_MP_SERVICES2_PPI *MpServices2, + IN EFI_PHYSICAL_ADDRESS SmmRelocationStart, + IN UINTN TileSize ) { UINT8 BakBuf[BACK_BUF_SIZE]; SMRAM_SAVE_STATE_MAP BakBuf2; SMRAM_SAVE_STATE_MAP *CpuStatePtr; @@ -196,17 +230,18 @@ SmmRelocateBases ( // Relocate SM bases for all APs // This is APs' 1st SMI - rebase will be done here, and APs' default SMI= handler will be overridden by gcSmmInitTemplate // BspIndex =3D (UINTN)-1; for (Index =3D 0; Index < mNumberOfCpus; Index++) { - mRebased[Index] =3D FALSE; if (BspApicId !=3D (UINT32)mProcessorInfo[Index].ProcessorId) { + mRebased =3D FALSE; + mSmBase =3D GetSmBase (Index, SmmRelocationStart, TileSize); SendSmiIpi ((UINT32)mProcessorInfo[Index].ProcessorId); // // Wait for this AP to finish its 1st SMI // - while (!mRebased[Index]) { + while (!mRebased) { } } else { // // BSP will be Relocated later // @@ -216,16 +251,18 @@ SmmRelocateBases ( // // Relocate BSP's SMM base // ASSERT (BspIndex !=3D (UINTN)-1); + mRebased =3D FALSE; + mSmBase =3D GetSmBase (BspIndex, SmmRelocationStart, TileSize); SendSmiIpi (BspApicId); // // Wait for the BSP to finish its 1st SMI // - while (!mRebased[BspIndex]) { + while (!mRebased) { } // // Restore contents at address 0x38000 // @@ -382,87 +419,10 @@ SplitSmramHobForSmmRelocation ( ZeroMem (&GuidHob->Name, sizeof (GuidHob->Name)); return EFI_SUCCESS; } -/** - This function will initialize SmBase for all CPUs. - - @param[in,out] SmBase Pointer to SmBase for all CPUs. - - @retval EFI_SUCCESS Initialize SmBase for all CPUs successfull= y. - @retval Others Failed to initialize SmBase for all CPUs. - -**/ -EFI_STATUS -InitSmBaseForAllCpus ( - IN OUT UINT64 **SmBase - ) -{ - EFI_STATUS Status; - UINTN TileSize; - UINT64 SmmRelocationSize; - EFI_PHYSICAL_ADDRESS SmmRelocationStart; - UINTN Index; - - SmmRelocationStart =3D 0; - - ASSERT (SmBase !=3D NULL); - - // - // Calculate SmmRelocationSize for all of the tiles. - // - // The CPU save state and code for the SMI entry point are tiled within = an SMRAM - // allocated buffer. The minimum size of this buffer for a uniprocessor = system - // is 32 KB, because the entry point is SMBASE + 32KB, and CPU save stat= e area - // just below SMBASE + 64KB. If more than one CPU is present in the plat= form, - // then the SMI entry point and the CPU save state areas can be tiles to= minimize - // the total amount SMRAM required for all the CPUs. The tile size can b= e computed - // by adding the CPU save state size, any extra CPU specific context, an= d - // the size of code that must be placed at the SMI entry point to transf= er - // control to a C function in the native SMM execution mode. This size i= s - // rounded up to the nearest power of 2 to give the tile size for a each= CPU. - // The total amount of memory required is the maximum number of CPUs tha= t - // platform supports times the tile size. - // - TileSize =3D SIZE_8KB; - SmmRelocationSize =3D EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (SIZE_32KB + = TileSize * (mMaxNumberOfCpus - 1))); - - // - // Split SmramReserve HOB to reserve SmmRelocationSize for Smm relocated= memory - // - Status =3D SplitSmramHobForSmmRelocation ( - SmmRelocationSize, - &SmmRelocationStart - ); - if (EFI_ERROR (Status)) { - return Status; - } - - ASSERT (SmmRelocationStart !=3D 0); - DEBUG ((DEBUG_INFO, "InitSmBaseForAllCpus - SmmRelocationSize: 0x%08x\n"= , SmmRelocationSize)); - DEBUG ((DEBUG_INFO, "InitSmBaseForAllCpus - SmmRelocationStart: 0x%08x\n= ", SmmRelocationStart)); - - // - // Init SmBase - // - *SmBase =3D (UINT64 *)AllocatePages (EFI_SIZE_TO_PAGES (sizeof (UINT64) = * mMaxNumberOfCpus)); - if (*SmBase =3D=3D NULL) { - return EFI_OUT_OF_RESOURCES; - } - - for (Index =3D 0; Index < mMaxNumberOfCpus; Index++) { - // - // Return each SmBase in SmBase - // - (*SmBase)[Index] =3D (UINTN)(SmmRelocationStart)+ Index * TileSize - S= MM_HANDLER_OFFSET; - DEBUG ((DEBUG_INFO, "InitSmBaseForAllCpus - SmBase For CPU[%d]: 0x%08x= \n", Index, (*SmBase)[Index])); - } - - return EFI_SUCCESS; -} - /** CPU SmmBase Relocation Init. This function is to relocate CPU SmmBase. @@ -476,17 +436,21 @@ EFI_STATUS EFIAPI SmmRelocationInit ( IN EDKII_PEI_MP_SERVICES2_PPI *MpServices2 ) { - EFI_STATUS Status; - UINTN NumberOfEnabledCpus; - UINTN SmmStackSize; - UINT8 *SmmStacks; - UINTN Index; + EFI_STATUS Status; + UINTN NumberOfEnabledCpus; + UINTN TileSize; + UINT64 SmmRelocationSize; + EFI_PHYSICAL_ADDRESS SmmRelocationStart; + UINTN SmmStackSize; + UINT8 *SmmStacks; + UINTN Index; - SmmStacks =3D NULL; + SmmRelocationStart =3D 0; + SmmStacks =3D NULL; DEBUG ((DEBUG_INFO, "SmmRelocationInit Start \n")); if (MpServices2 =3D=3D NULL) { return EFI_INVALID_PARAMETER; } @@ -528,17 +492,43 @@ SmmRelocationInit ( } } } // - // Initialize the SmBase for all CPUs + // Calculate SmmRelocationSize for all of the tiles. // - Status =3D InitSmBaseForAllCpus (&mSmBase); + // The CPU save state and code for the SMI entry point are tiled within = an SMRAM + // allocated buffer. The minimum size of this buffer for a uniprocessor = system + // is 32 KB, because the entry point is SMBASE + 32KB, and CPU save stat= e area + // just below SMBASE + 64KB. If more than one CPU is present in the plat= form, + // then the SMI entry point and the CPU save state areas can be tiles to= minimize + // the total amount SMRAM required for all the CPUs. The tile size can b= e computed + // by adding the CPU save state size, any extra CPU specific context, an= d + // the size of code that must be placed at the SMI entry point to transf= er + // control to a C function in the native SMM execution mode. This size i= s + // rounded up to the nearest power of 2 to give the tile size for a each= CPU. + // The total amount of memory required is the maximum number of CPUs tha= t + // platform supports times the tile size. + // + TileSize =3D SIZE_8KB; + SmmRelocationSize =3D EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (SIZE_32KB + = TileSize * (mMaxNumberOfCpus - 1))); + + // + // Split SmramReserve HOB to reserve SmmRelocationSize for Smm relocated= memory + // + Status =3D SplitSmramHobForSmmRelocation ( + SmmRelocationSize, + &SmmRelocationStart + ); if (EFI_ERROR (Status)) { goto ON_EXIT; } + ASSERT (SmmRelocationStart !=3D 0); + DEBUG ((DEBUG_INFO, "SmmRelocationInit - SmmRelocationSize: 0x%08x\n", S= mmRelocationSize)); + DEBUG ((DEBUG_INFO, "SmmRelocationInit - SmmRelocationStart: 0x%08x\n", = SmmRelocationStart)); + // // Fix up the address of the global variable or function referred in // SmmInit assembly files to be the absolute address // SmmInitFixupAddress (); @@ -569,32 +559,21 @@ SmmRelocationInit ( // InitSmmIdt (); // // Relocate SmmBases for each processor. - // Allocate mRebased as the flag to indicate the relocation is done for = each CPU. // - mRebased =3D (BOOLEAN *)AllocateZeroPool (sizeof (BOOLEAN) * mMaxNumberO= fCpus); - if (mRebased =3D=3D NULL) { - Status =3D EFI_OUT_OF_RESOURCES; - goto ON_EXIT; - } - - SmmRelocateBases (); + SmmRelocateBases (MpServices2, SmmRelocationStart, TileSize); // // Create the SmBase HOB for all CPUs // - Status =3D CreateSmmBaseHob (mSmBase); + Status =3D CreateSmmBaseHob (SmmRelocationStart, TileSize); ON_EXIT: if (SmmStacks !=3D NULL) { FreePages (SmmStacks, EFI_SIZE_TO_PAGES (SmmStackSize)); } - if (mSmBase !=3D NULL) { - FreePages (mSmBase, EFI_SIZE_TO_PAGES (sizeof (UINT64) * mMaxNumberOfC= pus)); - } - DEBUG ((DEBUG_INFO, "SmmRelocationInit Done\n")); return Status; } -- 2.16.2.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 (#117961): https://edk2.groups.io/g/devel/message/117961 Mute This Topic: https://groups.io/mt/105593572/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --_000_MN6PR11MB82448F44B42A7FAEA9ED71848C0E2MN6PR11MB8244namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
From: Wu, Jiaxin <jiaxin= .wu@intel.com>
Sent: Thursday, April 18, 2024 14:55
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel= .com>; Gerd Hoffmann <kraxel@redhat.com>; Kumar, Rahul R <rahul= .r.kumar@intel.com>
Subject: [PATCH v3 04/13] UefiCpuPkg/SmmRelocationLib: Avoid unneces= sary memory allocation
 
Since SMM relocation is performed serially for eac= h CPU, there is
no need to allocate buffers for all CPUs to store the SmBase
address in mSmBase and the Rebased flag in mRebased. A defined
global variable is sufficient.

This patch focuses on the mSmBase and mRebased global variables
to prevent unnecessary memory allocation for these variables.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 .../Library/SmmRelocationLib/SmmRelocationLib.c    | 2= 01 +++++++++------------
 1 file changed, 90 insertions(+), 111 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c b/UefiC= puPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index ca98f06a05..3694a07cbb 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -25,31 +25,57 @@ EFI_PROCESSOR_INFORMATION  *mProcessorInfo =3D NUL= L;
 // IDT used during SMM Init
 //
 IA32_DESCRIPTOR  gcSmmInitIdtr;
 
 //
-// Smbase for all CPUs
+// Smbase for current CPU
 //
-UINT64  *mSmBase =3D NULL;
+UINT64  mSmBase;
 
 //
-// SmBase Rebased flag for all CPUs
+// SmBase Rebased flag for current CPU
 //
-volatile BOOLEAN  *mRebased;
+volatile BOOLEAN  mRebased;
+
+/**
+  This function will get the SmBase for CpuIndex.
+
+  @param[in]   CpuIndex      =       The processor index.
+  @param[in]   SmmRelocationStart  The start address o= f Smm relocated memory in SMRAM.
+  @param[in]   TileSize      =       The total size required for a CPU save state= , any
+            &n= bsp;            = ;          additional CPU-spec= ific context and the size of code
+            &n= bsp;            = ;          for the SMI entry p= oint.
+
+  @retval The value of SmBase for CpuIndex.
+
+**/
+UINTN
+GetSmBase (
+  IN UINTN          = ;       CpuIndex,
+  IN EFI_PHYSICAL_ADDRESS  SmmRelocationStart,
+  IN UINTN          = ;       TileSize
+  )
+{
+  return (UINTN)(SmmRelocationStart) + CpuIndex * TileSize - SMM_HAND= LER_OFFSET;
+}
 
 /**
   This function will create SmBase for all CPUs.
 
-  @param[in] SmBase    Pointer to SmBase for all CPUs.=
+  @param[in]   SmmRelocationStart  The start address o= f Smm relocated memory in SMRAM.
+  @param[in]   TileSize      =       The total size required for a CPU save state= , any
+            &n= bsp;            = ;          additional CPU-spec= ific context and the size of code
+            &n= bsp;            = ;          for the SMI entry p= oint.
 
   @retval EFI_SUCCESS       &= nbsp;   Create SmBase for all CPUs successfully.
   @retval Others        =         Failed to create SmBase for all = CPUs.
 
 **/
 EFI_STATUS
 CreateSmmBaseHob (
-  IN UINT64  *SmBase
+  IN EFI_PHYSICAL_ADDRESS  SmmRelocationStart,
+  IN UINTN          = ;       TileSize
   )
 {
   UINTN         &nb= sp;    Index;
   SMM_BASE_HOB_DATA  *SmmBaseHobData;
   UINT32         &n= bsp;   CpuCount;
@@ -90,11 +116,11 @@ CreateSmmBaseHob (
     DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - SmmBa= seHobData[%d]->NumberOfProcessors: %d\n", HobCount, SmmBaseHobData-= >NumberOfProcessors));
     for (Index =3D 0; Index < SmmBaseHobData->Nu= mberOfProcessors; Index++) {
       //
       // Calculate the new SMBASE address        //
-      SmmBaseHobData->SmBase[Index] =3D SmBase= [Index + CpuCount];
+      SmmBaseHobData->SmBase[Index] =3D GetSmB= ase (Index + CpuCount, SmmRelocationStart, TileSize);
       DEBUG ((DEBUG_INFO, "CreateSmmBas= eHob - SmmBaseHobData[%d]->SmBase[%d]: 0x%08x\n", HobCount, Index, = SmmBaseHobData->SmBase[Index]));
     }
 
     CpuCount +=3D NumberOfProcessorsInHob;
     HobCount++;
@@ -126,18 +152,18 @@ SmmInitHandler (
   for (Index =3D 0; Index < mNumberOfCpus; Index++) {
     if (ApicId =3D=3D (UINT32)mProcessorInfo[Index].Pr= ocessorId) {
       //
       // Configure SmBase.
       //
-      ConfigureSmBase (mSmBase[Index]);
+      ConfigureSmBase (mSmBase);
 
       //
       // Hook return after RSM to set SMM re= -based flag
       // SMM re-based flag can't be set befo= re RSM, because SMM save state context might be override
       // by next AP flow before it take effe= ct.
       //
-      SemaphoreHook (Index, &mRebased[Index])= ;
+      SemaphoreHook (Index, &mRebased);
       return;
     }
   }
 
   ASSERT (FALSE);
@@ -145,14 +171,22 @@ SmmInitHandler (
 
 /**
   Relocate SmmBases for each processor.
   Execute on first boot and all S3 resumes
 
+  @param[in]   MpServices2     &nb= sp;   Pointer to this instance of the MpServices.
+  @param[in]   SmmRelocationStart  The start address o= f Smm relocated memory in SMRAM.
+  @param[in]   TileSize      =       The total size required for a CPU save state= , any
+            &n= bsp;            = ;          additional CPU-spec= ific context and the size of code
+            &n= bsp;            = ;          for the SMI entry p= oint.
+
 **/
 VOID
 SmmRelocateBases (
-  VOID
+  IN EDKII_PEI_MP_SERVICES2_PPI  *MpServices2,
+  IN EFI_PHYSICAL_ADDRESS        S= mmRelocationStart,
+  IN UINTN          = ;             T= ileSize
   )
 {
   UINT8         &nb= sp;       BakBuf[BACK_BUF_SIZE];
   SMRAM_SAVE_STATE_MAP  BakBuf2;
   SMRAM_SAVE_STATE_MAP  *CpuStatePtr;
@@ -196,17 +230,18 @@ SmmRelocateBases (
   // Relocate SM bases for all APs
   // This is APs' 1st SMI - rebase will be done here, and APs' d= efault SMI handler will be overridden by gcSmmInitTemplate
   //
   BspIndex =3D (UINTN)-1;
   for (Index =3D 0; Index < mNumberOfCpus; Index++) {
-    mRebased[Index] =3D FALSE;
     if (BspApicId !=3D (UINT32)mProcessorInfo[Index].P= rocessorId) {
+      mRebased =3D FALSE;
+      mSmBase  =3D GetSmBase (Index, SmmRelo= cationStart, TileSize);
       SendSmiIpi ((UINT32)mProcessorInfo[Ind= ex].ProcessorId);
       //
       // Wait for this AP to finish its 1st = SMI
       //
-      while (!mRebased[Index]) {
+      while (!mRebased) {
       }
     } else {
       //
       // BSP will be Relocated later
       //
@@ -216,16 +251,18 @@ SmmRelocateBases (
 
   //
   // Relocate BSP's SMM base
   //
   ASSERT (BspIndex !=3D (UINTN)-1);
+  mRebased =3D FALSE;
+  mSmBase  =3D GetSmBase (BspIndex, SmmRelocationStart, TileSize= );
   SendSmiIpi (BspApicId);
 
   //
   // Wait for the BSP to finish its 1st SMI
   //
-  while (!mRebased[BspIndex]) {
+  while (!mRebased) {
   }
 
   //
   // Restore contents at address 0x38000
   //
@@ -382,87 +419,10 @@ SplitSmramHobForSmmRelocation (
   ZeroMem (&GuidHob->Name, sizeof (GuidHob->Name));  
   return EFI_SUCCESS;
 }
 
-/**
-  This function will initialize SmBase for all CPUs.
-
-  @param[in,out] SmBase    Pointer to SmBase for all C= PUs.
-
-  @retval EFI_SUCCESS        =    Initialize SmBase for all CPUs successfully.
-  @retval Others         = ;       Failed to initialize SmBase for all C= PUs.
-
-**/
-EFI_STATUS
-InitSmBaseForAllCpus (
-  IN OUT UINT64  **SmBase
-  )
-{
-  EFI_STATUS         &nb= sp;  Status;
-  UINTN          &n= bsp;      TileSize;
-  UINT64          &= nbsp;     SmmRelocationSize;
-  EFI_PHYSICAL_ADDRESS  SmmRelocationStart;
-  UINTN          &n= bsp;      Index;
-
-  SmmRelocationStart =3D 0;
-
-  ASSERT (SmBase !=3D NULL);
-
-  //
-  // Calculate SmmRelocationSize for all of the tiles.
-  //
-  // The CPU save state and code for the SMI entry point are tiled wi= thin an SMRAM
-  // allocated buffer. The minimum size of this buffer for a uniproce= ssor system
-  // is 32 KB, because the entry point is SMBASE + 32KB, and CPU save= state area
-  // just below SMBASE + 64KB. If more than one CPU is present in the= platform,
-  // then the SMI entry point and the CPU save state areas can be til= es to minimize
-  // the total amount SMRAM required for all the CPUs. The tile size = can be computed
-  // by adding the CPU save state size, any extra CPU specific contex= t, and
-  // the size of code that must be placed at the SMI entry point to t= ransfer
-  // control to a C function in the native SMM execution mode. This s= ize is
-  // rounded up to the nearest power of 2 to give the tile size for a= each CPU.
-  // The total amount of memory required is the maximum number of CPU= s that
-  // platform supports times the tile size.
-  //
-  TileSize          =3D = SIZE_8KB;
-  SmmRelocationSize =3D EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (SIZE_32= KB + TileSize * (mMaxNumberOfCpus - 1)));
-
-  //
-  // Split SmramReserve HOB to reserve SmmRelocationSize for Smm relo= cated memory
-  //
-  Status =3D SplitSmramHobForSmmRelocation (
-             S= mmRelocationSize,
-             &= amp;SmmRelocationStart
-             )= ;
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
-
-  ASSERT (SmmRelocationStart !=3D 0);
-  DEBUG ((DEBUG_INFO, "InitSmBaseForAllCpus - SmmRelocationSize:= 0x%08x\n", SmmRelocationSize));
-  DEBUG ((DEBUG_INFO, "InitSmBaseForAllCpus - SmmRelocationStart= : 0x%08x\n", SmmRelocationStart));
-
-  //
-  // Init SmBase
-  //
-  *SmBase =3D (UINT64 *)AllocatePages (EFI_SIZE_TO_PAGES (sizeof (UIN= T64) * mMaxNumberOfCpus));
-  if (*SmBase =3D=3D NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  for (Index =3D 0; Index < mMaxNumberOfCpus; Index++) {
-    //
-    // Return each SmBase in SmBase
-    //
-    (*SmBase)[Index] =3D (UINTN)(SmmRelocationStart)+ Index= * TileSize - SMM_HANDLER_OFFSET;
-    DEBUG ((DEBUG_INFO, "InitSmBaseForAllCpus - SmBase= For CPU[%d]: 0x%08x\n", Index, (*SmBase)[Index]));
-  }
-
-  return EFI_SUCCESS;
-}
-
 /**
   CPU SmmBase Relocation Init.
 
   This function is to relocate CPU SmmBase.
 
@@ -476,17 +436,21 @@ EFI_STATUS
 EFIAPI
 SmmRelocationInit (
   IN EDKII_PEI_MP_SERVICES2_PPI  *MpServices2
   )
 {
-  EFI_STATUS  Status;
-  UINTN       NumberOfEnabledCpus;
-  UINTN       SmmStackSize;
-  UINT8       *SmmStacks;
-  UINTN       Index;
+  EFI_STATUS         &nb= sp;  Status;
+  UINTN          &n= bsp;      NumberOfEnabledCpus;
+  UINTN          &n= bsp;      TileSize;
+  UINT64          &= nbsp;     SmmRelocationSize;
+  EFI_PHYSICAL_ADDRESS  SmmRelocationStart;
+  UINTN          &n= bsp;      SmmStackSize;
+  UINT8          &n= bsp;      *SmmStacks;
+  UINTN          &n= bsp;      Index;
 
-  SmmStacks =3D NULL;
+  SmmRelocationStart =3D 0;
+  SmmStacks          =3D= NULL;
 
   DEBUG ((DEBUG_INFO, "SmmRelocationInit Start \n"));<= br>    if (MpServices2 =3D=3D NULL) {
     return EFI_INVALID_PARAMETER;
   }
@@ -528,17 +492,43 @@ SmmRelocationInit (
       }
     }
   }
 
   //
-  // Initialize the SmBase for all CPUs
+  // Calculate SmmRelocationSize for all of the tiles.
   //
-  Status =3D InitSmBaseForAllCpus (&mSmBase);
+  // The CPU save state and code for the SMI entry point are tiled wi= thin an SMRAM
+  // allocated buffer. The minimum size of this buffer for a uniproce= ssor system
+  // is 32 KB, because the entry point is SMBASE + 32KB, and CPU save= state area
+  // just below SMBASE + 64KB. If more than one CPU is present in the= platform,
+  // then the SMI entry point and the CPU save state areas can be til= es to minimize
+  // the total amount SMRAM required for all the CPUs. The tile size = can be computed
+  // by adding the CPU save state size, any extra CPU specific contex= t, and
+  // the size of code that must be placed at the SMI entry point to t= ransfer
+  // control to a C function in the native SMM execution mode. This s= ize is
+  // rounded up to the nearest power of 2 to give the tile size for a= each CPU.
+  // The total amount of memory required is the maximum number of CPU= s that
+  // platform supports times the tile size.
+  //
+  TileSize          =3D = SIZE_8KB;
+  SmmRelocationSize =3D EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (SIZE_32= KB + TileSize * (mMaxNumberOfCpus - 1)));
+
+  //
+  // Split SmramReserve HOB to reserve SmmRelocationSize for Smm relo= cated memory
+  //
+  Status =3D SplitSmramHobForSmmRelocation (
+             S= mmRelocationSize,
+             &= amp;SmmRelocationStart
+             )= ;
   if (EFI_ERROR (Status)) {
     goto ON_EXIT;
   }
 
+  ASSERT (SmmRelocationStart !=3D 0);
+  DEBUG ((DEBUG_INFO, "SmmRelocationInit - SmmRelocationSize: 0x= %08x\n", SmmRelocationSize));
+  DEBUG ((DEBUG_INFO, "SmmRelocationInit - SmmRelocationStart: 0= x%08x\n", SmmRelocationStart));
+
   //
   // Fix up the address of the global variable or function refer= red in
   // SmmInit assembly files to be the absolute address
   //
   SmmInitFixupAddress ();
@@ -569,32 +559,21 @@ SmmRelocationInit (
   //
   InitSmmIdt ();
 
   //
   // Relocate SmmBases for each processor.
-  // Allocate mRebased as the flag to indicate the relocation is done= for each CPU.
   //
-  mRebased =3D (BOOLEAN *)AllocateZeroPool (sizeof (BOOLEAN) * mMaxNu= mberOfCpus);
-  if (mRebased =3D=3D NULL) {
-    Status =3D EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
-  SmmRelocateBases ();
+  SmmRelocateBases (MpServices2, SmmRelocationStart, TileSize);
 
   //
   // Create the SmBase HOB for all CPUs
   //
-  Status =3D CreateSmmBaseHob (mSmBase);
+  Status =3D CreateSmmBaseHob (SmmRelocationStart, TileSize);
 
 ON_EXIT:
   if (SmmStacks !=3D NULL) {
     FreePages (SmmStacks, EFI_SIZE_TO_PAGES (SmmStackS= ize));
   }
 
-  if (mSmBase !=3D NULL) {
-    FreePages (mSmBase, EFI_SIZE_TO_PAGES (sizeof (UINT64) = * mMaxNumberOfCpus));
-  }
-
   DEBUG ((DEBUG_INFO, "SmmRelocationInit Done\n"));    return Status;
 }
--
2.16.2.windows.1

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

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

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

_._,_._,_
--_000_MN6PR11MB82448F44B42A7FAEA9ED71848C0E2MN6PR11MB8244namp_--