[edk2-devel] [PATCH V3] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c
Zeng, Star
star.zeng at intel.com
Fri Jan 8 14:13:23 UTC 2021
Thanks Laszlo.
Ray, Laszlo or Eric, need your help to commit it if no further comment to the patch.
Thanks,
Star
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Thursday, January 7, 2021 7:01 PM
> To: Zeng, Star <star.zeng at intel.com>; devel at edk2.groups.io
> Cc: Ni, Ray <ray.ni at intel.com>; Dong, Eric <eric.dong at intel.com>
> Subject: Re: [edk2-devel] [PATCH V3] UefiCpuPkg PiSmmCpuDxeSmm: Reduce
> SMRAM consumption in CpuS3.c
>
> On 01/07/21 11:46, Star Zeng wrote:
> > This patch makes two refinements to reduce SMRAM consumption in
> CpuS3.c.
> > 1. Only do CopyRegisterTable() when register table is not empty,
> > IsRegisterTableEmpty() is created to check whether the register table
> > is empty or not.
> >
> > Take empty PreSmmInitRegisterTable as example, about 24K SMRAM
> consumption
> > could be reduced when mAcpiCpuData.NumberOfCpus=1024.
> > sizeof (CPU_REGISTER_TABLE) = 24
> > mAcpiCpuData.NumberOfCpus = 1024 = 1K
> > mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE) = 24K
> >
> > 2. Only copy table entries buffer instead of whole buffer.
> > AllocatedSize in SourceRegisterTableList is the whole buffer size.
> > Actually, only the table entries buffer needs to be copied, and the size
> > is TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY).
> >
> > Take AllocatedSize=0x1000=4096, TableLength=100 and
> NumberOfCpus=1024 as example,
> > about 1696K SMRAM consumption could be reduced.
> > sizeof (CPU_REGISTER_TABLE_ENTRY) = 24
> > TableLength = 100
> > TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY) = 2400
> > AllocatedSize = 0x1000 = 4096
> > AllocatedSize - TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY) = 4096
> - 2400 = 1696
> > NumberOfCpus = 1024 = 1K
> > NumberOfCpus * (AllocatedSize - TableLength * sizeof
> > (CPU_REGISTER_TABLE_ENTRY)) = 1696K
> >
> > This patch also corrects the CopyRegisterTable() function description.
> >
> > Signed-off-by: Star Zeng <star.zeng at intel.com>
> > Reviewed-by: Ray Ni <ray.ni at intel.com>
> > Reviewed-by: Laszlo Ersek <lersek at redhat.com>
> > Cc: Ray Ni <ray.ni at intel.com>
> > Cc: Eric Dong <eric.dong at intel.com>
> > Cc: Laszlo Ersek <lersek at redhat.com>
> > ---
> > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 73
> > ++++++++++++++++++++++++-------
> > 1 file changed, 56 insertions(+), 17 deletions(-)
>
> Thanks for the update, my R-b stands.
>
> Laszlo
>
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > index 9592430636ec..724e5460ba6f 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> > @@ -1,7 +1,7 @@
> > /** @file
> > Code for Processor S3 restoration
> >
> > -Copyright (c) 2006 - 2020, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2006 - 2021, Intel Corporation. All rights
> > +reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -487,6 +487,9 @@ SetRegister (
> > } else {
> > RegisterTables = (CPU_REGISTER_TABLE
> *)(UINTN)mAcpiCpuData.RegisterTable;
> > }
> > + if (RegisterTables == NULL) {
> > + return;
> > + }
> >
> > InitApicId = GetInitialApicId ();
> > RegisterTable = NULL;
> > @@ -948,7 +951,7 @@ InitSmmS3ResumeState ( }
> >
> > /**
> > - Copy register table from ACPI NVS memory into SMRAM.
> > + Copy register table from non-SMRAM into SMRAM.
> >
> > @param[in] DestinationRegisterTableList Points to destination register
> table.
> > @param[in] SourceRegisterTableList Points to source register table.
> > @@ -967,7 +970,8 @@ CopyRegisterTable (
> >
> > CopyMem (DestinationRegisterTableList, SourceRegisterTableList,
> NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
> > for (Index = 0; Index < NumberOfCpus; Index++) {
> > - if (DestinationRegisterTableList[Index].AllocatedSize != 0) {
> > + if (DestinationRegisterTableList[Index].TableLength != 0) {
> > + DestinationRegisterTableList[Index].AllocatedSize =
> > + DestinationRegisterTableList[Index].TableLength * sizeof
> > + (CPU_REGISTER_TABLE_ENTRY);
> > RegisterTableEntry = AllocateCopyPool (
> > DestinationRegisterTableList[Index].AllocatedSize,
> > (VOID
> > *)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry
> > @@ -978,6 +982,33 @@ CopyRegisterTable (
> > }
> > }
> >
> > +/**
> > + Check whether the register table is empty or not.
> > +
> > + @param[in] RegisterTable Point to the register table.
> > +
> > + @retval TRUE The register table is empty.
> > + @retval FALSE The register table is not empty.
> > +**/
> > +BOOLEAN
> > +IsRegisterTableEmpty (
> > + IN CPU_REGISTER_TABLE *RegisterTable,
> > + IN UINT32 NumberOfCpus
> > + )
> > +{
> > + UINTN Index;
> > +
> > + if (RegisterTable != NULL) {
> > + for (Index = 0; Index < NumberOfCpus; Index++) {
> > + if (RegisterTable[Index].TableLength != 0) {
> > + return FALSE;
> > + }
> > + }
> > + }
> > +
> > + return TRUE;
> > +}
> > +
> > /**
> > Get ACPI CPU data.
> >
> > @@ -1032,23 +1063,31 @@ GetAcpiCpuData (
> >
> > CopyMem ((VOID *)(UINTN)mAcpiCpuData.IdtrProfile, (VOID
> > *)(UINTN)AcpiCpuData->IdtrProfile, sizeof (IA32_DESCRIPTOR));
> >
> > - mAcpiCpuData.PreSmmInitRegisterTable =
> > (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool
> (mAcpiCpuData.NumberOfCpus *
> > sizeof (CPU_REGISTER_TABLE));
> > - ASSERT (mAcpiCpuData.PreSmmInitRegisterTable != 0);
> > + if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE
> *)(UINTN)AcpiCpuData->PreSmmInitRegisterTable,
> mAcpiCpuData.NumberOfCpus)) {
> > + mAcpiCpuData.PreSmmInitRegisterTable =
> (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool
> (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
> > + ASSERT (mAcpiCpuData.PreSmmInitRegisterTable != 0);
> >
> > - CopyRegisterTable (
> > - (CPU_REGISTER_TABLE
> *)(UINTN)mAcpiCpuData.PreSmmInitRegisterTable,
> > - (CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData-
> >PreSmmInitRegisterTable,
> > - mAcpiCpuData.NumberOfCpus
> > - );
> > + CopyRegisterTable (
> > + (CPU_REGISTER_TABLE
> *)(UINTN)mAcpiCpuData.PreSmmInitRegisterTable,
> > + (CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData-
> >PreSmmInitRegisterTable,
> > + mAcpiCpuData.NumberOfCpus
> > + );
> > + } else {
> > + mAcpiCpuData.PreSmmInitRegisterTable = 0; }
> >
> > - mAcpiCpuData.RegisterTable =
> > (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool
> (mAcpiCpuData.NumberOfCpus *
> > sizeof (CPU_REGISTER_TABLE));
> > - ASSERT (mAcpiCpuData.RegisterTable != 0);
> > + if (!IsRegisterTableEmpty ((CPU_REGISTER_TABLE
> *)(UINTN)AcpiCpuData->RegisterTable, mAcpiCpuData.NumberOfCpus)) {
> > + mAcpiCpuData.RegisterTable =
> (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool
> (mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE));
> > + ASSERT (mAcpiCpuData.RegisterTable != 0);
> >
> > - CopyRegisterTable (
> > - (CPU_REGISTER_TABLE *)(UINTN)mAcpiCpuData.RegisterTable,
> > - (CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData->RegisterTable,
> > - mAcpiCpuData.NumberOfCpus
> > - );
> > + CopyRegisterTable (
> > + (CPU_REGISTER_TABLE *)(UINTN)mAcpiCpuData.RegisterTable,
> > + (CPU_REGISTER_TABLE *)(UINTN)AcpiCpuData->RegisterTable,
> > + mAcpiCpuData.NumberOfCpus
> > + );
> > + } else {
> > + mAcpiCpuData.RegisterTable = 0;
> > + }
> >
> > //
> > // Copy AP's GDT, IDT and Machine Check handler into SMRAM.
> >
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70046): https://edk2.groups.io/g/devel/message/70046
Mute This Topic: https://groups.io/mt/79496286/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list