Re: [edk2-devel] [PATCH v1 2/2] ShellPkg/Acpiview: Adds ACPI WSMT Table parser

2024-03-07 Thread PierreGondois

Hello Abdul,

I just have one comment, with that:
Reviewed-by: Pierre Gondois 

On 3/5/24 12:12, Abdul Lateef Attar wrote:

From: Abdul Lateef Attar 

Adds WSMT parse to the UefiShellAcpiViewCommandLib library.

Cc: Zhichao Gao 
Cc: Pierre Gondois  
Signed-off-by: Abdul Lateef Attar 
---
  .../UefiShellAcpiViewCommandLib/AcpiParser.h  | 17 
  .../Parsers/Wsmt/WsmtParser.c | 89 +++
  .../UefiShellAcpiViewCommandLib.c |  1 +
  .../UefiShellAcpiViewCommandLib.inf   |  1 +
  4 files changed, 108 insertions(+)
  create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index ba3364f2c2..6468fe5d8c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -985,6 +985,23 @@ ParseAcpiSsdt (
IN UINT8AcpiTableRevision
);
  
+/**

+  This function parses the ACPI WSMT table.
+
+  @param [in] Trace  If TRUE, trace the ACPI fields.
+  @param [in] PtrPointer to the start of the buffer.
+  @param [in] AcpiTableLengthLength of the ACPI table.
+  @param [in] AcpiTableRevision  Revision of the ACPI table.
+**/
+VOID
+EFIAPI
+ParseAcpiWsmt (
+  IN BOOLEAN  Trace,
+  IN UINT8*Ptr,
+  IN UINT32   AcpiTableLength,
+  IN UINT8AcpiTableRevision
+  );
+
  /**
This function parses the ACPI XSDT table
and optionally traces the ACPI table fields.
diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c
new file mode 100644
index 00..eb2668c059
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c
@@ -0,0 +1,89 @@
+/** @file
+  WSMT table parser
+
+  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+- Windows SMM Security Mitigation Table spec, version 1.0
+**/
+
+#include 
+#include "AcpiParser.h"
+
+STATIC ACPI_DESCRIPTION_HEADER_INFO  AcpiHdrInfo;
+
+/**
+  An ACPI_PARSER array describing the WSMT Protection flag .
+**/
+STATIC CONST ACPI_PARSER  WsmtProtectionFlagParser[] = {
+  { L"FIXED_COMM_BUFFERS ",1,  0, L"0x%x", NULL, NULL, NULL, 
NULL },
+  { L"COMM_BUFFER_NESTED_PTR_PROTECTION ", 1,  1, L"0x%x", NULL, NULL, NULL, 
NULL },
+  { L"SYSTEM_RESOURCE_PROTECTION ",1,  2, L"0x%x", NULL, NULL, NULL, 
NULL },
+  { L"Reserved ",  29, 3, L"0x%x", NULL, NULL, NULL, 
NULL },
+};
+
+/**
+  This function prints WSMT Protection flag.
+  If no format string is specified the Format must be NULL.
+
+  @param [in] Format  Optional format string for tracing the data.
+  @param [in] Ptr Pointer to the start of the buffer.
+**/
+VOID
+EFIAPI
+DumpWsmtProtectionFlag (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  if (Format != NULL) {
+Print (Format, *(UINT32 *)Ptr);
+return;
+  }
+
+  Print (L"0x%X\n", *(UINT32 *)Ptr);
+  ParseAcpiBitFields (
+TRUE,
+2,
+NULL,
+Ptr,
+4,
+PARSER_PARAMS (WsmtProtectionFlagParser)
+);
+}
+
+/**
+  An ACPI_PARSER array describing the ACPI WSMT Table.
+**/
+STATIC CONST ACPI_PARSER  WsmtParser[] = {
+  PARSE_ACPI_HEADER (),
+  { L"Protection Flag",4,36, NULL, DumpWsmtProtectionFlag, NULL, 
NULL, NULL }


Maybe a validate function could be added, similar to:
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c::ValidateFlags()

just to check that FIXED_COMM_BUFFERS  is set if 
COMM_BUFFER_NESTED_PTR_PROTECTION
is set, and use IncrementErrorCount() + Print() if



+};
+
+/**
+  This function parses the ACPI WSMT table.
+
+  @param [in] Trace  If TRUE, trace the ACPI fields.
+  @param [in] PtrPointer to the start of the buffer.
+  @param [in] AcpiTableLengthLength of the ACPI table.
+  @param [in] AcpiTableRevision  Revision of the ACPI table.
+**/
+VOID
+EFIAPI
+ParseAcpiWsmt (
+  IN BOOLEAN  Trace,
+  IN UINT8*Ptr,
+  IN UINT32   AcpiTableLength,
+  IN UINT8AcpiTableRevision
+  )
+{
+  ParseAcpi (
+Trace,
+0,
+"WSMT",
+Ptr,
+AcpiTableLength,
+PARSER_PARAMS (WsmtParser)
+);
+}
diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
index 9e15979ea2..0bdf068fe0 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
@@ -73,6 +73,7 @@ ACPI_TABLE_PARSER  ParserList[] = {
{ EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE, 
   ParseAcpiSpcr },

[edk2-devel] [PATCH v1 2/2] ShellPkg/Acpiview: Adds ACPI WSMT Table parser

2024-03-05 Thread Abdul Lateef Attar via groups.io
From: Abdul Lateef Attar 

Adds WSMT parse to the UefiShellAcpiViewCommandLib library.

Cc: Zhichao Gao 
Cc: Pierre Gondois  
Signed-off-by: Abdul Lateef Attar 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.h  | 17 
 .../Parsers/Wsmt/WsmtParser.c | 89 +++
 .../UefiShellAcpiViewCommandLib.c |  1 +
 .../UefiShellAcpiViewCommandLib.inf   |  1 +
 4 files changed, 108 insertions(+)
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index ba3364f2c2..6468fe5d8c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -985,6 +985,23 @@ ParseAcpiSsdt (
   IN UINT8AcpiTableRevision
   );
 
+/**
+  This function parses the ACPI WSMT table.
+
+  @param [in] Trace  If TRUE, trace the ACPI fields.
+  @param [in] PtrPointer to the start of the buffer.
+  @param [in] AcpiTableLengthLength of the ACPI table.
+  @param [in] AcpiTableRevision  Revision of the ACPI table.
+**/
+VOID
+EFIAPI
+ParseAcpiWsmt (
+  IN BOOLEAN  Trace,
+  IN UINT8*Ptr,
+  IN UINT32   AcpiTableLength,
+  IN UINT8AcpiTableRevision
+  );
+
 /**
   This function parses the ACPI XSDT table
   and optionally traces the ACPI table fields.
diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c
new file mode 100644
index 00..eb2668c059
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c
@@ -0,0 +1,89 @@
+/** @file
+  WSMT table parser
+
+  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Reference(s):
+- Windows SMM Security Mitigation Table spec, version 1.0
+**/
+
+#include 
+#include "AcpiParser.h"
+
+STATIC ACPI_DESCRIPTION_HEADER_INFO  AcpiHdrInfo;
+
+/**
+  An ACPI_PARSER array describing the WSMT Protection flag .
+**/
+STATIC CONST ACPI_PARSER  WsmtProtectionFlagParser[] = {
+  { L"FIXED_COMM_BUFFERS ",1,  0, L"0x%x", NULL, NULL, NULL, 
NULL },
+  { L"COMM_BUFFER_NESTED_PTR_PROTECTION ", 1,  1, L"0x%x", NULL, NULL, NULL, 
NULL },
+  { L"SYSTEM_RESOURCE_PROTECTION ",1,  2, L"0x%x", NULL, NULL, NULL, 
NULL },
+  { L"Reserved ",  29, 3, L"0x%x", NULL, NULL, NULL, 
NULL },
+};
+
+/**
+  This function prints WSMT Protection flag.
+  If no format string is specified the Format must be NULL.
+
+  @param [in] Format  Optional format string for tracing the data.
+  @param [in] Ptr Pointer to the start of the buffer.
+**/
+VOID
+EFIAPI
+DumpWsmtProtectionFlag (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  if (Format != NULL) {
+Print (Format, *(UINT32 *)Ptr);
+return;
+  }
+
+  Print (L"0x%X\n", *(UINT32 *)Ptr);
+  ParseAcpiBitFields (
+TRUE,
+2,
+NULL,
+Ptr,
+4,
+PARSER_PARAMS (WsmtProtectionFlagParser)
+);
+}
+
+/**
+  An ACPI_PARSER array describing the ACPI WSMT Table.
+**/
+STATIC CONST ACPI_PARSER  WsmtParser[] = {
+  PARSE_ACPI_HEADER (),
+  { L"Protection Flag",4,36, NULL, DumpWsmtProtectionFlag, NULL, 
NULL, NULL }
+};
+
+/**
+  This function parses the ACPI WSMT table.
+
+  @param [in] Trace  If TRUE, trace the ACPI fields.
+  @param [in] PtrPointer to the start of the buffer.
+  @param [in] AcpiTableLengthLength of the ACPI table.
+  @param [in] AcpiTableRevision  Revision of the ACPI table.
+**/
+VOID
+EFIAPI
+ParseAcpiWsmt (
+  IN BOOLEAN  Trace,
+  IN UINT8*Ptr,
+  IN UINT32   AcpiTableLength,
+  IN UINT8AcpiTableRevision
+  )
+{
+  ParseAcpi (
+Trace,
+0,
+"WSMT",
+Ptr,
+AcpiTableLength,
+PARSER_PARAMS (WsmtParser)
+);
+}
diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
index 9e15979ea2..0bdf068fe0 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
@@ -73,6 +73,7 @@ ACPI_TABLE_PARSER  ParserList[] = {
   { EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,  
  ParseAcpiSpcr },
   { EFI_ACPI_6_2_SYSTEM_RESOURCE_AFFINITY_TABLE_SIGNATURE, 
  ParseAcpiSrat },
   { EFI_ACPI_6_2_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE, 
  ParseAcpiSsdt },
+  { EFI_ACPI_6_5_WINDOWS_SMM_SECURITY_MITIGATION_TABLE_SIGNATURE,  
  ParseAcpiWsmt },
   {