public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp
@ 2021-03-09  8:58 Ni, Ray
  0 siblings, 0 replies; 3+ messages in thread
From: Ni, Ray @ 2021-03-09  8:58 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Laszlo Ersek, Rahul Kumar

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199

When Token points to mSmmStartupThisApToken, this routine is called
from SmmStartupThisAp() in non-blocking mode due to
PcdCpuSmmBlockStartupThisAp == FALSE.

In this case, caller wants to startup AP procedure in non-blocking
mode and cannot get the completion status from the Token because there
is no way to return the Token to caller from SmmStartupThisAp().
Caller needs to use its specific way to query the completion status.

There is no need to allocate a token for such case so the 3 overheads
can be avoided:
1. Call AllocateTokenBuffer() when there is no free token.
2. Get a free token from the token buffer.
3. Call ReleaseToken() in APHandler().

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 30 ++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 6227b2428a..4f3c5f60a1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,7 +1,7 @@
 /** @file
 SMM MP service implementation
 
-Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,6 +22,7 @@ UINTN                                       mSemaphoreSize;
 SPIN_LOCK                                   *mPFLock = NULL;
 SMM_CPU_SYNC_MODE                           mCpuSmmSyncMode;
 BOOLEAN                                     mMachineCheckSupported = FALSE;
+MM_COMPLETION                               mSmmStartupThisApToken;
 
 extern UINTN mSmmShadowStackSize;
 
@@ -1240,9 +1241,26 @@ InternalSmmStartupThisAp (
   mSmmMpSyncData->CpuData[CpuIndex].Procedure = Procedure;
   mSmmMpSyncData->CpuData[CpuIndex].Parameter = ProcArguments;
   if (Token != NULL) {
-    ProcToken= GetFreeToken (1);
-    mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
-    *Token = (MM_COMPLETION)ProcToken->SpinLock;
+    if (Token != &mSmmStartupThisApToken) {
+      //
+      // When Token points to mSmmStartupThisApToken, this routine is called
+      // from SmmStartupThisAp() in non-blocking mode (PcdCpuSmmBlockStartupThisAp == FALSE).
+      //
+      // In this case, caller wants to startup AP procedure in non-blocking
+      // mode and cannot get the completion status from the Token because there
+      // is no way to return the Token to caller from SmmStartupThisAp().
+      // Caller needs to use its implementation specific way to query the completion status.
+      //
+      // There is no need to allocate a token for such case so the 3 overheads
+      // can be avoided:
+      // 1. Call AllocateTokenBuffer() when there is no free token.
+      // 2. Get a free token from the token buffer.
+      // 3. Call ReleaseToken() in APHandler().
+      //
+      ProcToken= GetFreeToken (1);
+      mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
+      *Token = (MM_COMPLETION)ProcToken->SpinLock;
+    }
   }
   mSmmMpSyncData->CpuData[CpuIndex].Status    = CpuStatus;
   if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {
@@ -1474,8 +1492,6 @@ SmmStartupThisAp (
   IN OUT  VOID                      *ProcArguments OPTIONAL
   )
 {
-  MM_COMPLETION               Token;
-
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument = ProcArguments;
 
@@ -1486,7 +1502,7 @@ SmmStartupThisAp (
     ProcedureWrapper,
     CpuIndex,
     &gSmmCpuPrivate->ApWrapperFunc[CpuIndex],
-    FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : &Token,
+    FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : &mSmmStartupThisApToken,
     0,
     NULL
     );
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp
@ 2021-03-09  9:09 Ni, Ray
  2021-03-09 14:08 ` Dong, Eric
  0 siblings, 1 reply; 3+ messages in thread
From: Ni, Ray @ 2021-03-09  9:09 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Laszlo Ersek, Rahul Kumar

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199

When Token points to mSmmStartupThisApToken, this routine is called
from SmmStartupThisAp() in non-blocking mode due to
PcdCpuSmmBlockStartupThisAp == FALSE.

In this case, caller wants to startup AP procedure in non-blocking
mode and cannot get the completion status from the Token because there
is no way to return the Token to caller from SmmStartupThisAp().
Caller needs to use its specific way to query the completion status.

There is no need to allocate a token for such case so the 3 overheads
can be avoided:
1. Call AllocateTokenBuffer() when there is no free token.
2. Get a free token from the token buffer.
3. Call ReleaseToken() in APHandler().

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 30 ++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 6227b2428a..4f3c5f60a1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,7 +1,7 @@
 /** @file
 SMM MP service implementation
 
-Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,6 +22,7 @@ UINTN                                       mSemaphoreSize;
 SPIN_LOCK                                   *mPFLock = NULL;
 SMM_CPU_SYNC_MODE                           mCpuSmmSyncMode;
 BOOLEAN                                     mMachineCheckSupported = FALSE;
+MM_COMPLETION                               mSmmStartupThisApToken;
 
 extern UINTN mSmmShadowStackSize;
 
@@ -1240,9 +1241,26 @@ InternalSmmStartupThisAp (
   mSmmMpSyncData->CpuData[CpuIndex].Procedure = Procedure;
   mSmmMpSyncData->CpuData[CpuIndex].Parameter = ProcArguments;
   if (Token != NULL) {
-    ProcToken= GetFreeToken (1);
-    mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
-    *Token = (MM_COMPLETION)ProcToken->SpinLock;
+    if (Token != &mSmmStartupThisApToken) {
+      //
+      // When Token points to mSmmStartupThisApToken, this routine is called
+      // from SmmStartupThisAp() in non-blocking mode (PcdCpuSmmBlockStartupThisAp == FALSE).
+      //
+      // In this case, caller wants to startup AP procedure in non-blocking
+      // mode and cannot get the completion status from the Token because there
+      // is no way to return the Token to caller from SmmStartupThisAp().
+      // Caller needs to use its implementation specific way to query the completion status.
+      //
+      // There is no need to allocate a token for such case so the 3 overheads
+      // can be avoided:
+      // 1. Call AllocateTokenBuffer() when there is no free token.
+      // 2. Get a free token from the token buffer.
+      // 3. Call ReleaseToken() in APHandler().
+      //
+      ProcToken= GetFreeToken (1);
+      mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;
+      *Token = (MM_COMPLETION)ProcToken->SpinLock;
+    }
   }
   mSmmMpSyncData->CpuData[CpuIndex].Status    = CpuStatus;
   if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {
@@ -1474,8 +1492,6 @@ SmmStartupThisAp (
   IN OUT  VOID                      *ProcArguments OPTIONAL
   )
 {
-  MM_COMPLETION               Token;
-
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;
   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument = ProcArguments;
 
@@ -1486,7 +1502,7 @@ SmmStartupThisAp (
     ProcedureWrapper,
     CpuIndex,
     &gSmmCpuPrivate->ApWrapperFunc[CpuIndex],
-    FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : &Token,
+    FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : &mSmmStartupThisApToken,
     0,
     NULL
     );
-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp
  2021-03-09  9:09 [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp Ni, Ray
@ 2021-03-09 14:08 ` Dong, Eric
  0 siblings, 0 replies; 3+ messages in thread
From: Dong, Eric @ 2021-03-09 14:08 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io; +Cc: Laszlo Ersek, Kumar, Rahul1

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, March 9, 2021 5:09 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199

When Token points to mSmmStartupThisApToken, this routine is called
from SmmStartupThisAp() in non-blocking mode due to
PcdCpuSmmBlockStartupThisAp == FALSE.

In this case, caller wants to startup AP procedure in non-blocking
mode and cannot get the completion status from the Token because there
is no way to return the Token to caller from SmmStartupThisAp().
Caller needs to use its specific way to query the completion status.

There is no need to allocate a token for such case so the 3 overheads
can be avoided:
1. Call AllocateTokenBuffer() when there is no free token.
2. Get a free token from the token buffer.
3. Call ReleaseToken() in APHandler().

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 30 ++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 6227b2428a..4f3c5f60a1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,7 +1,7 @@
 /** @file

 SMM MP service implementation

 

-Copyright (c) 2009 - 2020, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>

 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>

 

 SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -22,6 +22,7 @@ UINTN                                       mSemaphoreSize;
 SPIN_LOCK                                   *mPFLock = NULL;

 SMM_CPU_SYNC_MODE                           mCpuSmmSyncMode;

 BOOLEAN                                     mMachineCheckSupported = FALSE;

+MM_COMPLETION                               mSmmStartupThisApToken;

 

 extern UINTN mSmmShadowStackSize;

 

@@ -1240,9 +1241,26 @@ InternalSmmStartupThisAp (
   mSmmMpSyncData->CpuData[CpuIndex].Procedure = Procedure;

   mSmmMpSyncData->CpuData[CpuIndex].Parameter = ProcArguments;

   if (Token != NULL) {

-    ProcToken= GetFreeToken (1);

-    mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;

-    *Token = (MM_COMPLETION)ProcToken->SpinLock;

+    if (Token != &mSmmStartupThisApToken) {

+      //

+      // When Token points to mSmmStartupThisApToken, this routine is called

+      // from SmmStartupThisAp() in non-blocking mode (PcdCpuSmmBlockStartupThisAp == FALSE).

+      //

+      // In this case, caller wants to startup AP procedure in non-blocking

+      // mode and cannot get the completion status from the Token because there

+      // is no way to return the Token to caller from SmmStartupThisAp().

+      // Caller needs to use its implementation specific way to query the completion status.

+      //

+      // There is no need to allocate a token for such case so the 3 overheads

+      // can be avoided:

+      // 1. Call AllocateTokenBuffer() when there is no free token.

+      // 2. Get a free token from the token buffer.

+      // 3. Call ReleaseToken() in APHandler().

+      //

+      ProcToken= GetFreeToken (1);

+      mSmmMpSyncData->CpuData[CpuIndex].Token = ProcToken;

+      *Token = (MM_COMPLETION)ProcToken->SpinLock;

+    }

   }

   mSmmMpSyncData->CpuData[CpuIndex].Status    = CpuStatus;

   if (mSmmMpSyncData->CpuData[CpuIndex].Status != NULL) {

@@ -1474,8 +1492,6 @@ SmmStartupThisAp (
   IN OUT  VOID                      *ProcArguments OPTIONAL

   )

 {

-  MM_COMPLETION               Token;

-

   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;

   gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument = ProcArguments;

 

@@ -1486,7 +1502,7 @@ SmmStartupThisAp (
     ProcedureWrapper,

     CpuIndex,

     &gSmmCpuPrivate->ApWrapperFunc[CpuIndex],

-    FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : &Token,

+    FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : &mSmmStartupThisApToken,

     0,

     NULL

     );

-- 
2.27.0.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-03-09 14:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-09  9:09 [PATCH v3] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp Ni, Ray
2021-03-09 14:08 ` Dong, Eric
  -- strict thread matches above, loose matches on Subject: below --
2021-03-09  8:58 Ni, Ray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox