Hi Pierre,
I think the provided patch also serve the same purpose of
validating the COMM_BUFFER_NESTED_PTR_PROTECTION against FIXED_COMM_BUFFERS.
Do you want me to remove the "ValidateReserved" implementation?
Here is the sample test results.
1)
Protection Flag : 0x5
FIXED_COMM_BUFFERS : 0x1
COMM_BUFFER_NESTED_PTR_PROTECTION : 0x0
SYSTEM_RESOURCE_PROTECTION : 0x1
Reserved : 0x0
Table Statistics:
0 Error(s)
0 Warning(s)
Shell>
2)
Protection Flag : 0x6
FIXED_COMM_BUFFERS : 0x0
COMM_BUFFER_NESTED_PTR_PROTECTION : 0x1
SYSTEM_RESOURCE_PROTECTION : 0x1
Reserved : 0x0
ERROR: COMM_BUFFER_NESTED_PTR_PROTECTION is set but FIXED_COMM_BUFFERS
is not set.
Table Statistics:
1 Error(s)
0 Warning(s)
Shell>
Thanks
AbduL
On 08-03-2024 14:50, PierreGondois via groups.io wrote:
Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.
Hello Abdul,
On 3/8/24 08:22, Abdul Lateef Attar wrote:
From: Abdul Lateef Attar <abdullateef.at...@amd.com>
Adds WSMT parse to the UefiShellAcpiViewCommandLib library.
Cc: Zhichao Gao <zhichao....@intel.com>
Cc: Pierre Gondois <pierre.gond...@arm.com>
Signed-off-by: Abdul Lateef Attar <abdullateef.at...@amd.com>
Reviewed-by: Pierre Gondois <pierre.gond...@arm.com>
---
.../UefiShellAcpiViewCommandLib/AcpiParser.h | 17 ++
.../Parsers/Wsmt/WsmtParser.c | 147 ++++++++++++++++++
.../UefiShellAcpiViewCommandLib.c | 1 +
.../UefiShellAcpiViewCommandLib.inf | 1 +
4 files changed, 166 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 UINT8 AcpiTableRevision
);
+/**
+ This function parses the ACPI WSMT table.
+
+ @param [in] Trace If TRUE, trace the ACPI fields.
+ @param [in] Ptr Pointer to the start of the buffer.
+ @param [in] AcpiTableLength Length 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 UINT8 AcpiTableRevision
+ );
+
/**
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 0000000000..3c7252b0bf
--- /dev/null
+++
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Wsmt/WsmtParser.c
@@ -0,0 +1,147 @@
+/** @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 <Library/UefiLib.h>
+#include <IndustryStandard/WindowsSmmSecurityMitigationTable.h>
+#include "AcpiParser.h"
+
+STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
+
+/**
+ This function validates the WSMT Protection flag.
+
+ @param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Context Pointer to context specific information e.g. this
+ could be a pointer to the ACPI table header.
+
+**/
+STATIC
+VOID
+EFIAPI
+ValidateWsmtProtectionFlag (
+ IN UINT8 *Ptr,
+ IN VOID *Context
+ )
+{
+ UINT32 ProtectionFlag;
+
+ ProtectionFlag = *(UINT32 *)Ptr;
+
+ if ((ProtectionFlag &
EFI_WSMT_PROTECTION_FLAGS_COMM_BUFFER_NESTED_PTR_PROTECTION) \
+ == EFI_WSMT_PROTECTION_FLAGS_COMM_BUFFER_NESTED_PTR_PROTECTION)
+ {
+ if ((ProtectionFlag &
EFI_WSMT_PROTECTION_FLAGS_FIXED_COMM_BUFFERS) \
+ != EFI_WSMT_PROTECTION_FLAGS_FIXED_COMM_BUFFERS)
+ {
+ IncrementErrorCount ();
+ Print (L"ERROR: COMM_BUFFER_NESTED_PTR_PROTECTION is set but
FIXED_COMM_BUFFERS is not set.\n");
+ }
+ }
+}
+
+/**
+ This function validates the reserved bits in the WSMT Protection
flag.
+
+ @param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Context Pointer to context specific information e.g. this
+ could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateReserved (
+ IN UINT8 *Ptr,
+ IN VOID *Context
+ )
+{
+ UINT32 ProtectionFlag;
+
+ ProtectionFlag = *(UINT32 *)Ptr;
+
+ if ((ProtectionFlag & 0xFFFFFFF8) != 0) {
+ IncrementErrorCount ();
+ Print (L"ERROR: Reserved bits are not zero.\n");
+ }
+}
+
+/**
+ 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, ValidateReserved, NULL },
I think we misunderstood each other here.
We should check that if COMM_BUFFER_NESTED_PTR_PROTECTION,
then FIXED_COMM_BUFFERS is also set.
So I think we need to:
- store the value of FIXED_COMM_BUFFERS (cf. ACPI_PARSER::ItemPtr in
other parsers)
- add a validate to COMM_BUFFER_NESTED_PTR_PROTECTION to check the above
I don't think it is necessary to check the reserved bits,
Regards,
Pierre
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116505): https://edk2.groups.io/g/devel/message/116505
Mute This Topic: https://groups.io/mt/104804849/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
