Address review comments and pipeline issues

SetupDataPkg/SetupDataDxe/SchemaXmlHashDriver/SchemaXmlHash.c

@@ -1,25 +1,32 @@
 /** @file SchemaXmlHash.c
-  Set config xml hash to BIOS variable
+  Set config xml hash to NVRAM variable.
 
   Copyright (c) Microsoft Corporation.
-
+  SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
+
 #include <Base.h>
-#include <Library/DebugLib.h>
-#include <Library/BaseLib.h>
-#include <Library/UefiRuntimeServicesTableLib.h>
 #include <ConfigStdStructDefs.h>
 #include <Generated/ConfigClientGenerated.h>
 #include <Generated/ConfigDataGenerated.h>
 #include <Generated/ConfigProfilesGenerated.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/VariablePolicyHelperLib.h>
+#include <Library/VariablePolicyLib.h>
+#include <Protocol/VariablePolicy.h>
+
+#define MAX_XML_HASH_STRING_LENGTH  32
 
 /**
   Module entry point that register to set PlatformConfigData.xml hash variable
 
-  @param FileHandle                     The image handle.
-  @param PeiServices                    The PEI services table.
+  @param[in] ImageHandle      The image handle.
+  @param[in] SystemTable      The UEFI SystemTable.
 
-  @retval Status                        From internal routine or boot object, should not fail
+  @retval Status              The status of the variable set operation.
 **/
 EFI_STATUS
 EFIAPI
@@ -28,27 +35,46 @@ SchemaXmlHashDxeEntry (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
-  EFI_STATUS  Status;
-  EFI_GUID schema_xml_hash_guid = SCHEMA_XML_HASH_GUID;
+  EFI_STATUS                      Status;
+  EFI_GUID                        SchemaXmlHashGuid  = SCHEMA_XML_HASH_GUID;
+  EDKII_VARIABLE_POLICY_PROTOCOL  *VarPolicyProtocol = NULL;
 
-  DEBUG((DEBUG_ERROR, "In %a\n", __FUNCTION__));
+  Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL, (VOID **)&VarPolicyProtocol);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a - Failed to locate var policy protocol (%r)\n", __func__, Status));
+    return Status;
+  }
 
   // Set variable "SCHEMA_XML_HASH" to with data SCHEMA_XML_HASH
   Status = gRT->SetVariable (
-                  L"SCHEMA_XML_HASH",
-                  &schema_xml_hash_guid,
-                  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
-                  AsciiStrLen(SCHEMA_XML_HASH),
+                  SCHEMA_XML_HASH_VAR_NAME,
+                  &SchemaXmlHashGuid,
+                  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+                  AsciiStrnLenS (SCHEMA_XML_HASH, MAX_XML_HASH_STRING_LENGTH),
                   SCHEMA_XML_HASH
                   );
 
-  if (EFI_ERROR(Status)) {
+  if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "Failed to set SCHEMA_XML_HASH variable. Status = %r\n", Status));
-  } else {
-    DEBUG ((DEBUG_INFO, "SCHEMA_XML_HASH variable set successfully\n"));
+    return Status;
   }
 
-  DEBUG ((DEBUG_ERROR, "Out %a\n", __FUNCTION__));
+  Status = RegisterBasicVariablePolicy (
+             VarPolicyProtocol,
+             &SchemaXmlHashGuid,
+             SCHEMA_XML_HASH_VAR_NAME,
+             MAX_XML_HASH_STRING_LENGTH,
+             MAX_XML_HASH_STRING_LENGTH,
+             EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+             EFI_VARIABLE_NON_VOLATILE,
+             VARIABLE_POLICY_TYPE_LOCK_NOW
+             );
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Unable to lock SCHEMA_XML_HASH. Status=%r\n", Status));
+  } else {
+    DEBUG ((DEBUG_INFO, "Variable SCHEMA_XML_HASH is locked\n"));
+  }
 
   return Status;
 }

SetupDataPkg/SetupDataDxe/SchemaXmlHashDriver/SchemaXmlHash.inf

@@ -1,11 +1,10 @@
 ## @file SchemaXmlHash.inf
-#  Set config xml hash to BIOS variable
-#
-#  Copyright (c) Microsoft Corporation. All rights reserved.
+#  Set config xml hash to NVRAM variable.
 #
