| Message ID | 1468420619-19262-3-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/15/16 02:26, Jordan Justen wrote: > On 2016-07-13 07:36:56, Laszlo Ersek wrote: >> Move the permanent PEI memory for the S3 resume boot path to the top of >> the low RAM (just below TSEG if the SMM driver stack is included in the >> build). The new size is derived from CpuMpPei's approximate memory demand. >> >> Save the base address and the size in new global variables, regardless of >> the boot path. On the normal boot path, use these variables for covering >> the area with EfiACPIMemoryNVS type memory. >> >> PcdS3AcpiReservedMemoryBase and PcdS3AcpiReservedMemorySize become unused >> in PlatformPei; remove them. >> >> Cc: Jeff Fan <jeff.fan@intel.com> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Michael Kinney <michael.d.kinney@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> Reviewed-by: Jeff Fan <jeff.fan@intel.com> >> --- >> >> Notes: >> v2: >> - new in v2 [Jordan, Jeff] >> >> OvmfPkg/PlatformPei/PlatformPei.inf | 3 +- >> OvmfPkg/PlatformPei/MemDetect.c | 39 ++++++++++++++------ >> 2 files changed, 28 insertions(+), 14 deletions(-) >> >> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf >> index 3556404017fc..229831b10da0 100644 >> --- a/OvmfPkg/PlatformPei/PlatformPei.inf >> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf >> @@ -65,7 +65,6 @@ [Pcd] >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize >> - gUefiOvmfPkgTokenSpaceGuid.PcdS3AcpiReservedMemoryBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase >> @@ -82,7 +81,6 @@ [Pcd] >> gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd >> gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes >> - gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize >> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress >> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize >> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize >> @@ -95,6 +93,7 @@ [Pcd] >> gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable >> gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable >> gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress >> + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber >> >> [FixedPcd] >> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress >> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c >> index d59b461547c5..7129ed26fa3e 100644 >> --- a/OvmfPkg/PlatformPei/MemDetect.c >> +++ b/OvmfPkg/PlatformPei/MemDetect.c >> @@ -39,6 +39,9 @@ Module Name: >> >> UINT8 mPhysMemAddressWidth; >> >> +STATIC UINT32 mS3AcpiReservedMemoryBase; >> +STATIC UINT32 mS3AcpiReservedMemorySize; >> + >> UINT32 >> GetSystemMemorySizeBelow4gb ( >> VOID >> @@ -335,18 +338,30 @@ PublishPeiMemory ( >> UINT64 LowerMemorySize; >> UINT32 PeiMemoryCap; >> >> + LowerMemorySize = GetSystemMemorySizeBelow4gb (); >> + if (FeaturePcdGet (PcdSmmSmramRequire)) { >> + // >> + // TSEG is chipped from the end of low RAM >> + // >> + LowerMemorySize -= FixedPcdGet8 (PcdQ35TsegMbytes) * SIZE_1MB; >> + } >> + >> + // >> + // If S3 is supported, then the S3 permanent PEI memory is placed next, >> + // downwards. Its size is primarily dictated by CpuMpPei. The formula below >> + // is an approximation. >> + // >> + if (mS3Supported) { >> + mS3AcpiReservedMemorySize = SIZE_512KB + >> + PcdGet32 (PcdCpuMaxLogicalProcessorNumber) * SIZE_32KB; > > Maybe use PcdCpuApStackSize rather than SIZE_32KB? Good idea, I will update the patch on commit. > Is there an chance that with OVMF's modest usage of PEI MP, perhaps we > could get by with a smaller stack size per processor? 4 or 8 KB? This > would reduce 8MB down to 1 or 2MB. > > Maybe we could use a separate PcdPeiCpuApStackSize for PEI? What do > think Jeff/Mike? That also sounds reasonable to me. I've heard from Mike and Jeff that they plan to extract and unify the internals of CpuMpPei and CpuDxe. Perhaps it's best to delay the introduction of a separate PCD for the PEI-phase AP stack size until that refactoring happens. (IOW, make the new PCD part of that series.) Just speculating, of course. > I think OVMF is more sensitive to the size, since we try to support > 255 processors. Max number of processors is a hornet's nest. I filed <https://github.com/tianocore/edk2/issues/87> earlier. I didn't provide much background info on it, but it's related to: [PATCH v2 0/4] support booting more than 255 CPUs with QEMU http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/10654 I'm not even thinking of *thinking* of that feature. :/ For now let's just stick with the default value 64 that we inherit from the UefiCpuPkg.dec file, and allow downstreams to raise it to 255 at the most if they want. > > Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> Thank you! Laszlo >> + mS3AcpiReservedMemoryBase = LowerMemorySize - mS3AcpiReservedMemorySize; >> + LowerMemorySize = mS3AcpiReservedMemoryBase; >> + } >> + >> if (mBootMode == BOOT_ON_S3_RESUME) { >> - MemoryBase = PcdGet32 (PcdS3AcpiReservedMemoryBase); >> - MemorySize = PcdGet32 (PcdS3AcpiReservedMemorySize); >> + MemoryBase = mS3AcpiReservedMemoryBase; >> + MemorySize = mS3AcpiReservedMemorySize; >> } else { >> - LowerMemorySize = GetSystemMemorySizeBelow4gb (); >> - if (FeaturePcdGet (PcdSmmSmramRequire)) { >> - // >> - // TSEG is chipped from the end of low RAM >> - // >> - LowerMemorySize -= FixedPcdGet8 (PcdQ35TsegMbytes) * SIZE_1MB; >> - } >> - >> PeiMemoryCap = GetPeiMemoryCap (); >> DEBUG ((EFI_D_INFO, "%a: mPhysMemAddressWidth=%d PeiMemoryCap=%u KB\n", >> __FUNCTION__, mPhysMemAddressWidth, PeiMemoryCap >> 10)); >> @@ -514,8 +529,8 @@ InitializeRamRegions ( >> // This is the memory range that will be used for PEI on S3 resume >> // >> BuildMemoryAllocationHob ( >> - (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdS3AcpiReservedMemoryBase), >> - (UINT64)(UINTN) PcdGet32 (PcdS3AcpiReservedMemorySize), >> + mS3AcpiReservedMemoryBase, >> + mS3AcpiReservedMemorySize, >> EfiACPIMemoryNVS >> ); >> >> -- >> 1.8.3.1 >> >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf index 3556404017fc..229831b10da0 100644 --- a/OvmfPkg/PlatformPei/PlatformPei.inf +++ b/OvmfPkg/PlatformPei/PlatformPei.inf @@ -65,7 +65,6 @@ [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize - gUefiOvmfPkgTokenSpaceGuid.PcdS3AcpiReservedMemoryBase gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase @@ -82,7 +81,6 @@ [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes - gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize @@ -95,6 +93,7 @@ [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress + gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber [FixedPcd] gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c index d59b461547c5..7129ed26fa3e 100644 --- a/OvmfPkg/PlatformPei/MemDetect.c +++ b/OvmfPkg/PlatformPei/MemDetect.c @@ -39,6 +39,9 @@ Module Name: UINT8 mPhysMemAddressWidth; +STATIC UINT32 mS3AcpiReservedMemoryBase; +STATIC UINT32 mS3AcpiReservedMemorySize; + UINT32 GetSystemMemorySizeBelow4gb ( VOID @@ -335,18 +338,30 @@ PublishPeiMemory ( UINT64 LowerMemorySize; UINT32 PeiMemoryCap; + LowerMemorySize = GetSystemMemorySizeBelow4gb (); + if (FeaturePcdGet (PcdSmmSmramRequire)) { + // + // TSEG is chipped from the end of low RAM + // + LowerMemorySize -= FixedPcdGet8 (PcdQ35TsegMbytes) * SIZE_1MB; + } + + // + // If S3 is supported, then the S3 permanent PEI memory is placed next, + // downwards. Its size is primarily dictated by CpuMpPei. The formula below + // is an approximation. + // + if (mS3Supported) { + mS3AcpiReservedMemorySize = SIZE_512KB + + PcdGet32 (PcdCpuMaxLogicalProcessorNumber) * SIZE_32KB; + mS3AcpiReservedMemoryBase = LowerMemorySize - mS3AcpiReservedMemorySize; + LowerMemorySize = mS3AcpiReservedMemoryBase; + } + if (mBootMode == BOOT_ON_S3_RESUME) { - MemoryBase = PcdGet32 (PcdS3AcpiReservedMemoryBase); - MemorySize = PcdGet32 (PcdS3AcpiReservedMemorySize); + MemoryBase = mS3AcpiReservedMemoryBase; + MemorySize = mS3AcpiReservedMemorySize; } else { - LowerMemorySize = GetSystemMemorySizeBelow4gb (); - if (FeaturePcdGet (PcdSmmSmramRequire)) { - // - // TSEG is chipped from the end of low RAM - // - LowerMemorySize -= FixedPcdGet8 (PcdQ35TsegMbytes) * SIZE_1MB; - } - PeiMemoryCap = GetPeiMemoryCap (); DEBUG ((EFI_D_INFO, "%a: mPhysMemAddressWidth=%d PeiMemoryCap=%u KB\n", __FUNCTION__, mPhysMemAddressWidth, PeiMemoryCap >> 10)); @@ -514,8 +529,8 @@ InitializeRamRegions ( // This is the memory range that will be used for PEI on S3 resume // BuildMemoryAllocationHob ( - (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdS3AcpiReservedMemoryBase), - (UINT64)(UINTN) PcdGet32 (PcdS3AcpiReservedMemorySize), + mS3AcpiReservedMemoryBase, + mS3AcpiReservedMemorySize, EfiACPIMemoryNVS );