From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.93; helo=mga11.intel.com; envelope-from=nathaniel.l.desimone@intel.com; receiver=edk2-devel@lists.01.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 578F0208F7AA8 for ; Fri, 25 Jan 2019 16:56:30 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Jan 2019 16:56:29 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,523,1539673200"; d="scan'208";a="294506371" Received: from orsmsx109.amr.corp.intel.com ([10.22.240.7]) by orsmga005.jf.intel.com with ESMTP; 25 Jan 2019 16:56:29 -0800 Received: from orsmsx159.amr.corp.intel.com (10.22.240.24) by ORSMSX109.amr.corp.intel.com (10.22.240.7) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 25 Jan 2019 16:56:29 -0800 Received: from orsmsx114.amr.corp.intel.com ([169.254.8.248]) by ORSMSX159.amr.corp.intel.com ([169.254.11.106]) with mapi id 14.03.0415.000; Fri, 25 Jan 2019 16:56:28 -0800 From: "Desimone, Nathaniel L" To: "Chiu, Chasel" , "edk2-devel@lists.01.org" CC: "Zeng, Star" Thread-Topic: [PATCH] IntelFsp2Pkg: FSP can utilize bootloader stack Thread-Index: AQHUsv9tJ68crtJSS0WezxcYdacSiqXAu29w Date: Sat, 26 Jan 2019 00:56:27 +0000 Message-ID: <02A34F284D1DA44BB705E61F7180EF0AAE81B923@ORSMSX114.amr.corp.intel.com> References: <20190123093311.16708-1-chasel.chiu@intel.com> In-Reply-To: <20190123093311.16708-1-chasel.chiu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZWRmMDgzYjUtZDA1NC00MDdmLThkNDktMTdmYzhhNjQwYzFlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiVTN3VmQyaVZOWExjUzdsUSt6cndtNVQxa20zR2Y3dzNlVjVobVJjNllWazdoOGtcL1VrTk51K1VTU0JNbEp5QkUifQ== x-ctpclassification: CTP_NT x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Subject: Re: [PATCH] IntelFsp2Pkg: FSP can utilize bootloader stack X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 26 Jan 2019 00:56:30 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Chasel, I agree with Star that we should just take "Bootloader stack" out of the di= agram for scenario 1. It doesn't matter where in the memory map the bootloa= der stack is really. My only extra comment is the definition for AsmReadEsp() should be placed i= n SecMain.h instead of SecMain.c. With those changes: Reviewed-by: Nate DeSimone -----Original Message----- From: Chiu, Chasel=20 Sent: Wednesday, January 23, 2019 1:33 AM To: edk2-devel@lists.01.org Cc: Desimone, Nathaniel L ; Zeng, Star ; Chiu, Chasel Subject: [PATCH] IntelFsp2Pkg: FSP can utilize bootloader stack REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1485 Current FSP utilizes pre-allocated temporary memory from boot loader for bo= th heap and stack. To reduce overall temporary memory usage FSP may share t= he same stack with boot loader and only needs a smaller memory for heap, no= separate memory required for stack. Setting PcdFspHeapSizePercentage to 0 to enable FSP sharing stack with boot= loader, in this case boot loader stack has to be large enough for FSP to u= se. Default is 50 (half memory heap and half memory stack) for backward com= patible with original model. Test: Verified on internal platform and booting successfully with both modes. Cc: Nate DeSimone Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Chasel Chiu --- IntelFsp2Pkg/FspSecCore/SecMain.c | 86 ++++++++++++++++++++++= ++++++++++++++++++++++++++++++++++++++++++++++++++++------------ IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf | 3 ++- IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm | 52 ++++++++++++++++++++++= ++++++++++++++++++++++++------ IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm | 28 ++++++++++++++++++++++= ++++++ IntelFsp2Pkg/IntelFsp2Pkg.dec | 4 ++++ 5 files changed, 154 insertions(+), 19 deletions(-) diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c b/IntelFsp2Pkg/FspSecCore/Se= cMain.c index 70460a3c8b..b0b5dda711 100644 --- a/IntelFsp2Pkg/FspSecCore/SecMain.c +++ b/IntelFsp2Pkg/FspSecCore/SecMain.c @@ -1,6 +1,6 @@ /** @file =20 - Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2019, Intel Corporation. All rights=20 + reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BS= D License which accompanies this distribution. The full text of the license may b= e found at @@ -38,6 +38,19 @@ UINT64 mIdtEntryTemplate =3D 0xffff8e000008f= fe4ULL; =20 /** =20 + Return value of esp + + @return value of esp + +**/ +UINT32 +EFIAPI +AsmReadEsp ( + VOID + ); + +/** + Entry point to the C language phase of SEC. After the SEC assembly code has initialized some temporary memory and set up the stack, the control is transferred to this function. @@ -83,7 +96,26 @@ SecStartup ( // InitializeFloatingPointUnits (); =20 + // + // Scenario 1 memory map when running on bootloader stack // //=20 + |-------------------|----> + // |Idt Table | + // |-------------------| + // |PeiService Pointer | + // |-------------------| + // | | + // | | + // | Heap | + // | | + // | | + // |-------------------|----> TempRamBase + // |Bootloader stack | + // |-------------------| =20 + // + // Scenario 2 memory map when running FSP on a separate stack // // |-------------------|----> // |Idt Table | // |-------------------| @@ -135,11 +167,19 @@ SecStartup ( SecCoreData.BootFirmwareVolumeSize =3D (UINT32)((EFI_FIRMWARE_VOLUME_HEA= DER *)BootFirmwareVolume)->FvLength; =20 SecCoreData.TemporaryRamBase =3D (VOID*)(UINTN) TempRamBase; - SecCoreData.TemporaryRamSize =3D SizeOfRam; - SecCoreData.PeiTemporaryRamBase =3D SecCoreData.TemporaryRamBase; - SecCoreData.PeiTemporaryRamSize =3D SecCoreData.TemporaryRamSize * Pc= dGet8 (PcdFspHeapSizePercentage) / 100; - SecCoreData.StackBase =3D (VOID*)(UINTN)((UINTN)SecCoreData= .TemporaryRamBase + SecCoreData.PeiTemporaryRamSize); - SecCoreData.StackSize =3D SecCoreData.TemporaryRamSize - Se= cCoreData.PeiTemporaryRamSize; + if (PcdGet8 (PcdFspHeapSizePercentage) =3D=3D 0) { + SecCoreData.TemporaryRamSize =3D SizeOfRam; // stack size that i= s going to be copied to the permanent memory + SecCoreData.PeiTemporaryRamBase =3D SecCoreData.TemporaryRamBase; + SecCoreData.PeiTemporaryRamSize =3D SecCoreData.TemporaryRamSize; + SecCoreData.StackBase =3D (VOID *)GetFspEntryStack(); // = Share the same boot loader stack + SecCoreData.StackSize =3D 0; + } else { + SecCoreData.TemporaryRamSize =3D SizeOfRam; + SecCoreData.PeiTemporaryRamBase =3D SecCoreData.TemporaryRamBase; + SecCoreData.PeiTemporaryRamSize =3D SecCoreData.TemporaryRamSize * = PcdGet8 (PcdFspHeapSizePercentage) / 100; + SecCoreData.StackBase =3D (VOID*)(UINTN)((UINTN)SecCoreDa= ta.TemporaryRamBase + SecCoreData.PeiTemporaryRamSize); + SecCoreData.StackSize =3D SecCoreData.TemporaryRamSize - = SecCoreData.PeiTemporaryRamSize; + } =20 DEBUG ((DEBUG_INFO, "Fsp BootFirmwareVolumeBase - 0x%x\n", SecCoreData.B= ootFirmwareVolumeBase)); DEBUG ((DEBUG_INFO, "Fsp BootFirmwareVolumeSize - 0x%x\n", SecCoreData.B= ootFirmwareVolumeSize)); @@ -194,15 +234,37 @@ SecTemporaryRamSupport ( UINTN HeapSize; UINTN StackSize; =20 - HeapSize =3D CopySize * PcdGet8 (PcdFspHeapSizePercentage) / 100 ; - StackSize =3D CopySize - HeapSize; + UINTN CurrentStack; + UINTN FspStackBase; + + if (PcdGet8 (PcdFspHeapSizePercentage) =3D=3D 0) { =20 - OldHeap =3D (VOID*)(UINTN)TemporaryMemoryBase; - NewHeap =3D (VOID*)((UINTN)PermanentMemoryBase + StackSize); + CurrentStack =3D AsmReadEsp(); + FspStackBase =3D (UINTN)GetFspEntryStack(); =20 - OldStack =3D (VOID*)((UINTN)TemporaryMemoryBase + HeapSize); - NewStack =3D (VOID*)(UINTN)PermanentMemoryBase; + StackSize =3D FspStackBase - CurrentStack; + HeapSize =3D CopySize; =20 + OldHeap =3D (VOID*)(UINTN)TemporaryMemoryBase; + NewHeap =3D (VOID*)((UINTN)PermanentMemoryBase); + + OldStack =3D (VOID*)CurrentStack; + // + //The old stack is copied at the end of the stack region because stack= grows down. + // + NewStack =3D (VOID*)((UINTN)PermanentMemoryBase - StackSize); + + } else { + HeapSize =3D CopySize * PcdGet8 (PcdFspHeapSizePercentage) / 100 ; + StackSize =3D CopySize - HeapSize; + + OldHeap =3D (VOID*)(UINTN)TemporaryMemoryBase; + NewHeap =3D (VOID*)((UINTN)PermanentMemoryBase + StackSize); + + OldStack =3D (VOID*)((UINTN)TemporaryMemoryBase + HeapSize); + NewStack =3D (VOID*)(UINTN)PermanentMemoryBase; + + } // // Migrate Heap // diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf b/IntelFsp2Pkg/FspSecC= ore/FspSecCoreM.inf index dafe6f5993..0024254e0e 100644 --- a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf @@ -1,7 +1,7 @@ ## @file # Sec Core for FSP # -# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2016 - 2019, Intel Corporation. All rights=20 +reserved.
# # This program and the accompanying materials # are licensed and made a= vailable under the terms and conditions of the BSD License @@ -38,6 +38,7 @= @ Ia32/FspApiEntryM.nasm Ia32/FspApiEntryCommon.nasm Ia32/FspHelper.nasm + Ia32/ReadEsp.nasm =20 [Binaries.Ia32] RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm b/IntelFsp2Pkg/= FspSecCore/Ia32/FspApiEntryM.nasm index 9c9a84db0a..916ad4ff0d 100644 --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm @@ -1,7 +1,7 @@ ;; @file ; Provide FSP API entry points. ; -; Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
+; Copyright (c) 2016 - 2019, Intel Corporation. All rights=20 +reserved.
; This program and the accompanying materials ; are licensed and made ava= ilable under the terms and conditions of the BSD License ; which accompani= es this distribution. The full text of the license may be found at @@ -19,= 6 +19,7 @@ extern ASM_PFX(PcdGet32(PcdTemporaryRamBase)) extern ASM_PFX(PcdGet32(PcdTemporaryRamSize)) extern ASM_PFX(PcdGet32(PcdFspTemporaryRamSize)) +extern ASM_PFX(PcdGet8 (PcdFspHeapSizePercentage)) =20 struc FSPM_UPD_COMMON ; FSP_UPD_HEADER { @@ -128,15 +129,55 @@ ASM_PFX(FspApiCommonContinue): add edx, [eax + FSP_HEADER_CFGREG_OFFSET] pop eax =20 - FspStackSetup: +FspStackSetup: + ; + ; StackBase =3D temp memory base, StackSize =3D temp memory size + ; mov edi, [edx + FSPM_UPD_COMMON.StackBase] mov ecx, [edx + FSPM_UPD_COMMON.StackSize] + + ; + ; Keep using bootloader stack if heap size % is 0 ; + mov bl, BYTE [ASM_PFX(PcdGet8 (PcdFspHeapSizePercentage))] + cmp bl, 0 + jz SkipStackSwitch + + ; + ; Set up a dedicated temp ram stack for FSP if FSP heap size %=20 + doesn't equal 0 ; add edi, ecx ; - ; Setup new FSP stack + ; Switch to new FSP stack ; - xchg edi, esp ; Exchange edi and esp, = edi will be assigned to the current esp pointer and esp will be Stack base = + Stack size - mov ebx, esp ; Put Stack base + Stack= size in ebx + xchg edi, esp ; Exchange edi and esp, e= di will be assigned to the current esp pointer and esp will be Stack base += Stack size + +SkipStackSwitch: + ; + ; If heap size % is 0: + ; EDI is FSPM_UPD_COMMON.StackBase and will hold ESP later (boot loade= r stack pointer) + ; ECX is FSPM_UPD_COMMON.StackSize + ; ESP is boot loader stack pointer (no stack switch) + ; BL is 0 to indicate no stack switch (EBX will hold FSPM_UPD_COMMON.= StackBase later) + ; + ; If heap size % is not 0 + ; EDI is boot loader stack pointer + ; ECX is FSPM_UPD_COMMON.StackSize + ; ESP is new stack (FSPM_UPD_COMMON.StackBase + FSPM_UPD_COMMON.StackS= ize) + ; BL is NOT 0 to indicate stack has switched + ; + cmp bl, 0 + jnz StackHasBeenSwitched + + mov ebx, edi ; Put FSPM_UPD_COMMON.Sta= ckBase to ebx as temp memory base + mov edi, esp ; Put boot loader stack p= ointer to edi + jmp StackSetupDone + +StackHasBeenSwitched: + mov ebx, esp ; Put Stack base + Stack = size in ebx + sub ebx, ecx ; Stack base + Stack size= - Stack size as temp memory base + +StackSetupDone: =20 ; ; Pass the API Idx to SecStartup @@ -170,7 +211,6 @@ ASM_PFX(FspApiCommonContinue): ; ; Pass stack base and size into the PEI Core ; - sub ebx, ecx ; Stack base + Stack size - Stack size push ebx push ecx =20 diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm b/IntelFsp2Pkg/FspSe= cCore/Ia32/ReadEsp.nasm new file mode 100644 index 0000000000..ca29f0529c --- /dev/null +++ b/IntelFsp2Pkg/FspSecCore/Ia32/ReadEsp.nasm @@ -0,0 +1,28 @@ +;; @file +; Provide read ESP function +; +; Copyright (c) 2019, Intel Corporation. All rights reserved.
;=20 +This program and the accompanying materials ; are licensed and made=20 +available under the terms and conditions of the BSD License ; which=20 +accompanies this distribution. The full text of the license may be=20 +found at ; http://opensource.org/licenses/bsd-license.php. +; +; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,=20 +; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMP= LIED. +;; +;---------------------------------------------------------------------- +-------- + + SECTION .text + +;---------------------------------------------------------------------- +-------- +; UINT32 +; EFIAPI +; AsmReadEsp ( +; VOID +; ); +;---------------------------------------------------------------------- +-------- +global ASM_PFX(AsmReadEsp) +ASM_PFX(AsmReadEsp): + mov eax, esp + ret + diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dec b/IntelFsp2Pkg/IntelFsp2Pkg.dec = index 64a31b3505..d4c6d221a1 100644 --- a/IntelFsp2Pkg/IntelFsp2Pkg.dec +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dec @@ -86,8 +86,12 @@ gIntelFsp2PkgTokenSpaceGuid.PcdFspBootFirmwareVolumeBase|0xFFF80000|UINT= 32|0x10000003 gIntelFsp2PkgTokenSpaceGuid.PcdFspHeaderSpecVersion | 0x20| UIN= T8|0x00000002 =20 + # # x % of FSP temporary memory will be used for heap # (100 - x) % of FSP temporary memory will be used for stack + # 0 means FSP will share the stack with boot loader and FSP temporary me= mory is heap + # Note: This mode assumes boot loader stack is large enough for FSP to= use. + # gIntelFsp2PkgTokenSpaceGuid.PcdFspHeapSizePercentage | 50| UIN= T8|0x10000004 # # Maximal Interrupt supported in IDT table. -- 2.13.3.windows.1