Re: [edk2-devel][edk2-platforms][RFC PATCH V1 1/1] MdeModulePkg/StatusCodeHandlerSmm: Add debug strings to memory buffering

["Michael D Kinney" <michael.d.kinney@intel.com>]

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
> 
> 
> 
> 
> 
> 
> 
> 
> 
>