* [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c
@ 2021-01-04 5:11 Zeng, Star
0 siblings, 0 replies; 4+ messages in thread
From: Zeng, Star @ 2021-01-04 5:11 UTC (permalink / raw)
To: devel; +Cc: Star Zeng, Ray Ni, Eric Dong, Laszlo Ersek
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@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 71 +++++++++++++++++++++++--------
1 file changed, 54 insertions(+), 17 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 9592430636ec..d87c2efb87e7 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 = (UINT32) MultU64x32 (DestinationRegisterTableList[Index].TableLength, sizeof (CPU_REGISTER_TABLE_ENTRY));
RegisterTableEntry = AllocateCopyPool (
DestinationRegisterTableList[Index].AllocatedSize,
(VOID *)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry
@@ -978,6 +982,31 @@ 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;
+
+ for (Index = 0; Index < NumberOfCpus; Index++) {
+ if (RegisterTable[Index].TableLength != 0) {
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+}
+
/**
Get ACPI CPU data.
@@ -1032,23 +1061,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.
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c
@ 2021-01-04 5:14 Zeng, Star
2021-01-06 6:33 ` Ni, Ray
0 siblings, 1 reply; 4+ messages in thread
From: Zeng, Star @ 2021-01-04 5:14 UTC (permalink / raw)
To: devel; +Cc: Star Zeng, Ray Ni, Eric Dong, Laszlo Ersek
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@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 71 +++++++++++++++++++++++--------
1 file changed, 54 insertions(+), 17 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 9592430636ec..d87c2efb87e7 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 = (UINT32) MultU64x32 (DestinationRegisterTableList[Index].TableLength, sizeof (CPU_REGISTER_TABLE_ENTRY));
RegisterTableEntry = AllocateCopyPool (
DestinationRegisterTableList[Index].AllocatedSize,
(VOID *)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry
@@ -978,6 +982,31 @@ 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;
+
+ for (Index = 0; Index < NumberOfCpus; Index++) {
+ if (RegisterTable[Index].TableLength != 0) {
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+}
+
/**
Get ACPI CPU data.
@@ -1032,23 +1061,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.
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c
2021-01-04 5:14 Zeng, Star
@ 2021-01-06 6:33 ` Ni, Ray
2021-01-06 6:39 ` Zeng, Star
0 siblings, 1 reply; 4+ messages in thread
From: Ni, Ray @ 2021-01-06 6:33 UTC (permalink / raw)
To: Zeng, Star, devel@edk2.groups.io; +Cc: Dong, Eric, Laszlo Ersek
Star,
Just one minor comments:
DestinationRegisterTableList[Index].AllocatedSize = (UINT32) MultU64x32 (DestinationRegisterTableList[Index].TableLength, sizeof (CPU_REGISTER_TABLE_ENTRY));
Please directly use " DestinationRegisterTableList[Index].TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY)".
Thanks,
Ray
> -----Original Message-----
> From: Zeng, Star <star.zeng@intel.com>
> Sent: Monday, January 4, 2021 1:14 PM
> To: devel@edk2.groups.io
> Cc: Zeng, Star <star.zeng@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM
> consumption in CpuS3.c
>
> 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@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 71 +++++++++++++++++++++++------
> --
> 1 file changed, 54 insertions(+), 17 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index 9592430636ec..d87c2efb87e7 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 = (UINT32) MultU64x32
> (DestinationRegisterTableList[Index].TableLength, sizeof
> (CPU_REGISTER_TABLE_ENTRY));
> RegisterTableEntry = AllocateCopyPool (
> DestinationRegisterTableList[Index].AllocatedSize,
> (VOID *)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry
> @@ -978,6 +982,31 @@ 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;
> +
> + for (Index = 0; Index < NumberOfCpus; Index++) {
> + if (RegisterTable[Index].TableLength != 0) {
> + return FALSE;
> + }
> + }
> +
> + return TRUE;
> +}
> +
> /**
> Get ACPI CPU data.
>
> @@ -1032,23 +1061,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.
> --
> 2.21.0.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c
2021-01-06 6:33 ` Ni, Ray
@ 2021-01-06 6:39 ` Zeng, Star
0 siblings, 0 replies; 4+ messages in thread
From: Zeng, Star @ 2021-01-06 6:39 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io; +Cc: Dong, Eric, Laszlo Ersek, Zeng, Star
Good catch, thanks.
It was my over consideration.
Let me send a new version patch.
Thanks,
Star
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, January 6, 2021 2:34 PM
To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
Subject: RE: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c
Star,
Just one minor comments:
DestinationRegisterTableList[Index].AllocatedSize = (UINT32) MultU64x32 (DestinationRegisterTableList[Index].TableLength, sizeof (CPU_REGISTER_TABLE_ENTRY));
Please directly use " DestinationRegisterTableList[Index].TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY)".
Thanks,
Ray
> -----Original Message-----
> From: Zeng, Star <star.zeng@intel.com>
> Sent: Monday, January 4, 2021 1:14 PM
> To: devel@edk2.groups.io
> Cc: Zeng, Star <star.zeng@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption
> in CpuS3.c
>
> 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@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 71 +++++++++++++++++++++++------
> --
> 1 file changed, 54 insertions(+), 17 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index 9592430636ec..d87c2efb87e7 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 = (UINT32)
> + MultU64x32
> (DestinationRegisterTableList[Index].TableLength, sizeof
> (CPU_REGISTER_TABLE_ENTRY));
> RegisterTableEntry = AllocateCopyPool (
> DestinationRegisterTableList[Index].AllocatedSize,
> (VOID
> *)(UINTN)SourceRegisterTableList[Index].RegisterTableEntry
> @@ -978,6 +982,31 @@ 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;
> +
> + for (Index = 0; Index < NumberOfCpus; Index++) {
> + if (RegisterTable[Index].TableLength != 0) {
> + return FALSE;
> + }
> + }
> +
> + return TRUE;
> +}
> +
> /**
> Get ACPI CPU data.
>
> @@ -1032,23 +1061,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.
> --
> 2.21.0.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-06 6:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-04 5:11 [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c Zeng, Star
-- strict thread matches above, loose matches on Subject: below --
2021-01-04 5:14 Zeng, Star
2021-01-06 6:33 ` Ni, Ray
2021-01-06 6:39 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox