public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ray" <ray.ni@intel.com>
To: "Wu, Jiaxin" <jiaxin.wu@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Zeng, Star" <star.zeng@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"Kumar, Rahul R" <rahul.r.kumar@intel.com>
Subject: Re: [edk2-devel] [PATCH v4 05/14] UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable
Date: Mon, 6 May 2024 03:30:42 +0000	[thread overview]
Message-ID: <MN6PR11MB8244127B0C2FF76A9F606B3A8C1C2@MN6PR11MB8244.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20240426121720.54308-6-jiaxin.wu@intel.com>

[-- Attachment #1: Type: text/plain, Size: 6941 bytes --]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
________________________________
From: Wu, Jiaxin <jiaxin.wu@intel.com>
Sent: Friday, April 26, 2024 20:17
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
Subject: [PATCH v4 05/14] UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable

This patch aims on mProcessorInfo global variable, which can be
defined as local variable in SmmRelocateBases(). With this patch,
no need to allocate the memory for all CPUs to store the
Processor Info.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zeng Star <star.zeng@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 .../Library/SmmRelocationLib/SmmRelocationLib.c    | 88 ++++++++--------------
 1 file changed, 32 insertions(+), 56 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
index 3694a07cbb..86df66a280 100644
--- a/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
+++ b/UefiCpuPkg/Library/SmmRelocationLib/SmmRelocationLib.c
@@ -14,15 +14,10 @@
 #include "InternalSmmRelocationLib.h"

 UINTN  mMaxNumberOfCpus = 1;
 UINTN  mNumberOfCpus    = 1;

-//
-// Record all Processors Info
-//
-EFI_PROCESSOR_INFORMATION  *mProcessorInfo = NULL;
-
 //
 // IDT used during SMM Init
 //
 IA32_DESCRIPTOR  gcSmmInitIdtr;

@@ -34,10 +29,15 @@ UINT64  mSmBase;
 //
 // SmBase Rebased flag for current CPU
 //
 volatile BOOLEAN  mRebased;

+//
+// CpuIndex for current CPU
+//
+UINTN  mCpuIndex;
+
 /**
   This function will get the SmBase for CpuIndex.

   @param[in]   CpuIndex            The processor index.
   @param[in]   SmmRelocationStart  The start address of Smm relocated memory in SMRAM.
@@ -138,37 +138,26 @@ VOID
 EFIAPI
 SmmInitHandler (
   VOID
   )
 {
-  UINT32  ApicId;
-  UINTN   Index;
-
   //
   // Update SMM IDT entries' code segment and load IDT
   //
   AsmWriteIdtr (&gcSmmInitIdtr);
-  ApicId = GetApicId ();

-  for (Index = 0; Index < mNumberOfCpus; Index++) {
-    if (ApicId == (UINT32)mProcessorInfo[Index].ProcessorId) {
-      //
-      // Configure SmBase.
-      //
-      ConfigureSmBase (mSmBase);
-
-      //
-      // Hook return after RSM to set SMM re-based flag
-      // SMM re-based flag can't be set before RSM, because SMM save state context might be override
-      // by next AP flow before it take effect.
-      //
-      SemaphoreHook (Index, &mRebased);
-      return;
-    }
-  }
+  //
+  // Configure SmBase.
+  //
+  ConfigureSmBase (mSmBase);

-  ASSERT (FALSE);
+  //
+  // Hook return after RSM to set SMM re-based flag
+  // SMM re-based flag can't be set before RSM, because SMM save state context might be override
+  // by next AP flow before it take effect.
+  //
+  SemaphoreHook (mCpuIndex, &mRebased);
 }

 /**
   Relocate SmmBases for each processor.
   Execute on first boot and all S3 resumes
@@ -185,17 +174,19 @@ SmmRelocateBases (
   IN EDKII_PEI_MP_SERVICES2_PPI  *MpServices2,
   IN EFI_PHYSICAL_ADDRESS        SmmRelocationStart,
   IN UINTN                       TileSize
   )
 {
-  UINT8                 BakBuf[BACK_BUF_SIZE];
-  SMRAM_SAVE_STATE_MAP  BakBuf2;
-  SMRAM_SAVE_STATE_MAP  *CpuStatePtr;
-  UINT8                 *U8Ptr;
-  UINTN                 Index;
-  UINTN                 BspIndex;
-  UINT32                BspApicId;
+  EFI_STATUS                 Status;
+  UINT8                      BakBuf[BACK_BUF_SIZE];
+  SMRAM_SAVE_STATE_MAP       BakBuf2;
+  SMRAM_SAVE_STATE_MAP       *CpuStatePtr;
+  UINT8                      *U8Ptr;
+  UINTN                      Index;
+  UINTN                      BspIndex;
+  UINT32                     BspApicId;
+  EFI_PROCESSOR_INFORMATION  ProcessorInfo;

   //
   // Make sure the reserved size is large enough for procedure SmmInitTemplate.
   //
   ASSERT (sizeof (BakBuf) >= gcSmmInitSize);
@@ -230,14 +221,18 @@ SmmRelocateBases (
   // Relocate SM bases for all APs
   // This is APs' 1st SMI - rebase will be done here, and APs' default SMI handler will be overridden by gcSmmInitTemplate
   //
   BspIndex = (UINTN)-1;
   for (Index = 0; Index < mNumberOfCpus; Index++) {
-    if (BspApicId != (UINT32)mProcessorInfo[Index].ProcessorId) {
-      mRebased = FALSE;
-      mSmBase  = GetSmBase (Index, SmmRelocationStart, TileSize);
-      SendSmiIpi ((UINT32)mProcessorInfo[Index].ProcessorId);
+    Status = MpServices2->GetProcessorInfo (MpServices2, Index | CPU_V2_EXTENDED_TOPOLOGY, &ProcessorInfo);
+    ASSERT_EFI_ERROR (Status);
+
+    if (BspApicId != (UINT32)ProcessorInfo.ProcessorId) {
+      mRebased  = FALSE;
+      mSmBase   = GetSmBase (Index, SmmRelocationStart, TileSize);
+      mCpuIndex = Index;
+      SendSmiIpi ((UINT32)ProcessorInfo.ProcessorId);
       //
       // Wait for this AP to finish its 1st SMI
       //
       while (!mRebased) {
       }
@@ -443,11 +438,10 @@ SmmRelocationInit (
   UINTN                 TileSize;
   UINT64                SmmRelocationSize;
   EFI_PHYSICAL_ADDRESS  SmmRelocationStart;
   UINTN                 SmmStackSize;
   UINT8                 *SmmStacks;
-  UINTN                 Index;

   SmmRelocationStart = 0;
   SmmStacks          = NULL;

   DEBUG ((DEBUG_INFO, "SmmRelocationInit Start \n"));
@@ -473,28 +467,10 @@ SmmRelocationInit (
     mMaxNumberOfCpus = mNumberOfCpus;
   }

   ASSERT (mNumberOfCpus <= mMaxNumberOfCpus);

-  //
-  // Retrieve the Processor Info for all CPUs
-  //
-  mProcessorInfo = (EFI_PROCESSOR_INFORMATION *)AllocatePool (sizeof (EFI_PROCESSOR_INFORMATION) * mMaxNumberOfCpus);
-  if (mProcessorInfo == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto ON_EXIT;
-  }
-
-  for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
-    if (Index < mNumberOfCpus) {
-      Status = MpServices2->GetProcessorInfo (MpServices2, Index | CPU_V2_EXTENDED_TOPOLOGY, &mProcessorInfo[Index]);
-      if (EFI_ERROR (Status)) {
-        goto ON_EXIT;
-      }
-    }
-  }
-
   //
   // Calculate SmmRelocationSize for all of the tiles.
   //
   // The CPU save state and code for the SMI entry point are tiled within an SMRAM
   // allocated buffer. The minimum size of this buffer for a uniprocessor system
--
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118586): https://edk2.groups.io/g/devel/message/118586
Mute This Topic: https://groups.io/mt/105748916/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 13040 bytes --]

  reply	other threads:[~2024-05-06  3:30 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-26 12:17 [edk2-devel] [PATCH v4 00/14] Add SmmRelocationLib Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 01/14] UefiCpuPkg: Add SmmRelocationLib class Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 02/14] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance Wu, Jiaxin
2024-04-30  5:58   ` Ni, Ray
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 03/14] UefiCpuPkg/SmmRelocationLib: Rename global variables Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 04/14] UefiCpuPkg/SmmRelocationLib: Avoid unnecessary memory allocation Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 05/14] UefiCpuPkg/SmmRelocationLib: Remove unnecessary global variable Wu, Jiaxin
2024-05-06  3:30   ` Ni, Ray [this message]
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 06/14] UefiCpuPkg/SmmRelocationLib: Remove unnecessary CpuIndex Wu, Jiaxin
2024-05-06  3:32   ` Ni, Ray
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 07/14] UefiCpuPkg/SmmRelocationLib: Add library instance for AMD Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 08/14] OvmfPkg/SmmRelocationLib: Add library instance for OVMF Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 09/14] OvmfPkg/PlatformInitLib: Create gEfiSmmSmramMemoryGuid Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 10/14] OvmfPkg: Refine SmmAccess implementation Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 11/14] OvmfPkg/SmmCpuFeaturesLib: Check Smbase Relocation is done or not Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 12/14] OvmfPkg/PlatformPei: Relocate SmBases in PEI phase Wu, Jiaxin
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 13/14] UefiPayloadPkg/UefiPayloadPkg.dsc: Include SmmRelocationLib Wu, Jiaxin
2024-04-26 12:46   ` Guo, Gua
2024-04-26 12:17 ` [edk2-devel] [PATCH v4 14/14] UefiCpuPkg/PiSmmCpuDxeSmm: Remove SmBases relocation logic Wu, Jiaxin
2024-05-06  3:38   ` Ni, Ray
2024-04-30  6:00 ` [edk2-devel] [PATCH v4 00/14] Add SmmRelocationLib Ni, Ray
2024-04-30 10:14   ` Wu, Jiaxin
2024-05-07  3:39   ` Wu, Jiaxin
2024-05-07  4:59     ` Yao, Jiewen
2024-05-07  5:18       ` Wu, Jiaxin
2024-05-08 16:43         ` Ard Biesheuvel
2024-05-09  7:36           ` 回复: " gaoliming via groups.io
2024-05-09  7:43             ` Ard Biesheuvel
2024-04-30  9:58 ` Gerd Hoffmann

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=MN6PR11MB8244127B0C2FF76A9F606B3A8C1C2@MN6PR11MB8244.namprd11.prod.outlook.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