* Re: [edk2-devel] [PATCH v2] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp
[not found] <166AA0D71171F5F0.18552@groups.io>
@ 2021-03-09 9:18 ` Ni, Ray
0 siblings, 0 replies; only message in thread
From: Ni, Ray @ 2021-03-09 9:18 UTC (permalink / raw)
To: devel@edk2.groups.io, Ni, Ray; +Cc: Dong, Eric, Laszlo Ersek, Kumar, Rahul1
[-- Attachment #1: Type: text/plain, Size: 5501 bytes --]
I don't want to break the community rule but somehow "git send-email" cannot send out the V3 patch out.
To avoid reviewers spending time reviewing v2 patch, I have to send out the v3 as attachment ASAP.
Meanwhile, I will investigate what happened to my system and avoid sending attachments in future.
Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Tuesday, March 9, 2021 4:58 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: [edk2-devel] [PATCH v2] 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 | 27
> ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 6227b2428a..91452ec398 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,23 @@ 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 overhead of
> SMRAM and
>
> + // the allocation operation can be avoided.
>
> + //
>
> + 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 +1489,6 @@ SmmStartupThisAp (
> IN OUT VOID *ProcArguments OPTIONAL
>
> )
>
> {
>
> - MM_COMPLETION Token;
>
> -
>
> gSmmCpuPrivate->ApWrapperFunc[CpuIndex].Procedure = Procedure;
>
> gSmmCpuPrivate->ApWrapperFunc[CpuIndex].ProcedureArgument =
> ProcArguments;
>
>
>
> @@ -1486,7 +1499,7 @@ SmmStartupThisAp (
> ProcedureWrapper,
>
> CpuIndex,
>
> &gSmmCpuPrivate->ApWrapperFunc[CpuIndex],
>
> - FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL : &Token,
>
> + FeaturePcdGet (PcdCpuSmmBlockStartupThisAp) ? NULL :
> &mSmmStartupThisApToken,
>
> 0,
>
> NULL
>
> );
>
> --
> 2.27.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#72579): https://edk2.groups.io/g/devel/message/72579
> Mute This Topic: https://groups.io/mt/81197093/1712937
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
> -=-=-=-=-=-=
>
>
[-- Attachment #2: 0001-UefiCpuPkg-PiSmmCpu-Don-t-allocate-Token-for-SmmStar.patch --]
[-- Type: application/octet-stream, Size: 4313 bytes --]
From 093d9fb8d2f2d58bf9908222134250f1cba89d9d Mon Sep 17 00:00:00 2001
From: Ray Ni <ray.ni@intel.com>
Date: Wed, 27 Jan 2021 15:45:25 +0800
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] only message in thread
only message in thread, other threads:[~2021-03-09 9:18 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <166AA0D71171F5F0.18552@groups.io>
2021-03-09 9:18 ` [edk2-devel] [PATCH v2] UefiCpuPkg/PiSmmCpu: Don't allocate Token for SmmStartupThisAp Ni, Ray
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox