[edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Restore CPU interrupt state

[Jeff Fan]

In DEBUG_AGENT_INIT_POSTMEM_SEC case, caller may disable/restore CPU interrupt
to protect the stack/heap migration. SecPeiDebugAgentLib cannot always enable
CPU interrupt. Otherwise system may crash during stack/heap migration.
SecPeiDebugAgentLib should restore original CPU interrupt state in
DEBUG_AGENT_INIT_POSTMEM_SEC case.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <***@intel.com>
CC: Ruiyu Ni <***@intel.com>
CC: Brian J. Johnson <***@sgi.com>
---
.../SecPeiDebugAgent/SecPeiDebugAgentLib.c | 29 ++++++++++++++--------
1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c
index 09216c2..faec574 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c
+++ b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c
@@ -376,8 +376,12 @@ InitializeDebugAgent (
UINT64 *MailboxLocationPointer;
EFI_PHYSICAL_ADDRESS Address;
UINT32 DebugTimerFrequency;
+ BOOLEAN CpuInterruptState;

- DisableInterrupts ();
+ //
+ // Disable interrupts and save current interrupt state
+ //
+ CpuInterruptState = SaveAndDisableInterrupts();

switch (InitFlag) {

@@ -611,18 +615,23 @@ InitializeDebugAgent (
break;
}

- //
- // Enable Debug Timer interrupt. In post-memory SEC, the caller enables it.
- //
- if (InitFlag != DEBUG_AGENT_INIT_POSTMEM_SEC) {
+ if (InitFlag == DEBUG_AGENT_INIT_POSTMEM_SEC) {
+ //
+ // Restore CPU Interrupt state and keep debug timer interrupt state as is
+ // in DEBUG_AGENT_INIT_POSTMEM_SEC case
+ //
+ SetInterruptState (CpuInterruptState);
+ } else {
+ //
+ // Enable Debug Timer interrupt
+ //
SaveAndSetDebugTimerInterrupt (TRUE);
+ //
+ // Enable CPU interrupts so debug timer interrupts can be delivered
+ //
+ EnableInterrupts ();
}
//
- // Enable CPU interrupts so debug timer interrupts can be delivered
- //
- EnableInterrupts ();
-
- //
// If Function is not NULL, invoke it always whatever debug agent was initialized sucesssfully or not.
//
if (Function != NULL) {

[Brian J. Johnson]

Reviewed, and tested in OVMF and a h/w environment with our internal
debugger. Seems good.

Post by Jeff Fan
In DEBUG_AGENT_INIT_POSTMEM_SEC case, caller may disable/restore CPU interrupt
to protect the stack/heap migration. SecPeiDebugAgentLib cannot always enable
CPU interrupt. Otherwise system may crash during stack/heap migration.
SecPeiDebugAgentLib should restore original CPU interrupt state in
DEBUG_AGENT_INIT_POSTMEM_SEC case.
Contributed-under: TianoCore Contribution Agreement 1.0
---
.../SecPeiDebugAgent/SecPeiDebugAgentLib.c | 29 ++++++++++++++--------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c
index 09216c2..faec574 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c
+++ b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c
@@ -376,8 +376,12 @@ InitializeDebugAgent (
UINT64 *MailboxLocationPointer;
EFI_PHYSICAL_ADDRESS Address;
UINT32 DebugTimerFrequency;
+ BOOLEAN CpuInterruptState;
- DisableInterrupts ();
+ //
+ // Disable interrupts and save current interrupt state
+ //
+ CpuInterruptState = SaveAndDisableInterrupts();
switch (InitFlag) {
@@ -611,18 +615,23 @@ InitializeDebugAgent (
break;
}
- //
- // Enable Debug Timer interrupt. In post-memory SEC, the caller enables it.
- //
- if (InitFlag != DEBUG_AGENT_INIT_POSTMEM_SEC) {
+ if (InitFlag == DEBUG_AGENT_INIT_POSTMEM_SEC) {
+ //
+ // Restore CPU Interrupt state and keep debug timer interrupt state as is
+ // in DEBUG_AGENT_INIT_POSTMEM_SEC case
+ //
+ SetInterruptState (CpuInterruptState);
+ } else {
+ //
+ // Enable Debug Timer interrupt
+ //
SaveAndSetDebugTimerInterrupt (TRUE);
+ //
+ // Enable CPU interrupts so debug timer interrupts can be delivered
+ //
+ EnableInterrupts ();
}
//
- // Enable CPU interrupts so debug timer interrupts can be delivered
- //
- EnableInterrupts ();
-
- //
// If Function is not NULL, invoke it always whatever debug agent was initialized sucesssfully or not.
//
if (Function != NULL) {