+#  Copyright (c) Microsoft Corporation.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 
-
 [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = SchemaXmlHashDxe
@@ -19,14 +18,19 @@
 
 [Packages]
   MdePkg/MdePkg.dec
-  PlatformPkg/PlatformPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   SetupDataPkg/SetupDataPkg.dec
 
+[Protocols]
+  gEdkiiVariablePolicyProtocolGuid       ## CONSUMES
+
 [LibraryClasses]
   BaseLib
-  UefiDriverEntryPoint
   DebugLib
-  PcdLib
+  UefiDriverEntryPoint
+  UefiBootServicesTableLib
+  UefiRuntimeServicesTableLib
+  VariablePolicyHelperLib
 
 [Depex]
-  gEfiVariableArchProtocolGuid
+  gEfiVariableArchProtocolGuid AND gEdkiiVariablePolicyProtocolGuid

SetupDataPkg/SetupDataPkg.dsc

@@ -87,6 +87,7 @@
 
 [Components.X64, Components.AARCH64]
   SetupDataPkg/ConfApp/ConfApp.inf
+  SetupDataPkg/SetupDataDxe/SchemaXmlHashDriver/SchemaXmlHash.inf
 
 [BuildOptions]
   *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES

SetupDataPkg/Tools/CommonUtility.py

@@ -102,33 +102,29 @@ def array_str_to_value(val_str):
 
 
 def get_xml_full_hash(xml_path):
-    try:
-        # Parse the XML file
-        tree = ET.parse(xml_path)
-        root = tree.getroot()
+    # Parse the XML file
+    tree = ET.parse(xml_path)
+    root = tree.getroot()
 
-        # List to store all tags, attributes, and values
-        hash_content = []
+    # List to store all tags, attributes, and values
+    hash_content = []
 
-        def traverse(element):
-            # Add tag name
-            hash_content.append(element.tag)
+    def traverse(element):
+        # Add tag name
+        hash_content.append(element.tag)
 
-            # Add attribute names and values
-            for key, value in sorted(element.attrib.items()):
-                hash_content.append(f"{key}={value}")
+        # Add attribute names and values
+        for key, value in sorted(element.attrib.items()):
+            hash_content.append(f"{key}={value}")
 
-            # Recursively process each child element
-            for child in element:
-                traverse(child)
+        # Recursively process each child element
+        for child in element:
+            traverse(child)
 
-        # Start traversing the root node
-        traverse(root)
+    # Start traversing the root node
+    traverse(root)
 
-        # Combine all items in hash_content list into a single string
-        combined_string = ''.join(hash_content)
+    # Combine all items in hash_content list into a single string
+    combined_string = ''.join(hash_content)
 
-        return hashlib.md5(combined_string.encode('utf-8')).hexdigest()
-    except Exception as e:
-        print(f"Error while calc hash of xml: {e}")
-        return "Unknown"
+    return hashlib.md5(combined_string.encode('utf-8')).hexdigest()

SetupDataPkg/Tools/ConfigEditor.py

@@ -940,7 +940,7 @@ def load_bin_file(self, path):
             messagebox.showerror("LOADING ERROR", str(e))
             return
 
-        self.output_current_status(f"{path} file is loaded", color="red")
+        self.output_current_status(f"{path} file is loaded")
 
     def load_cfg_file(self, path, file_id, clear_config):
         # Clear out old config if requested
@@ -979,8 +979,8 @@ def load_cfg_file(self, path, file_id, clear_config):
 
         # compare the xml version with self.bios_git_info, 7 digits should be enough
         if self.bios_schema_xml_hash != "Unknown" and config_xml_hash != self.bios_schema_xml_hash:
-            self.output_current_status("WARNING: Config xml file hash mismatches with SUT BIOS", color="red")
-            self.output_current_status(f"BIOS ConfigXml Hash = {self.bios_schema_xml_hash}", color="red")
+            self.output_current_status("WARNING: Config xml file hash mismatches with system FW", color="red")
+            self.output_current_status(f"FW ConfigXml Hash = {self.bios_schema_xml_hash}", color="red")
             self.output_current_status(f"{self.config_xml_path} Hash  = {config_xml_hash}", color="red")
 
         return 0

SetupDataPkg/Tools/KnobService.py

@@ -634,7 +634,7 @@ def generate_cached_implementation(schema, header_path, efi_type=False, no_chang
         out.write("//  Script: {}".format(sys.argv[0]) + get_line_ending(efi_type))
         out.write("//  Schema: {}".format(schema.path) + get_line_ending(efi_type))
         out.write('#define SCHEMA_XML_HASH "' + get_xml_full_hash(schema.path) + '"' + get_line_ending(efi_type))
-        out.write('#define SCHEMA_XML_HASH_VAR_NAME "SCHEMA_XML_HASH"' + get_line_ending(efi_type))
+        out.write('#define SCHEMA_XML_HASH_VAR_NAME L"SCHEMA_XML_HASH"' + get_line_ending(efi_type))
         xml_hash_guid = "{0x1321e012, 0x14c5, 0x42db, { 0x8c, 0xa9, 0xe7, 0x97, 0x1d, 0x88, 0x15, 0x18 }}"
         out.write('#define SCHEMA_XML_HASH_GUID ' + xml_hash_guid + get_line_ending(efi_type))
         out.write("" + get_line_ending(efi_type))