* [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering
[not found] <cover.1684258196.git.isaac.w.oram@intel.com>
@ 2023-05-16 17:34 ` Isaac Oram
[not found] ` <175FB03665B4DC78.20583@groups.io>
1 sibling, 0 replies; 5+ messages in thread
From: Isaac Oram @ 2023-05-16 17:34 UTC (permalink / raw)
To: devel; +Cc: Isaac Oram, Jian J Wang, Liming Gao
Extend the MemoryStatusCodeWorker functionality to log the debug strings.
In the event of an assert, dump the strings to serial port.
The rationale is that SMI latency is very important. This provides a
means to log messages quickly (to memory) and dump them in the event of
an assert.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
---
.../Include/Guid/MemoryStatusCodeRecord.h | 7 +
.../Smm/MemoryStatusCodeWorker.c | 199 ++++++++++++++++++
2 files changed, 206 insertions(+)
diff --git a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
index a924c592c9..d37b6304c6 100644
--- a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
+++ b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
@@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#ifndef __MEMORY_STATUS_CODE_RECORD_H__
#define __MEMORY_STATUS_CODE_RECORD_H__
+#include <Guid/StatusCodeDataTypeDebug.h>
+
///
/// Global ID used to identify GUIDed HOBs that start with a structure of type
/// MEMORY_STATUSCODE_PACKET_HEADER, followed by an array of structures of type
@@ -90,6 +92,11 @@ typedef struct {
/// the system. Valid instance numbers start with the number 1.
///
UINT32 Instance;
+
+ //
+ // Extra data
+ //
+ UINT64 Data[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) + 1];
} MEMORY_STATUSCODE_RECORD;
extern EFI_GUID gMemoryStatusCodeRecordGuid;
diff --git a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c
index 80c94e4682..c482c4bd9a 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c
+++ b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c
@@ -11,6 +11,141 @@
RUNTIME_MEMORY_STATUSCODE_HEADER *mMmMemoryStatusCodeTable;
+VOID
+DumpBufferToSerial (
+ VOID
+ );
+
+EFI_STATUS
+EFIAPI
+DumpRecordToSerial (
+ IN EFI_STATUS_CODE_TYPE CodeType,
+ IN EFI_STATUS_CODE_VALUE Value,
+ IN UINT32 Instance,
+ IN EFI_GUID *CallerId,
+ IN EFI_STATUS_CODE_DATA *Data OPTIONAL
+ )
+{
+ CHAR8 *Filename;
+ CHAR8 *Description;
+ CHAR8 *Format;
+ CHAR8 Buffer[MAX_EXTENDED_DATA_SIZE + sizeof (UINT64)];
+ UINT32 ErrorLevel;
+ UINT32 LineNumber;
+ UINTN CharCount;
+ BASE_LIST Marker;
+
+ Buffer[0] = '\0';
+
+ if ((Data != NULL) &&
+ ReportStatusCodeExtractAssertInfo (CodeType, Value, Data, &Filename, &Description, &LineNumber))
+ {
+ //
+ // Print ASSERT() information into output buffer.
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "\n\rDXE_ASSERT!: %a (%d): %a\n\r",
+ Filename,
+ LineNumber,
+ Description
+ );
+ } else if ((Data != NULL) &&
+ ReportStatusCodeExtractDebugInfo (Data, &ErrorLevel, &Marker, &Format))
+ {
+ //
+ // Print DEBUG() information into output buffer.
+ //
+ CharCount = AsciiBSPrint (
+ Buffer,
+ sizeof (Buffer),
+ Format,
+ Marker
+ );
+ } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) == EFI_ERROR_CODE) {
+ //
+ // Print ERROR information into output buffer.
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "ERROR: C%08x:V%08x I%x",
+ CodeType,
+ Value,
+ Instance
+ );
+ ASSERT (CharCount > 0);
+
+ if (CallerId != NULL) {
+ CharCount += AsciiSPrint (
+ &Buffer[CharCount],
+ (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
+ " %g",
+ CallerId
+ );
+ }
+
+ if (Data != NULL) {
+ CharCount += AsciiSPrint (
+ &Buffer[CharCount],
+ (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
+ " %x",
+ Data
+ );
+ }
+
+ CharCount += AsciiSPrint (
+ &Buffer[CharCount],
+ (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
+ "\n\r"
+ );
+ } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) == EFI_PROGRESS_CODE) {
+ //
+ // Print PROGRESS information into output buffer.
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "PROGRESS CODE: V%08x I%x\n\r",
+ Value,
+ Instance
+ );
+ } else if ((Data != NULL) &&
+ CompareGuid (&Data->Type, &gEfiStatusCodeDataTypeStringGuid) &&
+ (((EFI_STATUS_CODE_STRING_DATA *)Data)->StringType == EfiStringAscii))
+ {
+ //
+ // EFI_STATUS_CODE_STRING_DATA
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "%a",
+ ((EFI_STATUS_CODE_STRING_DATA *)Data)->String.Ascii
+ );
+ } else {
+ //
+ // Code type is not defined.
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "Undefined: C%08x:V%08x I%x\n\r",
+ CodeType,
+ Value,
+ Instance
+ );
+ }
+
+ //
+ // Call SerialPort Lib function to do print.
+ //
+ SerialPortWrite ((UINT8 *)Buffer, CharCount);
+
+ return EFI_SUCCESS;
+}
+
/**
Initialize MM memory status code table as initialization for memory status code worker
@@ -69,6 +204,9 @@ MemoryStatusCodeReportWorker (
)
{
MEMORY_STATUSCODE_RECORD *Record;
+ CHAR8 *Filename;
+ CHAR8 *Description;
+ UINT32 LineNumber;
//
// Locate current record buffer.
@@ -82,6 +220,7 @@ MemoryStatusCodeReportWorker (
Record->CodeType = CodeType;
Record->Value = Value;
Record->Instance = Instance;
+ CopyMem (&Record->Data, Data, (Data->HeaderSize + Data->Size));
//
// If record index equals to max record number, then wrap around record index to zero.
@@ -99,5 +238,65 @@ MemoryStatusCodeReportWorker (
mMmMemoryStatusCodeTable->RecordIndex = 0;
}
+ if ((Data != NULL) && ReportStatusCodeExtractAssertInfo (CodeType, Value, Data, &Filename, &Description, &LineNumber)) {
+ // Found an assert, dump the buffer
+ DumpBufferToSerial ();
+ }
+
return EFI_SUCCESS;
}
+
+VOID
+DumpBufferToSerial (
+ VOID
+ )
+{
+ MEMORY_STATUSCODE_RECORD *CurrentRecord;
+ UINT32 CurrentIndex;
+ UINT32 RecordsToPrint;
+ CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+ UINTN CharCount;
+
+ CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "Begin dump of SMM debug messages\n");
+ SerialPortWrite ((UINT8 *)Buffer, CharCount);
+
+ //
+ // Determine number of records to print
+ //
+
+ RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
+
+ //
+ // Determine first record in case we rolled over.
+ //
+ if (mMmMemoryStatusCodeTable->NumberOfRecords >= mMmMemoryStatusCodeTable->MaxRecordsNumber) {
+ RecordsToPrint = mMmMemoryStatusCodeTable->MaxRecordsNumber;
+ CurrentIndex = mMmMemoryStatusCodeTable->RecordIndex;
+ } else {
+ RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
+ CurrentIndex = 0;
+ }
+
+
+ //
+ // Locate first record.
+ //
+ CurrentRecord = (MEMORY_STATUSCODE_RECORD *)(mMmMemoryStatusCodeTable + 1);
+ CurrentRecord = &CurrentRecord[CurrentIndex];
+
+ //
+ // Print records
+ //
+ while (CurrentIndex < RecordsToPrint) {
+ DumpRecordToSerial (CurrentRecord->CodeType, CurrentRecord->Value, CurrentRecord->Instance, NULL, (EFI_STATUS_CODE_DATA *)CurrentRecord->Data);
+ CurrentIndex++;
+ if (CurrentIndex == mMmMemoryStatusCodeTable->MaxRecordsNumber) {
+ CurrentIndex = 0;
+ }
+ CurrentRecord = (MEMORY_STATUSCODE_RECORD *)(mMmMemoryStatusCodeTable + 1);
+ CurrentRecord = &CurrentRecord[CurrentIndex];
+ }
+
+ CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "End dump of SMM debug messages\n");
+ SerialPortWrite ((UINT8 *)Buffer, CharCount);
+}
--
2.40.0.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering
[not found] ` <175FB03665B4DC78.20583@groups.io>
@ 2023-05-16 17:39 ` Isaac Oram
2023-05-16 18:00 ` Michael D Kinney
0 siblings, 1 reply; 5+ messages in thread
From: Isaac Oram @ 2023-05-16 17:39 UTC (permalink / raw)
To: devel@edk2.groups.io, Oram, Isaac W
Cc: Wang, Jian J, Gao, Liming, Zimmer, Vincent, Chiu, Chasel
Apologies, this is edk2 scope, not edk2-platforms.
This is an RFC to see if this kind of feature is widely interesting and if someone wants to explore this in depth.
This is not a complete or mature implementation. I didn't fully decipher the extended data format.
Regards,
Isaac
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac Oram
Sent: Tuesday, May 16, 2023 10:35 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering
Extend the MemoryStatusCodeWorker functionality to log the debug strings.
In the event of an assert, dump the strings to serial port.
The rationale is that SMI latency is very important. This provides a means to log messages quickly (to memory) and dump them in the event of an assert.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
---
.../Include/Guid/MemoryStatusCodeRecord.h | 7 +
.../Smm/MemoryStatusCodeWorker.c | 199 ++++++++++++++++++
2 files changed, 206 insertions(+)
diff --git a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
index a924c592c9..d37b6304c6 100644
--- a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
+++ b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
@@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #ifndef __MEMORY_STATUS_CODE_RECORD_H__ #define __MEMORY_STATUS_CODE_RECORD_H__
+#include <Guid/StatusCodeDataTypeDebug.h>
+
///
/// Global ID used to identify GUIDed HOBs that start with a structure of type /// MEMORY_STATUSCODE_PACKET_HEADER, followed by an array of structures of type @@ -90,6 +92,11 @@ typedef struct {
/// the system. Valid instance numbers start with the number 1.
///
UINT32 Instance;
+
+ //
+ // Extra data
+ //
+ UINT64 Data[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) + 1];
} MEMORY_STATUSCODE_RECORD;
extern EFI_GUID gMemoryStatusCodeRecordGuid; diff --git a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c
index 80c94e4682..c482c4bd9a 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c
+++ b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorke
+++ r.c
@@ -11,6 +11,141 @@
RUNTIME_MEMORY_STATUSCODE_HEADER *mMmMemoryStatusCodeTable;
+VOID
+DumpBufferToSerial (
+ VOID
+ );
+
+EFI_STATUS
+EFIAPI
+DumpRecordToSerial (
+ IN EFI_STATUS_CODE_TYPE CodeType,
+ IN EFI_STATUS_CODE_VALUE Value,
+ IN UINT32 Instance,
+ IN EFI_GUID *CallerId,
+ IN EFI_STATUS_CODE_DATA *Data OPTIONAL
+ )
+{
+ CHAR8 *Filename;
+ CHAR8 *Description;
+ CHAR8 *Format;
+ CHAR8 Buffer[MAX_EXTENDED_DATA_SIZE + sizeof (UINT64)];
+ UINT32 ErrorLevel;
+ UINT32 LineNumber;
+ UINTN CharCount;
+ BASE_LIST Marker;
+
+ Buffer[0] = '\0';
+
+ if ((Data != NULL) &&
+ ReportStatusCodeExtractAssertInfo (CodeType, Value, Data,
+ &Filename, &Description, &LineNumber)) {
+ //
+ // Print ASSERT() information into output buffer.
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "\n\rDXE_ASSERT!: %a (%d): %a\n\r",
+ Filename,
+ LineNumber,
+ Description
+ );
+ } else if ((Data != NULL) &&
+ ReportStatusCodeExtractDebugInfo (Data, &ErrorLevel,
+ &Marker, &Format)) {
+ //
+ // Print DEBUG() information into output buffer.
+ //
+ CharCount = AsciiBSPrint (
+ Buffer,
+ sizeof (Buffer),
+ Format,
+ Marker
+ );
+ } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) == EFI_ERROR_CODE) {
+ //
+ // Print ERROR information into output buffer.
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "ERROR: C%08x:V%08x I%x",
+ CodeType,
+ Value,
+ Instance
+ );
+ ASSERT (CharCount > 0);
+
+ if (CallerId != NULL) {
+ CharCount += AsciiSPrint (
+ &Buffer[CharCount],
+ (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
+ " %g",
+ CallerId
+ );
+ }
+
+ if (Data != NULL) {
+ CharCount += AsciiSPrint (
+ &Buffer[CharCount],
+ (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
+ " %x",
+ Data
+ );
+ }
+
+ CharCount += AsciiSPrint (
+ &Buffer[CharCount],
+ (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
+ "\n\r"
+ );
+ } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) == EFI_PROGRESS_CODE) {
+ //
+ // Print PROGRESS information into output buffer.
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "PROGRESS CODE: V%08x I%x\n\r",
+ Value,
+ Instance
+ );
+ } else if ((Data != NULL) &&
+ CompareGuid (&Data->Type, &gEfiStatusCodeDataTypeStringGuid) &&
+ (((EFI_STATUS_CODE_STRING_DATA *)Data)->StringType ==
+ EfiStringAscii)) {
+ //
+ // EFI_STATUS_CODE_STRING_DATA
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "%a",
+ ((EFI_STATUS_CODE_STRING_DATA *)Data)->String.Ascii
+ );
+ } else {
+ //
+ // Code type is not defined.
+ //
+ CharCount = AsciiSPrint (
+ Buffer,
+ sizeof (Buffer),
+ "Undefined: C%08x:V%08x I%x\n\r",
+ CodeType,
+ Value,
+ Instance
+ );
+ }
+
+ //
+ // Call SerialPort Lib function to do print.
+ //
+ SerialPortWrite ((UINT8 *)Buffer, CharCount);
+
+ return EFI_SUCCESS;
+}
+
/**
Initialize MM memory status code table as initialization for memory status code worker
@@ -69,6 +204,9 @@ MemoryStatusCodeReportWorker (
)
{
MEMORY_STATUSCODE_RECORD *Record;
+ CHAR8 *Filename;
+ CHAR8 *Description;
+ UINT32 LineNumber;
//
// Locate current record buffer.
@@ -82,6 +220,7 @@ MemoryStatusCodeReportWorker (
Record->CodeType = CodeType;
Record->Value = Value;
Record->Instance = Instance;
+ CopyMem (&Record->Data, Data, (Data->HeaderSize + Data->Size));
//
// If record index equals to max record number, then wrap around record index to zero.
@@ -99,5 +238,65 @@ MemoryStatusCodeReportWorker (
mMmMemoryStatusCodeTable->RecordIndex = 0;
}
+ if ((Data != NULL) && ReportStatusCodeExtractAssertInfo (CodeType, Value, Data, &Filename, &Description, &LineNumber)) {
+ // Found an assert, dump the buffer
+ DumpBufferToSerial ();
+ }
+
return EFI_SUCCESS;
}
+
+VOID
+DumpBufferToSerial (
+ VOID
+ )
+{
+ MEMORY_STATUSCODE_RECORD *CurrentRecord;
+ UINT32 CurrentIndex;
+ UINT32 RecordsToPrint;
+ CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+ UINTN CharCount;
+
+ CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "Begin dump of SMM
+ debug messages\n"); SerialPortWrite ((UINT8 *)Buffer, CharCount);
+
+ //
+ // Determine number of records to print //
+
+ RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
+
+ //
+ // Determine first record in case we rolled over.
+ //
+ if (mMmMemoryStatusCodeTable->NumberOfRecords >= mMmMemoryStatusCodeTable->MaxRecordsNumber) {
+ RecordsToPrint = mMmMemoryStatusCodeTable->MaxRecordsNumber;
+ CurrentIndex = mMmMemoryStatusCodeTable->RecordIndex;
+ } else {
+ RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
+ CurrentIndex = 0;
+ }
+
+
+ //
+ // Locate first record.
+ //
+ CurrentRecord = (MEMORY_STATUSCODE_RECORD *)(mMmMemoryStatusCodeTable
+ + 1); CurrentRecord = &CurrentRecord[CurrentIndex];
+
+ //
+ // Print records
+ //
+ while (CurrentIndex < RecordsToPrint) {
+ DumpRecordToSerial (CurrentRecord->CodeType, CurrentRecord->Value, CurrentRecord->Instance, NULL, (EFI_STATUS_CODE_DATA *)CurrentRecord->Data);
+ CurrentIndex++;
+ if (CurrentIndex == mMmMemoryStatusCodeTable->MaxRecordsNumber) {
+ CurrentIndex = 0;
+ }
+ CurrentRecord = (MEMORY_STATUSCODE_RECORD *)(mMmMemoryStatusCodeTable + 1);
+ CurrentRecord = &CurrentRecord[CurrentIndex]; }
+
+ CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "End dump of SMM
+debug messages\n");
+ SerialPortWrite ((UINT8 *)Buffer, CharCount); }
--
2.40.0.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering
2023-05-16 17:39 ` Isaac Oram
@ 2023-05-16 18:00 ` Michael D Kinney
2023-05-16 19:11 ` Isaac Oram
0 siblings, 1 reply; 5+ messages in thread
From: Michael D Kinney @ 2023-05-16 18:00 UTC (permalink / raw)
To: devel@edk2.groups.io, Oram, Isaac W
Cc: Wang, Jian J, Gao, Liming, Zimmer, Vincent, Chiu, Chasel,
Kinney, Michael D
Hi Isaac,
Multiple status code handlers can be registered. One for memory and one for serial.
Why would we mix serial into memory one?
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac
> Oram
> Sent: Tuesday, May 16, 2023 10:39 AM
> To: devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Zimmer, Vincent <vincent.zimmer@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>
> Subject: Re: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1]
> MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory
> buffering
>
> Apologies, this is edk2 scope, not edk2-platforms.
>
> This is an RFC to see if this kind of feature is widely interesting and if
> someone wants to explore this in depth.
> This is not a complete or mature implementation. I didn't fully decipher the
> extended data format.
>
> Regards,
> Isaac
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac
> Oram
> Sent: Tuesday, May 16, 2023 10:35 AM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1]
> MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory
> buffering
>
> Extend the MemoryStatusCodeWorker functionality to log the debug strings.
> In the event of an assert, dump the strings to serial port.
> The rationale is that SMI latency is very important. This provides a means to
> log messages quickly (to memory) and dump them in the event of an assert.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
> ---
> .../Include/Guid/MemoryStatusCodeRecord.h | 7 +
> .../Smm/MemoryStatusCodeWorker.c | 199 ++++++++++++++++++
> 2 files changed, 206 insertions(+)
>
> diff --git a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> index a924c592c9..d37b6304c6 100644
> --- a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> +++ b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> @@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #ifndef
> __MEMORY_STATUS_CODE_RECORD_H__ #define
> __MEMORY_STATUS_CODE_RECORD_H__
>
> +#include <Guid/StatusCodeDataTypeDebug.h>
> +
> ///
> /// Global ID used to identify GUIDed HOBs that start with a structure of type
> /// MEMORY_STATUSCODE_PACKET_HEADER, followed by an array of
> structures of type @@ -90,6 +92,11 @@ typedef struct {
> /// the system. Valid instance numbers start with the number 1.
> ///
> UINT32 Instance;
> +
> + //
> + // Extra data
> + //
> + UINT64 Data[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) + 1];
> } MEMORY_STATUSCODE_RECORD;
>
> extern EFI_GUID gMemoryStatusCodeRecordGuid; diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWo
> rker.c
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeW
> orker.c
> index 80c94e4682..c482c4bd9a 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWo
> rker.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeW
> orke
> +++ r.c
> @@ -11,6 +11,141 @@
>
> RUNTIME_MEMORY_STATUSCODE_HEADER *mMmMemoryStatusCodeTable;
>
> +VOID
> +DumpBufferToSerial (
> + VOID
> + );
> +
> +EFI_STATUS
> +EFIAPI
> +DumpRecordToSerial (
> + IN EFI_STATUS_CODE_TYPE CodeType,
> + IN EFI_STATUS_CODE_VALUE Value,
> + IN UINT32 Instance,
> + IN EFI_GUID *CallerId,
> + IN EFI_STATUS_CODE_DATA *Data OPTIONAL
> + )
> +{
> + CHAR8 *Filename;
> + CHAR8 *Description;
> + CHAR8 *Format;
> + CHAR8 Buffer[MAX_EXTENDED_DATA_SIZE + sizeof (UINT64)];
> + UINT32 ErrorLevel;
> + UINT32 LineNumber;
> + UINTN CharCount;
> + BASE_LIST Marker;
> +
> + Buffer[0] = '\0';
> +
> + if ((Data != NULL) &&
> + ReportStatusCodeExtractAssertInfo (CodeType, Value, Data,
> + &Filename, &Description, &LineNumber)) {
> + //
> + // Print ASSERT() information into output buffer.
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "\n\rDXE_ASSERT!: %a (%d): %a\n\r",
> + Filename,
> + LineNumber,
> + Description
> + );
> + } else if ((Data != NULL) &&
> + ReportStatusCodeExtractDebugInfo (Data, &ErrorLevel,
> + &Marker, &Format)) {
> + //
> + // Print DEBUG() information into output buffer.
> + //
> + CharCount = AsciiBSPrint (
> + Buffer,
> + sizeof (Buffer),
> + Format,
> + Marker
> + );
> + } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) == EFI_ERROR_CODE)
> {
> + //
> + // Print ERROR information into output buffer.
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "ERROR: C%08x:V%08x I%x",
> + CodeType,
> + Value,
> + Instance
> + );
> + ASSERT (CharCount > 0);
> +
> + if (CallerId != NULL) {
> + CharCount += AsciiSPrint (
> + &Buffer[CharCount],
> + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> + " %g",
> + CallerId
> + );
> + }
> +
> + if (Data != NULL) {
> + CharCount += AsciiSPrint (
> + &Buffer[CharCount],
> + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> + " %x",
> + Data
> + );
> + }
> +
> + CharCount += AsciiSPrint (
> + &Buffer[CharCount],
> + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> + "\n\r"
> + );
> + } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) ==
> EFI_PROGRESS_CODE) {
> + //
> + // Print PROGRESS information into output buffer.
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "PROGRESS CODE: V%08x I%x\n\r",
> + Value,
> + Instance
> + );
> + } else if ((Data != NULL) &&
> + CompareGuid (&Data->Type, &gEfiStatusCodeDataTypeStringGuid)
> &&
> + (((EFI_STATUS_CODE_STRING_DATA *)Data)->StringType ==
> + EfiStringAscii)) {
> + //
> + // EFI_STATUS_CODE_STRING_DATA
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "%a",
> + ((EFI_STATUS_CODE_STRING_DATA *)Data)->String.Ascii
> + );
> + } else {
> + //
> + // Code type is not defined.
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "Undefined: C%08x:V%08x I%x\n\r",
> + CodeType,
> + Value,
> + Instance
> + );
> + }
> +
> + //
> + // Call SerialPort Lib function to do print.
> + //
> + SerialPortWrite ((UINT8 *)Buffer, CharCount);
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> Initialize MM memory status code table as initialization for memory status
> code worker
>
> @@ -69,6 +204,9 @@ MemoryStatusCodeReportWorker (
> )
> {
> MEMORY_STATUSCODE_RECORD *Record;
> + CHAR8 *Filename;
> + CHAR8 *Description;
> + UINT32 LineNumber;
>
> //
> // Locate current record buffer.
> @@ -82,6 +220,7 @@ MemoryStatusCodeReportWorker (
> Record->CodeType = CodeType;
> Record->Value = Value;
> Record->Instance = Instance;
> + CopyMem (&Record->Data, Data, (Data->HeaderSize + Data->Size));
>
> //
> // If record index equals to max record number, then wrap around record
> index to zero.
> @@ -99,5 +238,65 @@ MemoryStatusCodeReportWorker (
> mMmMemoryStatusCodeTable->RecordIndex = 0;
> }
>
> + if ((Data != NULL) && ReportStatusCodeExtractAssertInfo (CodeType, Value,
> Data, &Filename, &Description, &LineNumber)) {
> + // Found an assert, dump the buffer
> + DumpBufferToSerial ();
> + }
> +
> return EFI_SUCCESS;
> }
> +
> +VOID
> +DumpBufferToSerial (
> + VOID
> + )
> +{
> + MEMORY_STATUSCODE_RECORD *CurrentRecord;
> + UINT32 CurrentIndex;
> + UINT32 RecordsToPrint;
> + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> + UINTN CharCount;
> +
> + CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "Begin dump of SMM
> + debug messages\n"); SerialPortWrite ((UINT8 *)Buffer, CharCount);
> +
> + //
> + // Determine number of records to print //
> +
> + RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
> +
> + //
> + // Determine first record in case we rolled over.
> + //
> + if (mMmMemoryStatusCodeTable->NumberOfRecords >=
> mMmMemoryStatusCodeTable->MaxRecordsNumber) {
> + RecordsToPrint = mMmMemoryStatusCodeTable->MaxRecordsNumber;
> + CurrentIndex = mMmMemoryStatusCodeTable->RecordIndex;
> + } else {
> + RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
> + CurrentIndex = 0;
> + }
> +
> +
> + //
> + // Locate first record.
> + //
> + CurrentRecord = (MEMORY_STATUSCODE_RECORD
> *)(mMmMemoryStatusCodeTable
> + + 1); CurrentRecord = &CurrentRecord[CurrentIndex];
> +
> + //
> + // Print records
> + //
> + while (CurrentIndex < RecordsToPrint) {
> + DumpRecordToSerial (CurrentRecord->CodeType, CurrentRecord->Value,
> CurrentRecord->Instance, NULL, (EFI_STATUS_CODE_DATA *)CurrentRecord-
> >Data);
> + CurrentIndex++;
> + if (CurrentIndex == mMmMemoryStatusCodeTable->MaxRecordsNumber) {
> + CurrentIndex = 0;
> + }
> + CurrentRecord = (MEMORY_STATUSCODE_RECORD
> *)(mMmMemoryStatusCodeTable + 1);
> + CurrentRecord = &CurrentRecord[CurrentIndex]; }
> +
> + CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "End dump of SMM
> +debug messages\n");
> + SerialPortWrite ((UINT8 *)Buffer, CharCount); }
> --
> 2.40.0.windows.1
>
>
>
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering
2023-05-16 18:00 ` Michael D Kinney
@ 2023-05-16 19:11 ` Isaac Oram
2023-05-16 21:41 ` Michael D Kinney
0 siblings, 1 reply; 5+ messages in thread
From: Isaac Oram @ 2023-05-16 19:11 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Cc: Wang, Jian J, Gao, Liming, Zimmer, Vincent, Chiu, Chasel
It is sometimes desirable to have debug logging enabled even in production systems.
But printing to serial ports on an SMI handler is slow, so SMM debug message printing is typically disabled.
This provides the ability to get the data via serial/BMC/production available channels.
It should be high performance to add to the memory buffer, though I didn't measure.
It is a one time event to dump on assert.
It could be extended to dump on error codes. Exceptions?
It could be provided as a protocol or such to allow anyone to trigger.
I just added the Data logging and connected the serial dump to ASSERT, but again, this is more an RFC on the general capability of logging strings (SMRAM is more plentiful than it once was, so only recording status code and value seems unnecessarily frugal) and dumping them to serial port.
I simply wanted to make it available and RFC seemed as good a way to do it as any. If there is a preferred path for such topics, let me know.
Regards,
Isaac
-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Tuesday, May 16, 2023 11:01 AM
To: devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Zimmer, Vincent <vincent.zimmer@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering
Hi Isaac,
Multiple status code handlers can be registered. One for memory and one for serial.
Why would we mix serial into memory one?
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac
> Oram
> Sent: Tuesday, May 16, 2023 10:39 AM
> To: devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
> Subject: Re: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1]
> MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory
> buffering
>
> Apologies, this is edk2 scope, not edk2-platforms.
>
> This is an RFC to see if this kind of feature is widely interesting
> and if someone wants to explore this in depth.
> This is not a complete or mature implementation. I didn't fully
> decipher the extended data format.
>
> Regards,
> Isaac
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac
> Oram
> Sent: Tuesday, May 16, 2023 10:35 AM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1]
> MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory
> buffering
>
> Extend the MemoryStatusCodeWorker functionality to log the debug strings.
> In the event of an assert, dump the strings to serial port.
> The rationale is that SMI latency is very important. This provides a
> means to log messages quickly (to memory) and dump them in the event of an assert.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
> ---
> .../Include/Guid/MemoryStatusCodeRecord.h | 7 +
> .../Smm/MemoryStatusCodeWorker.c | 199 ++++++++++++++++++
> 2 files changed, 206 insertions(+)
>
> diff --git a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> index a924c592c9..d37b6304c6 100644
> --- a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> +++ b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> @@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #ifndef __MEMORY_STATUS_CODE_RECORD_H__ #define
> __MEMORY_STATUS_CODE_RECORD_H__
>
> +#include <Guid/StatusCodeDataTypeDebug.h>
> +
> ///
> /// Global ID used to identify GUIDed HOBs that start with a
> structure of type /// MEMORY_STATUSCODE_PACKET_HEADER, followed by an
> array of structures of type @@ -90,6 +92,11 @@ typedef struct {
> /// the system. Valid instance numbers start with the number 1.
> ///
> UINT32 Instance;
> +
> + //
> + // Extra data
> + //
> + UINT64 Data[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) + 1];
> } MEMORY_STATUSCODE_RECORD;
>
> extern EFI_GUID gMemoryStatusCodeRecordGuid; diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWo
> rker.c
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeW
> orker.c
> index 80c94e4682..c482c4bd9a 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWo
> rker.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeW
> orke
> +++ r.c
> @@ -11,6 +11,141 @@
>
> RUNTIME_MEMORY_STATUSCODE_HEADER *mMmMemoryStatusCodeTable;
>
> +VOID
> +DumpBufferToSerial (
> + VOID
> + );
> +
> +EFI_STATUS
> +EFIAPI
> +DumpRecordToSerial (
> + IN EFI_STATUS_CODE_TYPE CodeType,
> + IN EFI_STATUS_CODE_VALUE Value,
> + IN UINT32 Instance,
> + IN EFI_GUID *CallerId,
> + IN EFI_STATUS_CODE_DATA *Data OPTIONAL
> + )
> +{
> + CHAR8 *Filename;
> + CHAR8 *Description;
> + CHAR8 *Format;
> + CHAR8 Buffer[MAX_EXTENDED_DATA_SIZE + sizeof (UINT64)];
> + UINT32 ErrorLevel;
> + UINT32 LineNumber;
> + UINTN CharCount;
> + BASE_LIST Marker;
> +
> + Buffer[0] = '\0';
> +
> + if ((Data != NULL) &&
> + ReportStatusCodeExtractAssertInfo (CodeType, Value, Data,
> + &Filename, &Description, &LineNumber)) {
> + //
> + // Print ASSERT() information into output buffer.
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "\n\rDXE_ASSERT!: %a (%d): %a\n\r",
> + Filename,
> + LineNumber,
> + Description
> + );
> + } else if ((Data != NULL) &&
> + ReportStatusCodeExtractDebugInfo (Data, &ErrorLevel,
> + &Marker, &Format)) {
> + //
> + // Print DEBUG() information into output buffer.
> + //
> + CharCount = AsciiBSPrint (
> + Buffer,
> + sizeof (Buffer),
> + Format,
> + Marker
> + );
> + } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) ==
> + EFI_ERROR_CODE)
> {
> + //
> + // Print ERROR information into output buffer.
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "ERROR: C%08x:V%08x I%x",
> + CodeType,
> + Value,
> + Instance
> + );
> + ASSERT (CharCount > 0);
> +
> + if (CallerId != NULL) {
> + CharCount += AsciiSPrint (
> + &Buffer[CharCount],
> + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> + " %g",
> + CallerId
> + );
> + }
> +
> + if (Data != NULL) {
> + CharCount += AsciiSPrint (
> + &Buffer[CharCount],
> + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> + " %x",
> + Data
> + );
> + }
> +
> + CharCount += AsciiSPrint (
> + &Buffer[CharCount],
> + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> + "\n\r"
> + );
> + } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) ==
> EFI_PROGRESS_CODE) {
> + //
> + // Print PROGRESS information into output buffer.
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "PROGRESS CODE: V%08x I%x\n\r",
> + Value,
> + Instance
> + );
> + } else if ((Data != NULL) &&
> + CompareGuid (&Data->Type,
> + &gEfiStatusCodeDataTypeStringGuid)
> &&
> + (((EFI_STATUS_CODE_STRING_DATA *)Data)->StringType ==
> + EfiStringAscii)) {
> + //
> + // EFI_STATUS_CODE_STRING_DATA
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "%a",
> + ((EFI_STATUS_CODE_STRING_DATA *)Data)->String.Ascii
> + );
> + } else {
> + //
> + // Code type is not defined.
> + //
> + CharCount = AsciiSPrint (
> + Buffer,
> + sizeof (Buffer),
> + "Undefined: C%08x:V%08x I%x\n\r",
> + CodeType,
> + Value,
> + Instance
> + );
> + }
> +
> + //
> + // Call SerialPort Lib function to do print.
> + //
> + SerialPortWrite ((UINT8 *)Buffer, CharCount);
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> Initialize MM memory status code table as initialization for memory
> status code worker
>
> @@ -69,6 +204,9 @@ MemoryStatusCodeReportWorker (
> )
> {
> MEMORY_STATUSCODE_RECORD *Record;
> + CHAR8 *Filename;
> + CHAR8 *Description;
> + UINT32 LineNumber;
>
> //
> // Locate current record buffer.
> @@ -82,6 +220,7 @@ MemoryStatusCodeReportWorker (
> Record->CodeType = CodeType;
> Record->Value = Value;
> Record->Instance = Instance;
> + CopyMem (&Record->Data, Data, (Data->HeaderSize + Data->Size));
>
> //
> // If record index equals to max record number, then wrap around
> record index to zero.
> @@ -99,5 +238,65 @@ MemoryStatusCodeReportWorker (
> mMmMemoryStatusCodeTable->RecordIndex = 0;
> }
>
> + if ((Data != NULL) && ReportStatusCodeExtractAssertInfo (CodeType,
> + Value,
> Data, &Filename, &Description, &LineNumber)) {
> + // Found an assert, dump the buffer
> + DumpBufferToSerial ();
> + }
> +
> return EFI_SUCCESS;
> }
> +
> +VOID
> +DumpBufferToSerial (
> + VOID
> + )
> +{
> + MEMORY_STATUSCODE_RECORD *CurrentRecord;
> + UINT32 CurrentIndex;
> + UINT32 RecordsToPrint;
> + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> + UINTN CharCount;
> +
> + CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "Begin dump of
> + SMM debug messages\n"); SerialPortWrite ((UINT8 *)Buffer,
> + CharCount);
> +
> + //
> + // Determine number of records to print //
> +
> + RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
> +
> + //
> + // Determine first record in case we rolled over.
> + //
> + if (mMmMemoryStatusCodeTable->NumberOfRecords >=
> mMmMemoryStatusCodeTable->MaxRecordsNumber) {
> + RecordsToPrint = mMmMemoryStatusCodeTable->MaxRecordsNumber;
> + CurrentIndex = mMmMemoryStatusCodeTable->RecordIndex;
> + } else {
> + RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
> + CurrentIndex = 0;
> + }
> +
> +
> + //
> + // Locate first record.
> + //
> + CurrentRecord = (MEMORY_STATUSCODE_RECORD
> *)(mMmMemoryStatusCodeTable
> + + 1); CurrentRecord = &CurrentRecord[CurrentIndex];
> +
> + //
> + // Print records
> + //
> + while (CurrentIndex < RecordsToPrint) {
> + DumpRecordToSerial (CurrentRecord->CodeType,
> + CurrentRecord->Value,
> CurrentRecord->Instance, NULL, (EFI_STATUS_CODE_DATA *)CurrentRecord-
> >Data);
> + CurrentIndex++;
> + if (CurrentIndex == mMmMemoryStatusCodeTable->MaxRecordsNumber) {
> + CurrentIndex = 0;
> + }
> + CurrentRecord = (MEMORY_STATUSCODE_RECORD
> *)(mMmMemoryStatusCodeTable + 1);
> + CurrentRecord = &CurrentRecord[CurrentIndex]; }
> +
> + CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "End dump of SMM
> +debug messages\n");
> + SerialPortWrite ((UINT8 *)Buffer, CharCount); }
> --
> 2.40.0.windows.1
>
>
>
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering
2023-05-16 19:11 ` Isaac Oram
@ 2023-05-16 21:41 ` Michael D Kinney
0 siblings, 0 replies; 5+ messages in thread
From: Michael D Kinney @ 2023-05-16 21:41 UTC (permalink / raw)
To: Oram, Isaac W, devel@edk2.groups.io
Cc: Wang, Jian J, Gao, Liming, Zimmer, Vincent, Chiu, Chasel,
Kinney, Michael D
Hi Isaac,
We do have a standard handler for unexpected exception that dumps all CPU state to serial.
UefiCpuPkg\Library\CpuExceptionHandlerLib
This seems similar/related use case.
Mike
> -----Original Message-----
> From: Oram, Isaac W <isaac.w.oram@intel.com>
> Sent: Tuesday, May 16, 2023 12:12 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Zimmer, Vincent <vincent.zimmer@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>
> Subject: RE: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1]
> MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory
> buffering
>
> It is sometimes desirable to have debug logging enabled even in production
> systems.
> But printing to serial ports on an SMI handler is slow, so SMM debug message
> printing is typically disabled.
> This provides the ability to get the data via serial/BMC/production available
> channels.
> It should be high performance to add to the memory buffer, though I didn't
> measure.
> It is a one time event to dump on assert.
> It could be extended to dump on error codes. Exceptions?
> It could be provided as a protocol or such to allow anyone to trigger.
>
>
> I just added the Data logging and connected the serial dump to ASSERT, but
> again, this is more an RFC on the general capability of logging strings (SMRAM
> is more plentiful than it once was, so only recording status code and value
> seems unnecessarily frugal) and dumping them to serial port.
>
> I simply wanted to make it available and RFC seemed as good a way to do it
> as any. If there is a preferred path for such topics, let me know.
>
> Regards,
> Isaac
>
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, May 16, 2023 11:01 AM
> To: devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Zimmer, Vincent <vincent.zimmer@intel.com>;
> Chiu, Chasel <chasel.chiu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1]
> MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory
> buffering
>
> Hi Isaac,
>
> Multiple status code handlers can be registered. One for memory and one for
> serial.
>
> Why would we mix serial into memory one?
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac
> > Oram
> > Sent: Tuesday, May 16, 2023 10:39 AM
> > To: devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Zimmer, Vincent
> > <vincent.zimmer@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
> > Subject: Re: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1]
> > MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory
> > buffering
> >
> > Apologies, this is edk2 scope, not edk2-platforms.
> >
> > This is an RFC to see if this kind of feature is widely interesting
> > and if someone wants to explore this in depth.
> > This is not a complete or mature implementation. I didn't fully
> > decipher the extended data format.
> >
> > Regards,
> > Isaac
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac
> > Oram
> > Sent: Tuesday, May 16, 2023 10:35 AM
> > To: devel@edk2.groups.io
> > Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> > Subject: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1]
> > MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory
> > buffering
> >
> > Extend the MemoryStatusCodeWorker functionality to log the debug
> strings.
> > In the event of an assert, dump the strings to serial port.
> > The rationale is that SMI latency is very important. This provides a
> > means to log messages quickly (to memory) and dump them in the event of
> an assert.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
> > ---
> > .../Include/Guid/MemoryStatusCodeRecord.h | 7 +
> > .../Smm/MemoryStatusCodeWorker.c | 199 ++++++++++++++++++
> > 2 files changed, 206 insertions(+)
> >
> > diff --git a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> > b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> > index a924c592c9..d37b6304c6 100644
> > --- a/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> > +++ b/MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
> > @@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #ifndef __MEMORY_STATUS_CODE_RECORD_H__ #define
> > __MEMORY_STATUS_CODE_RECORD_H__
> >
> > +#include <Guid/StatusCodeDataTypeDebug.h>
> > +
> > ///
> > /// Global ID used to identify GUIDed HOBs that start with a
> > structure of type /// MEMORY_STATUSCODE_PACKET_HEADER, followed by
> an
> > array of structures of type @@ -90,6 +92,11 @@ typedef struct {
> > /// the system. Valid instance numbers start with the number 1.
> > ///
> > UINT32 Instance;
> > +
> > + //
> > + // Extra data
> > + //
> > + UINT64 Data[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) +
> 1];
> > } MEMORY_STATUSCODE_RECORD;
> >
> > extern EFI_GUID gMemoryStatusCodeRecordGuid; diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWo
> > rker.c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeW
> > orker.c
> > index 80c94e4682..c482c4bd9a 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWo
> > rker.c
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeW
> > orke
> > +++ r.c
> > @@ -11,6 +11,141 @@
> >
> > RUNTIME_MEMORY_STATUSCODE_HEADER
> *mMmMemoryStatusCodeTable;
> >
> > +VOID
> > +DumpBufferToSerial (
> > + VOID
> > + );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +DumpRecordToSerial (
> > + IN EFI_STATUS_CODE_TYPE CodeType,
> > + IN EFI_STATUS_CODE_VALUE Value,
> > + IN UINT32 Instance,
> > + IN EFI_GUID *CallerId,
> > + IN EFI_STATUS_CODE_DATA *Data OPTIONAL
> > + )
> > +{
> > + CHAR8 *Filename;
> > + CHAR8 *Description;
> > + CHAR8 *Format;
> > + CHAR8 Buffer[MAX_EXTENDED_DATA_SIZE + sizeof (UINT64)];
> > + UINT32 ErrorLevel;
> > + UINT32 LineNumber;
> > + UINTN CharCount;
> > + BASE_LIST Marker;
> > +
> > + Buffer[0] = '\0';
> > +
> > + if ((Data != NULL) &&
> > + ReportStatusCodeExtractAssertInfo (CodeType, Value, Data,
> > + &Filename, &Description, &LineNumber)) {
> > + //
> > + // Print ASSERT() information into output buffer.
> > + //
> > + CharCount = AsciiSPrint (
> > + Buffer,
> > + sizeof (Buffer),
> > + "\n\rDXE_ASSERT!: %a (%d): %a\n\r",
> > + Filename,
> > + LineNumber,
> > + Description
> > + );
> > + } else if ((Data != NULL) &&
> > + ReportStatusCodeExtractDebugInfo (Data, &ErrorLevel,
> > + &Marker, &Format)) {
> > + //
> > + // Print DEBUG() information into output buffer.
> > + //
> > + CharCount = AsciiBSPrint (
> > + Buffer,
> > + sizeof (Buffer),
> > + Format,
> > + Marker
> > + );
> > + } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) ==
> > + EFI_ERROR_CODE)
> > {
> > + //
> > + // Print ERROR information into output buffer.
> > + //
> > + CharCount = AsciiSPrint (
> > + Buffer,
> > + sizeof (Buffer),
> > + "ERROR: C%08x:V%08x I%x",
> > + CodeType,
> > + Value,
> > + Instance
> > + );
> > + ASSERT (CharCount > 0);
> > +
> > + if (CallerId != NULL) {
> > + CharCount += AsciiSPrint (
> > + &Buffer[CharCount],
> > + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> > + " %g",
> > + CallerId
> > + );
> > + }
> > +
> > + if (Data != NULL) {
> > + CharCount += AsciiSPrint (
> > + &Buffer[CharCount],
> > + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> > + " %x",
> > + Data
> > + );
> > + }
> > +
> > + CharCount += AsciiSPrint (
> > + &Buffer[CharCount],
> > + (sizeof (Buffer) - (sizeof (Buffer[0]) * CharCount)),
> > + "\n\r"
> > + );
> > + } else if ((CodeType & EFI_STATUS_CODE_TYPE_MASK) ==
> > EFI_PROGRESS_CODE) {
> > + //
> > + // Print PROGRESS information into output buffer.
> > + //
> > + CharCount = AsciiSPrint (
> > + Buffer,
> > + sizeof (Buffer),
> > + "PROGRESS CODE: V%08x I%x\n\r",
> > + Value,
> > + Instance
> > + );
> > + } else if ((Data != NULL) &&
> > + CompareGuid (&Data->Type,
> > + &gEfiStatusCodeDataTypeStringGuid)
> > &&
> > + (((EFI_STATUS_CODE_STRING_DATA *)Data)->StringType ==
> > + EfiStringAscii)) {
> > + //
> > + // EFI_STATUS_CODE_STRING_DATA
> > + //
> > + CharCount = AsciiSPrint (
> > + Buffer,
> > + sizeof (Buffer),
> > + "%a",
> > + ((EFI_STATUS_CODE_STRING_DATA *)Data)->String.Ascii
> > + );
> > + } else {
> > + //
> > + // Code type is not defined.
> > + //
> > + CharCount = AsciiSPrint (
> > + Buffer,
> > + sizeof (Buffer),
> > + "Undefined: C%08x:V%08x I%x\n\r",
> > + CodeType,
> > + Value,
> > + Instance
> > + );
> > + }
> > +
> > + //
> > + // Call SerialPort Lib function to do print.
> > + //
> > + SerialPortWrite ((UINT8 *)Buffer, CharCount);
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > /**
> > Initialize MM memory status code table as initialization for memory
> > status code worker
> >
> > @@ -69,6 +204,9 @@ MemoryStatusCodeReportWorker (
> > )
> > {
> > MEMORY_STATUSCODE_RECORD *Record;
> > + CHAR8 *Filename;
> > + CHAR8 *Description;
> > + UINT32 LineNumber;
> >
> > //
> > // Locate current record buffer.
> > @@ -82,6 +220,7 @@ MemoryStatusCodeReportWorker (
> > Record->CodeType = CodeType;
> > Record->Value = Value;
> > Record->Instance = Instance;
> > + CopyMem (&Record->Data, Data, (Data->HeaderSize + Data->Size));
> >
> > //
> > // If record index equals to max record number, then wrap around
> > record index to zero.
> > @@ -99,5 +238,65 @@ MemoryStatusCodeReportWorker (
> > mMmMemoryStatusCodeTable->RecordIndex = 0;
> > }
> >
> > + if ((Data != NULL) && ReportStatusCodeExtractAssertInfo (CodeType,
> > + Value,
> > Data, &Filename, &Description, &LineNumber)) {
> > + // Found an assert, dump the buffer
> > + DumpBufferToSerial ();
> > + }
> > +
> > return EFI_SUCCESS;
> > }
> > +
> > +VOID
> > +DumpBufferToSerial (
> > + VOID
> > + )
> > +{
> > + MEMORY_STATUSCODE_RECORD *CurrentRecord;
> > + UINT32 CurrentIndex;
> > + UINT32 RecordsToPrint;
> > + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> > + UINTN CharCount;
> > +
> > + CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "Begin dump of
> > + SMM debug messages\n"); SerialPortWrite ((UINT8 *)Buffer,
> > + CharCount);
> > +
> > + //
> > + // Determine number of records to print //
> > +
> > + RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
> > +
> > + //
> > + // Determine first record in case we rolled over.
> > + //
> > + if (mMmMemoryStatusCodeTable->NumberOfRecords >=
> > mMmMemoryStatusCodeTable->MaxRecordsNumber) {
> > + RecordsToPrint = mMmMemoryStatusCodeTable->MaxRecordsNumber;
> > + CurrentIndex = mMmMemoryStatusCodeTable->RecordIndex;
> > + } else {
> > + RecordsToPrint = mMmMemoryStatusCodeTable->NumberOfRecords;
> > + CurrentIndex = 0;
> > + }
> > +
> > +
> > + //
> > + // Locate first record.
> > + //
> > + CurrentRecord = (MEMORY_STATUSCODE_RECORD
> > *)(mMmMemoryStatusCodeTable
> > + + 1); CurrentRecord = &CurrentRecord[CurrentIndex];
> > +
> > + //
> > + // Print records
> > + //
> > + while (CurrentIndex < RecordsToPrint) {
> > + DumpRecordToSerial (CurrentRecord->CodeType,
> > + CurrentRecord->Value,
> > CurrentRecord->Instance, NULL, (EFI_STATUS_CODE_DATA *)CurrentRecord-
> > >Data);
> > + CurrentIndex++;
> > + if (CurrentIndex == mMmMemoryStatusCodeTable-
> >MaxRecordsNumber) {
> > + CurrentIndex = 0;
> > + }
> > + CurrentRecord = (MEMORY_STATUSCODE_RECORD
> > *)(mMmMemoryStatusCodeTable + 1);
> > + CurrentRecord = &CurrentRecord[CurrentIndex]; }
> > +
> > + CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "End dump of SMM
> > +debug messages\n");
> > + SerialPortWrite ((UINT8 *)Buffer, CharCount); }
> > --
> > 2.40.0.windows.1
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-16 21:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1684258196.git.isaac.w.oram@intel.com>
2023-05-16 17:34 ` [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering Isaac Oram
[not found] ` <175FB03665B4DC78.20583@groups.io>
2023-05-16 17:39 ` Isaac Oram
2023-05-16 18:00 ` Michael D Kinney
2023-05-16 19:11 ` Isaac Oram
2023-05-16 21:41 ` Michael D Kinney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox