From 1cdb41eb5704bb5b1282bed2cdf72c25c2b81926 Mon Sep 17 00:00:00 2001
From: Kun Qin <kuqin@microsoft.com>
Date: Tue, 19 Sep 2023 10:38:21 -0700
Subject: [PATCH 1/6] Updated scripts to accept cache for policy values

---
 SetupDataPkg/Tools/KnobService.py | 55 +++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 17 deletions(-)

diff --git a/SetupDataPkg/Tools/KnobService.py b/SetupDataPkg/Tools/KnobService.py
index 8d183e29..07ce7f3e 100644
--- a/SetupDataPkg/Tools/KnobService.py
+++ b/SetupDataPkg/Tools/KnobService.py
@@ -184,15 +184,18 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         if knob.help != "":
             out.write("// {}".format(knob.help) + get_line_ending(efi_type))
 
-        out.write("// Get the current value of the {} knob".format(knob.name) + get_line_ending(efi_type))
-        out.write("EFI_STATUS {}{} (".format(
+        # Implement the cached getter function
+        out.write("// Get the current value of the {} knob from supplied cache".format(knob.name) + get_line_ending(efi_type))
+        out.write("EFI_STATUS {}{}FromCache (".format(
             naming_convention_filter("config_get_", False, efi_type),
             knob.name
         ) + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
-        out.write("{} *Knob".format(
+        out.write("{} *Knob,".format(
             get_type_string(knob.format.c_type, efi_type)
         ) + get_line_ending(efi_type))
+        out.write(get_spacing_string(efi_type) + "UINT8 *Cache," + get_line_ending(efi_type))
+        out.write(get_spacing_string(efi_type) + "UINT16 CacheSize" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type) + ")" + get_line_ending(efi_type))
         out.write("{" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type) + "EFI_STATUS Status;")
@@ -221,7 +224,7 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         out.write(get_spacing_string(efi_type))
         out.write("if (!CachedPolicyInitialized) {" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))
-        out.write("Status = InitConfigPolicyCache ();" + get_line_ending(efi_type))
+        out.write("Status = InitConfigPolicyCache (Cache, CacheSize);" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))
         out.write("if (EFI_ERROR (Status)) {" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=3))
@@ -235,7 +238,7 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         out.write(get_line_ending(efi_type))
 
         out.write(get_spacing_string(efi_type))
