UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 67 ++++++++++++++++++-- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 +++++ UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 + UefiCpuPkg/UefiCpuPkg.dec | 4 ++ UefiCpuPkg/UefiCpuPkg.uni | 3 + 5 files changed, 88 insertions(+), 4 deletions(-)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388
Token is new introduced by MM MP Protocol. Current logic allocate Token
every time when need to use it. The logic caused SMI latency raised to
very high. Update logic to allocate Token buffer at driver's entry point.
Later use the token from the allocated token buffer. Only when all the
buffer have been used, then need to allocate new buffer.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
V5 changes:
Refine PCD names and some code comments.
V4 changes:
Specify PCD type to FixedPcd in code.
V3 changes:
Introduce PCD to control the pre allocated toke buffer size.
v2 changes:
Minor update based on comments.
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 67 ++++++++++++++++++--
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 +++++
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +
UefiCpuPkg/UefiCpuPkg.dec | 4 ++
UefiCpuPkg/UefiCpuPkg.uni | 3 +
5 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 0685637c2b..757f1056f7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -492,6 +492,24 @@ FreeTokens (
{
LIST_ENTRY *Link;
PROCEDURE_TOKEN *ProcToken;
+ TOKEN_BUFFER *TokenBuf;
+
+ //
+ // Only free the token buffer recorded in the OldTOkenBufList
+ // upon exiting SMI. Current token buffer stays allocated so
+ // next SMI doesn't need to re-allocate.
+ //
+ gSmmCpuPrivate->UsedTokenNum = 0;
+
+ Link = GetFirstNode (&gSmmCpuPrivate->OldTokenBufList);
+ while (!IsNull (&gSmmCpuPrivate->OldTokenBufList, Link)) {
+ TokenBuf = TOKEN_BUFFER_FROM_LINK (Link);
+
+ Link = RemoveEntryList (&TokenBuf->Link);
+
+ FreePool (TokenBuf->Buffer);
+ FreePool (TokenBuf);
+ }
while (!IsListEmpty (&gSmmCpuPrivate->TokenList)) {
Link = GetFirstNode (&gSmmCpuPrivate->TokenList);
@@ -499,7 +517,6 @@ FreeTokens (
RemoveEntryList (&ProcToken->Link);
- FreePool ((VOID *)ProcToken->ProcedureToken);
FreePool (ProcToken);
}
}
@@ -1115,13 +1132,37 @@ CreateToken (
VOID
)
{
- PROCEDURE_TOKEN *ProcToken;
+ PROCEDURE_TOKEN *ProcToken;
SPIN_LOCK *CpuToken;
UINTN SpinLockSize;
+ TOKEN_BUFFER *TokenBuf;
+ UINT32 TokenCountPerChunk;
SpinLockSize = GetSpinLockProperties ();
- CpuToken = AllocatePool (SpinLockSize);
- ASSERT (CpuToken != NULL);
+ TokenCountPerChunk = FixedPcdGet32 (PcdCpuSmmMpTokenCountPerChunk);
+
+ if (gSmmCpuPrivate->UsedTokenNum == TokenCountPerChunk) {
+ DEBUG ((DEBUG_VERBOSE, "CpuSmm: No free token buffer, allocate new buffer!\n"));
+
+ //
+ // Record current token buffer for later free action usage.
+ // Current used token buffer not in this list.
+ //
+ TokenBuf = AllocatePool (sizeof (TOKEN_BUFFER));
+ ASSERT (TokenBuf != NULL);
+ TokenBuf->Signature = TOKEN_BUFFER_SIGNATURE;
+ TokenBuf->Buffer = gSmmCpuPrivate->CurrentTokenBuf;
+
+ InsertTailList (&gSmmCpuPrivate->OldTokenBufList, &TokenBuf->Link);
+
+ gSmmCpuPrivate->CurrentTokenBuf = AllocatePool (SpinLockSize * TokenCountPerChunk);
+ ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL);
+ gSmmCpuPrivate->UsedTokenNum = 0;
+ }
+
+ CpuToken = (SPIN_LOCK *)(gSmmCpuPrivate->CurrentTokenBuf + SpinLockSize * gSmmCpuPrivate->UsedTokenNum);
+ gSmmCpuPrivate->UsedTokenNum++;
+
InitializeSpinLock (CpuToken);
AcquireSpinLock (CpuToken);
@@ -1732,10 +1773,28 @@ InitializeDataForMmMp (
VOID
)
{
+ UINTN SpinLockSize;
+ UINT32 TokenCountPerChunk;
+
+ SpinLockSize = GetSpinLockProperties ();
+ TokenCountPerChunk = FixedPcdGet32 (PcdCpuSmmMpTokenCountPerChunk);
+ ASSERT (TokenCountPerChunk != 0);
+ if (TokenCountPerChunk == 0) {
+ DEBUG ((DEBUG_ERROR, "PcdCpuSmmMpTokenCountPerChunk should not be Zero!\n"));
+ CpuDeadLoop ();
+ }
+ DEBUG ((DEBUG_INFO, "CpuSmm: SpinLock Size = 0x%x, PcdCpuSmmMpTokenCountPerChunk = 0x%x\n", SpinLockSize, TokenCountPerChunk));
+
+ gSmmCpuPrivate->CurrentTokenBuf = AllocatePool (SpinLockSize * TokenCountPerChunk);
+ ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL);
+
+ gSmmCpuPrivate->UsedTokenNum = 0;
+
gSmmCpuPrivate->ApWrapperFunc = AllocatePool (sizeof (PROCEDURE_WRAPPER) * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
ASSERT (gSmmCpuPrivate->ApWrapperFunc != NULL);
InitializeListHead (&gSmmCpuPrivate->TokenList);
+ InitializeListHead (&gSmmCpuPrivate->OldTokenBufList);
}
/**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 7e7c73f27f..5c1a01e42b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -217,6 +217,17 @@ typedef struct {
#define PROCEDURE_TOKEN_FROM_LINK(a) CR (a, PROCEDURE_TOKEN, Link, PROCEDURE_TOKEN_SIGNATURE)
+#define TOKEN_BUFFER_SIGNATURE SIGNATURE_32 ('T', 'K', 'B', 'S')
+
+typedef struct {
+ UINTN Signature;
+ LIST_ENTRY Link;
+
+ UINT8 *Buffer;
+} TOKEN_BUFFER;
+
+#define TOKEN_BUFFER_FROM_LINK(a) CR (a, TOKEN_BUFFER, Link, TOKEN_BUFFER_SIGNATURE)
+
//
// Private structure for the SMM CPU module that is stored in DXE Runtime memory
// Contains the SMM Configuration Protocols that is produced.
@@ -243,6 +254,10 @@ typedef struct {
PROCEDURE_WRAPPER *ApWrapperFunc;
LIST_ENTRY TokenList;
+ LIST_ENTRY OldTokenBufList;
+
+ UINT8 *CurrentTokenBuf;
+ UINT32 UsedTokenNum; // Only record tokens used in CurrentTokenBuf.
} SMM_CPU_PRIVATE_DATA;
extern SMM_CPU_PRIVATE_DATA *gSmmCpuPrivate;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index b12b2691f8..76b1462996 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -140,6 +140,9 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES
gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask ## CONSUMES
+[FixedPcd]
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmMpTokenCountPerChunk ## CONSUMES
+
[Pcd.X64]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmRestrictedMemoryAccess ## CONSUMES
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 12f4413ea5..797f948631 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -148,6 +148,10 @@
# @Prompt Specify size of good stack of exception which need switching stack.
gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize|2048|UINT32|0x30002001
+ ## Count of pre allocated SMM MP tokens per chunk.
+ # @Prompt Specify the count of pre allocated SMM MP tokens per chunk.
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmMpTokenCountPerChunk|64|UINT32|0x30002002
+
[PcdsFixedAtBuild, PcdsPatchableInModule]
## This value is the CPU Local APIC base address, which aligns the address on a 4-KByte boundary.
# @Prompt Configure base address of CPU Local APIC
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index bfd696f48c..442ce5cb85 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -272,3 +272,6 @@
"24000000 - 6th and 7th generation Intel Core processors and Intel Xeon W Processor Family(24MHz).<BR>\n"
"19200000 - Intel Atom processors based on Goldmont Microarchitecture with CPUID signature 06_5CH(19.2MHz).<BR>\n"
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_PROMPT #language en-US "Specify the count of pre allocated SMM MP tokens per chunk.\n"
+
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_HELP #language en-US "This value used to specify the count of pre allocated SMM MP tokens per chunk.\n"
\ No newline at end of file
--
2.23.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51768): https://edk2.groups.io/g/devel/message/51768
Mute This Topic: https://groups.io/mt/66924873/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Dong, Eric <eric.dong@intel.com>
> Sent: Thursday, December 5, 2019 3:08 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH v5] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid allocate Token
> every time
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388
>
> Token is new introduced by MM MP Protocol. Current logic allocate Token
> every time when need to use it. The logic caused SMI latency raised to
> very high. Update logic to allocate Token buffer at driver's entry point.
> Later use the token from the allocated token buffer. Only when all the
> buffer have been used, then need to allocate new buffer.
>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
>
> V5 changes:
>
> Refine PCD names and some code comments.
>
> V4 changes:
>
> Specify PCD type to FixedPcd in code.
>
> V3 changes:
>
> Introduce PCD to control the pre allocated toke buffer size.
>
> v2 changes:
>
> Minor update based on comments.
>
>
> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 67
> ++++++++++++++++++--
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 +++++
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +
> UefiCpuPkg/UefiCpuPkg.dec | 4 ++
> UefiCpuPkg/UefiCpuPkg.uni | 3 +
> 5 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 0685637c2b..757f1056f7 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -492,6 +492,24 @@ FreeTokens (
> {
>
> LIST_ENTRY *Link;
>
> PROCEDURE_TOKEN *ProcToken;
>
> + TOKEN_BUFFER *TokenBuf;
>
> +
>
> + //
>
> + // Only free the token buffer recorded in the OldTOkenBufList
>
> + // upon exiting SMI. Current token buffer stays allocated so
>
> + // next SMI doesn't need to re-allocate.
>
> + //
>
> + gSmmCpuPrivate->UsedTokenNum = 0;
>
> +
>
> + Link = GetFirstNode (&gSmmCpuPrivate->OldTokenBufList);
>
> + while (!IsNull (&gSmmCpuPrivate->OldTokenBufList, Link)) {
>
> + TokenBuf = TOKEN_BUFFER_FROM_LINK (Link);
>
> +
>
> + Link = RemoveEntryList (&TokenBuf->Link);
>
> +
>
> + FreePool (TokenBuf->Buffer);
>
> + FreePool (TokenBuf);
>
> + }
>
>
>
> while (!IsListEmpty (&gSmmCpuPrivate->TokenList)) {
>
> Link = GetFirstNode (&gSmmCpuPrivate->TokenList);
>
> @@ -499,7 +517,6 @@ FreeTokens (
>
>
> RemoveEntryList (&ProcToken->Link);
>
>
>
> - FreePool ((VOID *)ProcToken->ProcedureToken);
>
> FreePool (ProcToken);
>
> }
>
> }
>
> @@ -1115,13 +1132,37 @@ CreateToken (
> VOID
>
> )
>
> {
>
> - PROCEDURE_TOKEN *ProcToken;
>
> + PROCEDURE_TOKEN *ProcToken;
>
> SPIN_LOCK *CpuToken;
>
> UINTN SpinLockSize;
>
> + TOKEN_BUFFER *TokenBuf;
>
> + UINT32 TokenCountPerChunk;
>
>
>
> SpinLockSize = GetSpinLockProperties ();
>
> - CpuToken = AllocatePool (SpinLockSize);
>
> - ASSERT (CpuToken != NULL);
>
> + TokenCountPerChunk = FixedPcdGet32
> (PcdCpuSmmMpTokenCountPerChunk);
>
> +
>
> + if (gSmmCpuPrivate->UsedTokenNum == TokenCountPerChunk) {
>
> + DEBUG ((DEBUG_VERBOSE, "CpuSmm: No free token buffer, allocate
> new buffer!\n"));
>
> +
>
> + //
>
> + // Record current token buffer for later free action usage.
>
> + // Current used token buffer not in this list.
>
> + //
>
> + TokenBuf = AllocatePool (sizeof (TOKEN_BUFFER));
>
> + ASSERT (TokenBuf != NULL);
>
> + TokenBuf->Signature = TOKEN_BUFFER_SIGNATURE;
>
> + TokenBuf->Buffer = gSmmCpuPrivate->CurrentTokenBuf;
>
> +
>
> + InsertTailList (&gSmmCpuPrivate->OldTokenBufList, &TokenBuf->Link);
>
> +
>
> + gSmmCpuPrivate->CurrentTokenBuf = AllocatePool (SpinLockSize *
> TokenCountPerChunk);
>
> + ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL);
>
> + gSmmCpuPrivate->UsedTokenNum = 0;
>
> + }
>
> +
>
> + CpuToken = (SPIN_LOCK *)(gSmmCpuPrivate->CurrentTokenBuf +
> SpinLockSize * gSmmCpuPrivate->UsedTokenNum);
>
> + gSmmCpuPrivate->UsedTokenNum++;
>
> +
>
> InitializeSpinLock (CpuToken);
>
> AcquireSpinLock (CpuToken);
>
>
>
> @@ -1732,10 +1773,28 @@ InitializeDataForMmMp (
> VOID
>
> )
>
> {
>
> + UINTN SpinLockSize;
>
> + UINT32 TokenCountPerChunk;
>
> +
>
> + SpinLockSize = GetSpinLockProperties ();
>
> + TokenCountPerChunk = FixedPcdGet32
> (PcdCpuSmmMpTokenCountPerChunk);
>
> + ASSERT (TokenCountPerChunk != 0);
>
> + if (TokenCountPerChunk == 0) {
>
> + DEBUG ((DEBUG_ERROR, "PcdCpuSmmMpTokenCountPerChunk should
> not be Zero!\n"));
>
> + CpuDeadLoop ();
>
> + }
>
> + DEBUG ((DEBUG_INFO, "CpuSmm: SpinLock Size = 0x%x,
> PcdCpuSmmMpTokenCountPerChunk = 0x%x\n", SpinLockSize,
> TokenCountPerChunk));
>
> +
>
> + gSmmCpuPrivate->CurrentTokenBuf = AllocatePool (SpinLockSize *
> TokenCountPerChunk);
>
> + ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL);
>
> +
>
> + gSmmCpuPrivate->UsedTokenNum = 0;
>
> +
>
> gSmmCpuPrivate->ApWrapperFunc = AllocatePool (sizeof
> (PROCEDURE_WRAPPER) * gSmmCpuPrivate-
> >SmmCoreEntryContext.NumberOfCpus);
>
> ASSERT (gSmmCpuPrivate->ApWrapperFunc != NULL);
>
>
>
> InitializeListHead (&gSmmCpuPrivate->TokenList);
>
> + InitializeListHead (&gSmmCpuPrivate->OldTokenBufList);
>
> }
>
>
>
> /**
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 7e7c73f27f..5c1a01e42b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -217,6 +217,17 @@ typedef struct {
>
>
> #define PROCEDURE_TOKEN_FROM_LINK(a) CR (a, PROCEDURE_TOKEN,
> Link, PROCEDURE_TOKEN_SIGNATURE)
>
>
>
> +#define TOKEN_BUFFER_SIGNATURE SIGNATURE_32 ('T', 'K', 'B', 'S')
>
> +
>
> +typedef struct {
>
> + UINTN Signature;
>
> + LIST_ENTRY Link;
>
> +
>
> + UINT8 *Buffer;
>
> +} TOKEN_BUFFER;
>
> +
>
> +#define TOKEN_BUFFER_FROM_LINK(a) CR (a, TOKEN_BUFFER, Link,
> TOKEN_BUFFER_SIGNATURE)
>
> +
>
> //
>
> // Private structure for the SMM CPU module that is stored in DXE Runtime
> memory
>
> // Contains the SMM Configuration Protocols that is produced.
>
> @@ -243,6 +254,10 @@ typedef struct {
> PROCEDURE_WRAPPER *ApWrapperFunc;
>
> LIST_ENTRY TokenList;
>
>
>
> + LIST_ENTRY OldTokenBufList;
>
> +
>
> + UINT8 *CurrentTokenBuf;
>
> + UINT32 UsedTokenNum; // Only record tokens used in
> CurrentTokenBuf.
>
> } SMM_CPU_PRIVATE_DATA;
>
>
>
> extern SMM_CPU_PRIVATE_DATA *gSmmCpuPrivate;
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index b12b2691f8..76b1462996 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -140,6 +140,9 @@
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> ## CONSUMES
>
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask
> ## CONSUMES
>
>
>
> +[FixedPcd]
>
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmMpTokenCountPerChunk
> ## CONSUMES
>
> +
>
> [Pcd.X64]
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmRestrictedMemoryAccess ##
> CONSUMES
>
>
>
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index 12f4413ea5..797f948631 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -148,6 +148,10 @@
> # @Prompt Specify size of good stack of exception which need switching
> stack.
>
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize|2048|UINT32|0
> x30002001
>
>
>
> + ## Count of pre allocated SMM MP tokens per chunk.
>
> + # @Prompt Specify the count of pre allocated SMM MP tokens per chunk.
>
> +
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmMpTokenCountPerChunk|64|UI
> NT32|0x30002002
>
> +
>
> [PcdsFixedAtBuild, PcdsPatchableInModule]
>
> ## This value is the CPU Local APIC base address, which aligns the address
> on a 4-KByte boundary.
>
> # @Prompt Configure base address of CPU Local APIC
>
> diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
> index bfd696f48c..442ce5cb85 100644
> --- a/UefiCpuPkg/UefiCpuPkg.uni
> +++ b/UefiCpuPkg/UefiCpuPkg.uni
> @@ -272,3 +272,6 @@
> "24000000 - 6th and 7th
> generation Intel Core processors and Intel Xeon W Processor
> Family(24MHz).<BR>\n"
>
> "19200000 - Intel Atom
> processors based on Goldmont Microarchitecture with CPUID signature
> 06_5CH(19.2MHz).<BR>\n"
>
>
>
> +#string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_P
> ROMPT #language en-US "Specify the count of pre allocated SMM MP
> tokens per chunk.\n"
>
> +
>
> +#string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_H
> ELP #language en-US "This value used to specify the count of pre allocated
> SMM MP tokens per chunk.\n"
> \ No newline at end of file
> --
> 2.23.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51769): https://edk2.groups.io/g/devel/message/51769
Mute This Topic: https://groups.io/mt/66924873/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 12/05/19 08:08, Eric Dong wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388
>
> Token is new introduced by MM MP Protocol. Current logic allocate Token
> every time when need to use it. The logic caused SMI latency raised to
> very high. Update logic to allocate Token buffer at driver's entry point.
> Later use the token from the allocated token buffer. Only when all the
> buffer have been used, then need to allocate new buffer.
>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
> V5 changes:
> Refine PCD names and some code comments.
> V4 changes:
> Specify PCD type to FixedPcd in code.
> V3 changes:
> Introduce PCD to control the pre allocated toke buffer size.
> v2 changes:
> Minor update based on comments.
>
> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 67 ++++++++++++++++++--
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 +++++
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +
> UefiCpuPkg/UefiCpuPkg.dec | 4 ++
> UefiCpuPkg/UefiCpuPkg.uni | 3 +
> 5 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 0685637c2b..757f1056f7 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -492,6 +492,24 @@ FreeTokens (
> {
> LIST_ENTRY *Link;
> PROCEDURE_TOKEN *ProcToken;
> + TOKEN_BUFFER *TokenBuf;
> +
> + //
> + // Only free the token buffer recorded in the OldTOkenBufList
> + // upon exiting SMI. Current token buffer stays allocated so
> + // next SMI doesn't need to re-allocate.
> + //
> + gSmmCpuPrivate->UsedTokenNum = 0;
> +
> + Link = GetFirstNode (&gSmmCpuPrivate->OldTokenBufList);
> + while (!IsNull (&gSmmCpuPrivate->OldTokenBufList, Link)) {
> + TokenBuf = TOKEN_BUFFER_FROM_LINK (Link);
> +
> + Link = RemoveEntryList (&TokenBuf->Link);
> +
> + FreePool (TokenBuf->Buffer);
> + FreePool (TokenBuf);
> + }
>
> while (!IsListEmpty (&gSmmCpuPrivate->TokenList)) {
> Link = GetFirstNode (&gSmmCpuPrivate->TokenList);
> @@ -499,7 +517,6 @@ FreeTokens (
>
> RemoveEntryList (&ProcToken->Link);
>
> - FreePool ((VOID *)ProcToken->ProcedureToken);
> FreePool (ProcToken);
> }
> }
> @@ -1115,13 +1132,37 @@ CreateToken (
> VOID
> )
> {
> - PROCEDURE_TOKEN *ProcToken;
> + PROCEDURE_TOKEN *ProcToken;
> SPIN_LOCK *CpuToken;
> UINTN SpinLockSize;
> + TOKEN_BUFFER *TokenBuf;
> + UINT32 TokenCountPerChunk;
>
> SpinLockSize = GetSpinLockProperties ();
> - CpuToken = AllocatePool (SpinLockSize);
> - ASSERT (CpuToken != NULL);
> + TokenCountPerChunk = FixedPcdGet32 (PcdCpuSmmMpTokenCountPerChunk);
> +
> + if (gSmmCpuPrivate->UsedTokenNum == TokenCountPerChunk) {
> + DEBUG ((DEBUG_VERBOSE, "CpuSmm: No free token buffer, allocate new buffer!\n"));
> +
> + //
> + // Record current token buffer for later free action usage.
> + // Current used token buffer not in this list.
> + //
> + TokenBuf = AllocatePool (sizeof (TOKEN_BUFFER));
> + ASSERT (TokenBuf != NULL);
> + TokenBuf->Signature = TOKEN_BUFFER_SIGNATURE;
> + TokenBuf->Buffer = gSmmCpuPrivate->CurrentTokenBuf;
> +
> + InsertTailList (&gSmmCpuPrivate->OldTokenBufList, &TokenBuf->Link);
> +
> + gSmmCpuPrivate->CurrentTokenBuf = AllocatePool (SpinLockSize * TokenCountPerChunk);
> + ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL);
> + gSmmCpuPrivate->UsedTokenNum = 0;
> + }
> +
> + CpuToken = (SPIN_LOCK *)(gSmmCpuPrivate->CurrentTokenBuf + SpinLockSize * gSmmCpuPrivate->UsedTokenNum);
> + gSmmCpuPrivate->UsedTokenNum++;
> +
> InitializeSpinLock (CpuToken);
> AcquireSpinLock (CpuToken);
>
> @@ -1732,10 +1773,28 @@ InitializeDataForMmMp (
> VOID
> )
> {
> + UINTN SpinLockSize;
> + UINT32 TokenCountPerChunk;
> +
> + SpinLockSize = GetSpinLockProperties ();
> + TokenCountPerChunk = FixedPcdGet32 (PcdCpuSmmMpTokenCountPerChunk);
> + ASSERT (TokenCountPerChunk != 0);
> + if (TokenCountPerChunk == 0) {
> + DEBUG ((DEBUG_ERROR, "PcdCpuSmmMpTokenCountPerChunk should not be Zero!\n"));
> + CpuDeadLoop ();
> + }
> + DEBUG ((DEBUG_INFO, "CpuSmm: SpinLock Size = 0x%x, PcdCpuSmmMpTokenCountPerChunk = 0x%x\n", SpinLockSize, TokenCountPerChunk));
> +
> + gSmmCpuPrivate->CurrentTokenBuf = AllocatePool (SpinLockSize * TokenCountPerChunk);
> + ASSERT (gSmmCpuPrivate->CurrentTokenBuf != NULL);
> +
> + gSmmCpuPrivate->UsedTokenNum = 0;
> +
> gSmmCpuPrivate->ApWrapperFunc = AllocatePool (sizeof (PROCEDURE_WRAPPER) * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
> ASSERT (gSmmCpuPrivate->ApWrapperFunc != NULL);
>
> InitializeListHead (&gSmmCpuPrivate->TokenList);
> + InitializeListHead (&gSmmCpuPrivate->OldTokenBufList);
> }
>
> /**
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 7e7c73f27f..5c1a01e42b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -217,6 +217,17 @@ typedef struct {
>
> #define PROCEDURE_TOKEN_FROM_LINK(a) CR (a, PROCEDURE_TOKEN, Link, PROCEDURE_TOKEN_SIGNATURE)
>
> +#define TOKEN_BUFFER_SIGNATURE SIGNATURE_32 ('T', 'K', 'B', 'S')
> +
> +typedef struct {
> + UINTN Signature;
> + LIST_ENTRY Link;
> +
> + UINT8 *Buffer;
> +} TOKEN_BUFFER;
> +
> +#define TOKEN_BUFFER_FROM_LINK(a) CR (a, TOKEN_BUFFER, Link, TOKEN_BUFFER_SIGNATURE)
> +
> //
> // Private structure for the SMM CPU module that is stored in DXE Runtime memory
> // Contains the SMM Configuration Protocols that is produced.
> @@ -243,6 +254,10 @@ typedef struct {
> PROCEDURE_WRAPPER *ApWrapperFunc;
> LIST_ENTRY TokenList;
>
> + LIST_ENTRY OldTokenBufList;
> +
> + UINT8 *CurrentTokenBuf;
> + UINT32 UsedTokenNum; // Only record tokens used in CurrentTokenBuf.
> } SMM_CPU_PRIVATE_DATA;
>
> extern SMM_CPU_PRIVATE_DATA *gSmmCpuPrivate;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index b12b2691f8..76b1462996 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -140,6 +140,9 @@
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask ## CONSUMES
>
> +[FixedPcd]
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmMpTokenCountPerChunk ## CONSUMES
> +
> [Pcd.X64]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmRestrictedMemoryAccess ## CONSUMES
>
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index 12f4413ea5..797f948631 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -148,6 +148,10 @@
> # @Prompt Specify size of good stack of exception which need switching stack.
> gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize|2048|UINT32|0x30002001
>
> + ## Count of pre allocated SMM MP tokens per chunk.
> + # @Prompt Specify the count of pre allocated SMM MP tokens per chunk.
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmMpTokenCountPerChunk|64|UINT32|0x30002002
> +
> [PcdsFixedAtBuild, PcdsPatchableInModule]
> ## This value is the CPU Local APIC base address, which aligns the address on a 4-KByte boundary.
> # @Prompt Configure base address of CPU Local APIC
> diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
> index bfd696f48c..442ce5cb85 100644
> --- a/UefiCpuPkg/UefiCpuPkg.uni
> +++ b/UefiCpuPkg/UefiCpuPkg.uni
> @@ -272,3 +272,6 @@
> "24000000 - 6th and 7th generation Intel Core processors and Intel Xeon W Processor Family(24MHz).<BR>\n"
> "19200000 - Intel Atom processors based on Goldmont Microarchitecture with CPUID signature 06_5CH(19.2MHz).<BR>\n"
>
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_PROMPT #language en-US "Specify the count of pre allocated SMM MP tokens per chunk.\n"
> +
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmMpTokenCountPerChunk_HELP #language en-US "This value used to specify the count of pre allocated SMM MP tokens per chunk.\n"
> \ No newline at end of file
>
Thank you for the updates!
Acked-by: Laszlo Ersek <lersek@redhat.com>
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51795): https://edk2.groups.io/g/devel/message/51795
Mute This Topic: https://groups.io/mt/66924873/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.