From: "Zeng, Star" <star.zeng@intel.com>
To: "Dong, Eric" <eric.dong@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
"Yao, Jiewen" <jiewen.yao@intel.com>,
"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code.
Date: Thu, 28 Sep 2017 09:30:44 +0000 [thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B97C4C7@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1506590107-7324-3-git-send-email-eric.dong@intel.com>
Just FYI, another idea is to declare SetProcessorRegister() like below, then the caller of SetProcessorRegister() has no need to touch mAcpiCpuData.
VOID
SetProcessorRegister (
IN BOOLEAN PreSmmFlag
)
{
CPU_REGISTER_TABLE *RegisterTableList;
...
if (PreSmmFlag) {
RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable;
} else {
RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable;
}
...
Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Eric Dong
Sent: Thursday, September 28, 2017 5:15 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: [edk2] [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code.
Refine code to avoid duplicate code to set processor register.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 78 ++++++++++-----------------------------
1 file changed, 20 insertions(+), 58 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index ae4b516..500a0e2 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -208,18 +208,28 @@ Returns:
This function programs registers for the calling processor.
- @param RegisterTable Pointer to register table of the running processor.
+ @param RegisterTableList Pointer to register table of the running processor.
**/
VOID
SetProcessorRegister (
- IN CPU_REGISTER_TABLE *RegisterTable
+ IN CPU_REGISTER_TABLE *RegisterTableList
)
{
CPU_REGISTER_TABLE_ENTRY *RegisterTableEntry;
UINTN Index;
UINTN Value;
SPIN_LOCK *MsrSpinLock;
+ UINT32 InitApicId;
+ CPU_REGISTER_TABLE *RegisterTable;
+
+ InitApicId = GetInitialApicId ();
+ for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) {
+ if (RegisterTableList[Index].InitialApicId == InitApicId) {
+ RegisterTable = &RegisterTableList[Index];
+ break;
+ }
+ }
//
// Traverse Register Table of this logical processor @@ -347,8 +357,6 @@ SetProcessorRegister (
}
}
-
-
/**
AP initialization before then after SMBASE relocation in the S3 boot path.
**/
@@ -357,26 +365,12 @@ MPRendezvousProcedure (
VOID
)
{
- CPU_REGISTER_TABLE *RegisterTableList;
- UINT32 InitApicId;
- UINTN Index;
UINTN TopOfStack;
UINT8 Stack[128];
LoadMtrrData (mAcpiCpuData.MtrrTable);
- //
- // Find processor number for this CPU.
- //
- RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable;
- InitApicId = GetInitialApicId ();
- for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) {
- if (RegisterTableList[Index].InitialApicId == InitApicId) {
- SetProcessorRegister (&RegisterTableList[Index]);
- break;
- }
- }
-
+ SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN)
+ mAcpiCpuData.PreSmmInitRegisterTable);
//
// Count down the number with lock mechanism.
@@ -393,14 +387,7 @@ MPRendezvousProcedure (
ProgramVirtualWireMode ();
DisableLvtInterrupts ();
- RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable;
- InitApicId = GetInitialApicId ();
- for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) {
- if (RegisterTableList[Index].InitialApicId == InitApicId) {
- SetProcessorRegister (&RegisterTableList[Index]);
- break;
- }
- }
+ SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN)
+ mAcpiCpuData.RegisterTable);
//
// Place AP into the safe code, count down the number with lock mechanism in the safe code.
@@ -475,27 +462,13 @@ PrepareApStartupVector (
**/
VOID
-EarlyInitializeCpu (
+InitializeCpuBeforeRebase (
VOID
)
{
- CPU_REGISTER_TABLE *RegisterTableList;
- UINT32 InitApicId;
- UINTN Index;
-
LoadMtrrData (mAcpiCpuData.MtrrTable);
- //
- // Find processor number for this CPU.
- //
- RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.PreSmmInitRegisterTable;
- InitApicId = GetInitialApicId ();
- for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) {
- if (RegisterTableList[Index].InitialApicId == InitApicId) {
- SetProcessorRegister (&RegisterTableList[Index]);
- break;
- }
- }
+ SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN)
+ mAcpiCpuData.PreSmmInitRegisterTable);
ProgramVirtualWireMode ();
@@ -527,22 +500,11 @@ EarlyInitializeCpu (
**/
VOID
-InitializeCpu (
+InitializeCpuAfterRebase (
VOID
)
{
- CPU_REGISTER_TABLE *RegisterTableList;
- UINT32 InitApicId;
- UINTN Index;
-
- RegisterTableList = (CPU_REGISTER_TABLE *) (UINTN) mAcpiCpuData.RegisterTable;
- InitApicId = GetInitialApicId ();
- for (Index = 0; Index < mAcpiCpuData.NumberOfCpus; Index++) {
- if (RegisterTableList[Index].InitialApicId == InitApicId) {
- SetProcessorRegister (&RegisterTableList[Index]);
- break;
- }
- }
+ SetProcessorRegister ((CPU_REGISTER_TABLE *) (UINTN)
+ mAcpiCpuData.RegisterTable);
mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1;
@@ -660,7 +622,7 @@ SmmRestoreCpu (
//
// First time microcode load and restore MTRRs
//
- EarlyInitializeCpu ();
+ InitializeCpuBeforeRebase ();
}
//
@@ -675,7 +637,7 @@ SmmRestoreCpu (
//
// Restore MSRs for BSP and all APs
//
- InitializeCpu ();
+ InitializeCpuAfterRebase ();
}
//
--
2.7.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2017-09-28 9:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-28 9:15 [Patch 0/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enhance S3 code Eric Dong
2017-09-28 9:15 ` [Patch 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: Combine INIT-SIPI-SIPI Eric Dong
2017-09-28 9:15 ` [Patch 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Refine code to avoid duplicated code Eric Dong
2017-09-28 9:30 ` Zeng, Star [this message]
2017-09-28 10:04 ` Ni, Ruiyu
2017-09-28 11:08 ` Yao, Jiewen
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=0C09AFA07DD0434D9E2A0C6AEB0483103B97C4C7@shsmsx102.ccr.corp.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