[edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering
Michael D Kinney
michael.d.kinney at intel.com
Tue May 16 21:41:53 UTC 2023
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 at intel.com>
> Sent: Tuesday, May 16, 2023 12:12 PM
> To: Kinney, Michael D <michael.d.kinney at intel.com>; devel at edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Gao, Liming
> <gaoliming at byosoft.com.cn>; Zimmer, Vincent <vincent.zimmer at intel.com>;
> Chiu, Chasel <chasel.chiu at 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 at intel.com>
> Sent: Tuesday, May 16, 2023 11:01 AM
> To: devel at edk2.groups.io; Oram, Isaac W <isaac.w.oram at intel.com>
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Gao, Liming
> <gaoliming at byosoft.com.cn>; Zimmer, Vincent <vincent.zimmer at intel.com>;
> Chiu, Chasel <chasel.chiu at intel.com>; Kinney, Michael D
> <michael.d.kinney at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Isaac
> > Oram
> > Sent: Tuesday, May 16, 2023 10:39 AM
> > To: devel at edk2.groups.io; Oram, Isaac W <isaac.w.oram at intel.com>
> > Cc: Wang, Jian J <jian.j.wang at intel.com>; Gao, Liming
> > <gaoliming at byosoft.com.cn>; Zimmer, Vincent
> > <vincent.zimmer at intel.com>; Chiu, Chasel <chasel.chiu at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Isaac
> > Oram
> > Sent: Tuesday, May 16, 2023 10:35 AM
> > To: devel at edk2.groups.io
> > Cc: Oram, Isaac W <isaac.w.oram at intel.com>; Wang, Jian J
> > <jian.j.wang at intel.com>; Gao, Liming <gaoliming at 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 at intel.com>
> > Cc: Liming Gao <gaoliming at byosoft.com.cn>
> > Signed-off-by: Isaac Oram <isaac.w.oram at 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
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104956): https://edk2.groups.io/g/devel/message/104956
Mute This Topic: https://groups.io/mt/98931693/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3943202/1813853/130120423/xyzzy [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list