-        out.write("if (Offset + sizeof({}) > CachedPolicySize) {{".format(
+        out.write("if (Offset + sizeof({}) > CacheSize) {{".format(
             get_type_string(knob.format.c_type, efi_type)
         ) + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))
@@ -247,7 +250,7 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         out.write(get_line_ending(efi_type))
 
         out.write(get_spacing_string(efi_type))
-        out.write("CopyMem(Knob, CachedPolicy + Offset, sizeof ({}));".format(
+        out.write("CopyMem(Knob, Cache + Offset, sizeof ({}));".format(
             get_type_string(knob.format.c_type, efi_type)
         ) + get_line_ending(efi_type))
 
@@ -255,6 +258,25 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         out.write("return EFI_SUCCESS;" + get_line_ending(efi_type))
         out.write("}" + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
+
+        # Implement the normal getter function, from the global cache
+        out.write("// Get the current value of the {} knob from cache".format(knob.name) + get_line_ending(efi_type))
+        out.write("EFI_STATUS {}{} (".format(
+            naming_convention_filter("config_get_", False, efi_type),
+            knob.name
+        ) + get_line_ending(efi_type))
+        out.write(get_spacing_string(efi_type))
+        out.write("{} *Knob".format(
+            get_type_string(knob.format.c_type, efi_type)
+        ) + get_line_ending(efi_type))
+        out.write(get_spacing_string(efi_type) + ")" + get_line_ending(efi_type))
+        out.write("{" + get_line_ending(efi_type))
+        out.write(get_spacing_string(efi_type) + "return {}{}FromCache (Knob, CachedPolicy, sizeof (CachedPolicy));".format(
+            naming_convention_filter("config_get_", False, efi_type),
+            knob.name
+        ) + get_line_ending(efi_type))
+        out.write("}" + get_line_ending(efi_type))
+        out.write(get_line_ending(efi_type))
         pass
 
 
@@ -927,17 +949,15 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write(get_line_ending(efi_type))
 
         policy_size = hex(get_conf_policy_size(schema))
-        out.write("STATIC CONST UINT16  CachedPolicySize = {};".format(
-            policy_size
-        ) + get_line_ending(efi_type))
-        out.write("STATIC CHAR8 CachedPolicy[{}];".format(
+        out.write("#define CACHED_POLICY_SIZE {}".format(
             policy_size
         ) + get_line_ending(efi_type))
+        out.write(get_line_ending(efi_type))
+
+        out.write("STATIC CHAR8 CachedPolicy[CACHED_POLICY_SIZE];" + get_line_ending(efi_type))
         out.write("STATIC BOOLEAN CachedPolicyInitialized = FALSE;")
         out.write(get_line_ending(efi_type))
-        out.write(get_assert_style(efi_type, "({} <= MAX_UINT16".format(
-            policy_size
-        ), '"Config too large!"'))
+        out.write(get_assert_style(efi_type, "(CACHED_POLICY_SIZE <= MAX_UINT16", '"Config too large!"'))
         out.write(get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
 
@@ -946,21 +966,22 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write("EFI_STATUS" + get_line_ending(efi_type))
         out.write(naming_convention_filter("init_config_policy_cache (", False, efi_type))
         out.write(get_line_ending(efi_type))
-        out.write(get_spacing_string(efi_type) + get_type_string("void", efi_type))
+        out.write(get_spacing_string(efi_type) + get_type_string("UINT8   *Cache,", efi_type) + get_line_ending(efi_type))
+        out.write(get_spacing_string(efi_type) + get_type_string("UINT16  CacheSize", efi_type))
         out.write(get_line_ending(efi_type) + ")" + get_line_ending(efi_type) + "{")
         out.write(get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
         out.write("EFI_STATUS Status;" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
-        out.write("UINT16 ConfPolSize = CachedPolicySize;" + get_line_ending(efi_type))
+        out.write("UINT16 ConfPolSize = CacheSize;" + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
 
         out.write(get_spacing_string(efi_type))
         out.write("Status = GetPolicy (")
         out.write("PcdGetPtr (PcdConfigurationPolicyGuid), NULL,")
-        out.write(" CachedPolicy, &ConfPolSize);" + get_line_ending(efi_type))
+        out.write(" Cache, &ConfPolSize);" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
-        out.write("if ((EFI_ERROR (Status)) || (ConfPolSize != CachedPolicySize)) {" + get_line_ending(efi_type))
+        out.write("if ((EFI_ERROR (Status)) || (ConfPolSize != CACHED_POLICY_SIZE)) {" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2) + "ASSERT (FALSE);")
         out.write(get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))

From a617a772f19be4680a78e988ba351bab345a2485 Mon Sep 17 00:00:00 2001
From: Kun Qin <kuqin@microsoft.com>
Date: Tue, 19 Sep 2023 11:03:37 -0700
Subject: [PATCH 2/6] fixing flake8

---
 SetupDataPkg/Tools/KnobService.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/SetupDataPkg/Tools/KnobService.py b/SetupDataPkg/Tools/KnobService.py
index 07ce7f3e..e6d79c18 100644
--- a/SetupDataPkg/Tools/KnobService.py
+++ b/SetupDataPkg/Tools/KnobService.py
@@ -185,7 +185,9 @@ def write_uefi_getter_implementations(efi_type, out, schema):
             out.write("// {}".format(knob.help) + get_line_ending(efi_type))
 
         # Implement the cached getter function
-        out.write("// Get the current value of the {} knob from supplied cache".format(knob.name) + get_line_ending(efi_type))
+        out.write("// Get the current value of the {} knob from supplied cache".format(
+            knob.name
+        ) + get_line_ending(efi_type))
         out.write("EFI_STATUS {}{}FromCache (".format(
             naming_convention_filter("config_get_", False, efi_type),
             knob.name
@@ -271,7 +273,8 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         ) + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type) + ")" + get_line_ending(efi_type))
         out.write("{" + get_line_ending(efi_type))
-        out.write(get_spacing_string(efi_type) + "return {}{}FromCache (Knob, CachedPolicy, sizeof (CachedPolicy));".format(
+        out.write(get_spacing_string(efi_type))
+        out.write("return {}{}FromCache (Knob, CachedPolicy, sizeof (CachedPolicy));".format(
             naming_convention_filter("config_get_", False, efi_type),
             knob.name
         ) + get_line_ending(efi_type))
@@ -966,8 +969,8 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write("EFI_STATUS" + get_line_ending(efi_type))
         out.write(naming_convention_filter("init_config_policy_cache (", False, efi_type))
         out.write(get_line_ending(efi_type))
-        out.write(get_spacing_string(efi_type) + get_type_string("UINT8   *Cache,", efi_type) + get_line_ending(efi_type))
-        out.write(get_spacing_string(efi_type) + get_type_string("UINT16  CacheSize", efi_type))
+        out.write(get_spacing_string(efi_type) + "UINT8   *Cache," + get_line_ending(efi_type))
+        out.write(get_spacing_string(efi_type) + "UINT16  CacheSize")
         out.write(get_line_ending(efi_type) + ")" + get_line_ending(efi_type) + "{")
         out.write(get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))

From 1e2fea15a00f550b652d8809377484ecab76ff99 Mon Sep 17 00:00:00 2001
From: Kun Qin <kuqin@microsoft.com>
Date: Wed, 20 Sep 2023 09:29:04 -0700
Subject: [PATCH 3/6] Embedded a signature

---
 SetupDataPkg/Tools/KnobService.py | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/SetupDataPkg/Tools/KnobService.py b/SetupDataPkg/Tools/KnobService.py
index e6d79c18..97737ed2 100644
--- a/SetupDataPkg/Tools/KnobService.py
+++ b/SetupDataPkg/Tools/KnobService.py
@@ -207,7 +207,7 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         # minus the data size and CRC32, which comes after the data
         offset += get_variable_list_size(knob)
         offset -= 4 + knob.format.size_in_bytes()
-        out.write("CONST UINTN Offset = {};".format(
+        out.write("CONST UINTN Offset = CACHED_POLICY_HEADER_SIZE + {};".format(
             offset
         ) + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
@@ -216,7 +216,7 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         offset += 4 + knob.format.size_in_bytes()
 
         out.write(get_spacing_string(efi_type))
-        out.write("if (Knob == NULL) {" + get_line_ending(efi_type))
+        out.write("if ((Knob == NULL) || (Cache == NULL)) {" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))
         out.write("return EFI_INVALID_PARAMETER;" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
@@ -224,7 +224,7 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         out.write(get_line_ending(efi_type))
 
         out.write(get_spacing_string(efi_type))
-        out.write("if (!CachedPolicyInitialized) {" + get_line_ending(efi_type))
+        out.write("if (((CACHED_POLICY_HEADER*)Cache)->Signature != CACHED_POLICY_SINGATURE) {" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))
         out.write("Status = InitConfigPolicyCache (Cache, CacheSize);" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))
@@ -952,15 +952,29 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write(get_line_ending(efi_type))
 
         policy_size = hex(get_conf_policy_size(schema))
+        out.write("#define CACHED_POLICY_SINGATURE    SIGNATURE_32 ('C', 'P', 'O', 'L')" + get_line_ending(efi_type))
+        out.write("#define CACHED_POLICY_HEADER_SIZE  sizeof (CACHED_POLICY_HEADER)" + get_line_ending(efi_type))
+        out.write(get_line_ending(efi_type))
+
         out.write("#define CACHED_POLICY_SIZE {}".format(
             policy_size
         ) + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
 
-        out.write("STATIC CHAR8 CachedPolicy[CACHED_POLICY_SIZE];" + get_line_ending(efi_type))
+        out.write("// Cached policy header, used to validate the cache internally" + get_line_ending(efi_type))
+        out.write("#pragma pack (1)" + get_line_ending(efi_type))
+        out.write(get_line_ending(efi_type))
+        out.write("typedef struct {" + get_line_ending(efi_type))
+        out.write(get_spacing_string(efi_type) + "UINT32 Signature;" + get_line_ending(efi_type))
+        out.write("} CACHED_POLICY_HEADER;" + get_line_ending(efi_type))
+        out.write(get_line_ending(efi_type))
+        out.write("#pragma pack ()" + get_line_ending(efi_type))
+        out.write(get_line_ending(efi_type))
+
+        out.write("STATIC CHAR8 CachedPolicy[CACHED_POLICY_SIZE + CACHED_POLICY_HEADER_SIZE];" + get_line_ending(efi_type))
         out.write("STATIC BOOLEAN CachedPolicyInitialized = FALSE;")
         out.write(get_line_ending(efi_type))
-        out.write(get_assert_style(efi_type, "(CACHED_POLICY_SIZE <= MAX_UINT16", '"Config too large!"'))
+        out.write(get_assert_style(efi_type, "(CACHED_POLICY_SIZE + CACHED_POLICY_HEADER_SIZE <= MAX_UINT16", '"Config too large!"'))
         out.write(get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
 
@@ -982,7 +996,7 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write(get_spacing_string(efi_type))
         out.write("Status = GetPolicy (")
         out.write("PcdGetPtr (PcdConfigurationPolicyGuid), NULL,")
-        out.write(" Cache, &ConfPolSize);" + get_line_ending(efi_type))
+        out.write(" Cache + CACHED_POLICY_HEADER_SIZE, &ConfPolSize);" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
         out.write("if ((EFI_ERROR (Status)) || (ConfPolSize != CACHED_POLICY_SIZE)) {" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2) + "ASSERT (FALSE);")
@@ -993,7 +1007,7 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write("}" + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
-        out.write("CachedPolicyInitialized = TRUE;" + get_line_ending(efi_type))
+        out.write("((CACHED_POLICY_HEADER*)Cache)->Signature = CACHED_POLICY_SINGATURE;" + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
         out.write("return Status;" + get_line_ending(efi_type))

From 2b6f58e3728ff2b89efe650aed0e6f761e2ba2a3 Mon Sep 17 00:00:00 2001
From: Kun Qin <kuqin@microsoft.com>
Date: Wed, 20 Sep 2023 15:52:04 -0700
Subject: [PATCH 4/6] Fixing the PR comment

---
 SetupDataPkg/Tools/KnobService.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/SetupDataPkg/Tools/KnobService.py b/SetupDataPkg/Tools/KnobService.py
index 97737ed2..8e3a831b 100644
--- a/SetupDataPkg/Tools/KnobService.py
+++ b/SetupDataPkg/Tools/KnobService.py
@@ -224,7 +224,8 @@ def write_uefi_getter_implementations(efi_type, out, schema):
         out.write(get_line_ending(efi_type))
 
         out.write(get_spacing_string(efi_type))
-        out.write("if (((CACHED_POLICY_HEADER*)Cache)->Signature != CACHED_POLICY_SINGATURE) {" + get_line_ending(efi_type))
+        out.write("if (((CACHED_POLICY_HEADER *)Cache)->Signature != CACHED_POLICY_SIGNATURE) {")
+        out.write(get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))
         out.write("Status = InitConfigPolicyCache (Cache, CacheSize);" + get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type, num=2))
@@ -952,7 +953,7 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write(get_line_ending(efi_type))
 
         policy_size = hex(get_conf_policy_size(schema))
-        out.write("#define CACHED_POLICY_SINGATURE    SIGNATURE_32 ('C', 'P', 'O', 'L')" + get_line_ending(efi_type))
+        out.write("#define CACHED_POLICY_SIGNATURE    SIGNATURE_32 ('C', 'P', 'O', 'L')" + get_line_ending(efi_type))
         out.write("#define CACHED_POLICY_HEADER_SIZE  sizeof (CACHED_POLICY_HEADER)" + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
 
@@ -971,7 +972,8 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write("#pragma pack ()" + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
 
-        out.write("STATIC CHAR8 CachedPolicy[CACHED_POLICY_SIZE + CACHED_POLICY_HEADER_SIZE];" + get_line_ending(efi_type))
+        out.write("STATIC CHAR8 CachedPolicy[CACHED_POLICY_SIZE + CACHED_POLICY_HEADER_SIZE];")
+        out.write(get_line_ending(efi_type))
         out.write("STATIC BOOLEAN CachedPolicyInitialized = FALSE;")
         out.write(get_line_ending(efi_type))
         out.write(get_assert_style(efi_type, "(CACHED_POLICY_SIZE + CACHED_POLICY_HEADER_SIZE <= MAX_UINT16", '"Config too large!"'))
@@ -1007,7 +1009,7 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write("}" + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
-        out.write("((CACHED_POLICY_HEADER*)Cache)->Signature = CACHED_POLICY_SINGATURE;" + get_line_ending(efi_type))
+        out.write("((CACHED_POLICY_HEADER*)Cache)->Signature = CACHED_POLICY_SIGNATURE;" + get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
         out.write(get_spacing_string(efi_type))
         out.write("return Status;" + get_line_ending(efi_type))

From cb579dc1ec662a94370af9187c457c9f1a1b3b14 Mon Sep 17 00:00:00 2001
From: Kun Qin <kuqin@microsoft.com>
Date: Wed, 20 Sep 2023 15:59:36 -0700
Subject: [PATCH 5/6] Fixing flake8

---
 SetupDataPkg/Tools/KnobService.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/SetupDataPkg/Tools/KnobService.py b/SetupDataPkg/Tools/KnobService.py
index 8e3a831b..add9fb89 100644
--- a/SetupDataPkg/Tools/KnobService.py
+++ b/SetupDataPkg/Tools/KnobService.py
@@ -976,7 +976,10 @@ def generate_getter_implementation(schema, header_path, efi_type):
         out.write(get_line_ending(efi_type))
         out.write("STATIC BOOLEAN CachedPolicyInitialized = FALSE;")
         out.write(get_line_ending(efi_type))
-        out.write(get_assert_style(efi_type, "(CACHED_POLICY_SIZE + CACHED_POLICY_HEADER_SIZE <= MAX_UINT16", '"Config too large!"'))
+        out.write(get_assert_style(
+            efi_type,
+            "(CACHED_POLICY_SIZE + CACHED_POLICY_HEADER_SIZE <= MAX_UINT16",
+            '"Config too large!"'))
         out.write(get_line_ending(efi_type))
         out.write(get_line_ending(efi_type))
 

From 5801958c0bbb364c57930a43d66f638f11f79470 Mon Sep 17 00:00:00 2001
From: Kun Qin <kuqin@microsoft.com>
Date: Wed, 20 Sep 2023 16:39:29 -0700
Subject: [PATCH 6/6] added doc

---
 .../PlatformIntegration/PlatformIntegrationSteps.md   | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/SetupDataPkg/Docs/PlatformIntegration/PlatformIntegrationSteps.md b/SetupDataPkg/Docs/PlatformIntegration/PlatformIntegrationSteps.md
index 4ff11c9e..2a0fd44c 100644
--- a/SetupDataPkg/Docs/PlatformIntegration/PlatformIntegrationSteps.md
+++ b/SetupDataPkg/Docs/PlatformIntegration/PlatformIntegrationSteps.md
@@ -140,6 +140,17 @@ are available for the code using the getters. In our mu_tiano_platforms example,
 Mapper is compiled with several C files that only include the client header, as the service header is only needed once
 per module to query the config knobs out of the config policy.
 
+The getter functions are implemented in the service header file. The traditional way to fetch a knob value will be to call
+the getter function with the knob name by supplying the pointer to hold such knob
+(i.e. `ConfigGetKnob1 (&Knob)`).
+
+Alternatively, getter functions can also be called with a knob name and a size-specified buffer. In this case, the getter
+function will inspect the cache buffer and try to populate the content if it is uninitialized. The caller should keep note
+of the intialized buffer for subsquent calls in the same module for optimal performance. The caller should also be aware
+that the buffer size should be large enough (`CACHED_POLICY_SIZE + CACHED_POLICY_HEADER_SIZE`) to hold the knob value.
+The usage of this alternative could work around the limitation of the traditional way that the relies on global variables
+when the modules are XIP (execute in place).
+
 The Silicon Policy Consumers do not need to include any of the above headers and will instead fetch their configuration
 directly from silicon policy.