public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: devel@edk2.groups.io
Cc: Star Zeng <star.zeng@intel.com>, Ray Ni <ray.ni@intel.com>,
	Laszlo Ersek <lersek@redhat.com>, Eric Dong <eric.dong@intel.com>
Subject: [PATCH V4] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c
Date: Mon, 11 Jan 2021 09:54:19 +0800	[thread overview]
Message-ID: <20210111015419.28368-1-star.zeng@intel.com> (raw)

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>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---

Notes:
   V2: Use "DestinationRegisterTableList[Index].TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY)" directly to cover Ray's comment.
   V3: Handle "RegisterTable == NULL" case to cover Laszlo's comment.
   V4: Add @param[in] for NumberOfCpus parameter of IsRegisterTableEmpty().

 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 74 ++++++++++++++++++++++++-------
 1 file changed, 57 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index 9592430636ec..ab7f39aa2bd4 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,34 @@ CopyRegisterTable (
   }
 }
 
+/**
+  Check whether the register table is empty or not.
+
+  @param[in] RegisterTable  Point to the register table.
+  @param[in] NumberOfCpus   Number of CPUs.
+
+  @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 +1064,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


             reply	other threads:[~2021-01-11  1:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-11  1:54 Zeng, Star [this message]
2021-01-11  9:23 ` [edk2-devel] [PATCH V4] UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM consumption in CpuS3.c Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210111015419.28368-1-star.zeng@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox