Moving the global variable from header file into library instances (#473

)

## Description

The current implementation of instantiating global variables will make
each c file that includes this header to carry a copy of such variable,
which is space redundant.

This change removed the variable into the single module that uses it and
removed the structure it defines. The function descriptions are also
updated.

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [x] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

This is a non-functional change.

## Integration Instructions

N/A

---------

Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>

MfciPkg/Include/Library/MfciDeviceIdSupportLib.h

@@ -8,48 +8,45 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
-#ifndef __MFCI_DEVICE_ID_SUPPORT_LIB_H__
-#define __MFCI_DEVICE_ID_SUPPORT_LIB_H__
+#ifndef MFCI_DEVICE_ID_SUPPORT_LIB_H_
+#define MFCI_DEVICE_ID_SUPPORT_LIB_H_
 
 #include <MfciVariables.h>
 
-// define a generic function prototype shared by all library functions
-
 /**
- * Get a device-unique targeting value
- *
- * @param[out] String   Device targeting value, a UTF-16 little endian string.
- *                      Includes a wide NULL terminator.
- *                      Refer to MfciPkg/Include/MfciVariables.h for more details.
- *
- * @param[out] StringSize  (OPTIONAL) String size in bytes including the wide NULL terminator.
- *                         NULL may be supplied if the size is not requested (it is NULL terminated after all)
- *
- * It is the callers responsibility to free the String buffer returned using FreePool()
- *
- * @return EFI_STATUS
- *
- * @retval EFI_UNSUPPORTED         Likely using the NULL library instance
- * @retval EFI_SUCCESS             Successfully retrieved the string and length
- */
-
+  Function pointer definition to get a device-unique targeting value.
+  It is the callers responsibility to free the String buffer returned using FreePool().
+
+  @param[out] String      Device targeting value, a UTF-16 little endian string.
+                          Includes a wide NULL terminator.
+                          Refer to MfciPkg/Include/MfciVariables.h for more details.
+  @param[out] StringSize  (OPTIONAL) String size in bytes including the wide NULL terminator.
+                          NULL may be supplied if the size is not requested (it is NULL terminated after all)
+
+  @retval EFI_UNSUPPORTED       Likely using the NULL library instance.
+  @retval EFI_SUCCESS           Successfully retrieved the string and length.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough memory to allocate the string.
+  @retval EFI_INVALID_PARAMETER The String is NULL.
+**/
 typedef
 EFI_STATUS
 (EFIAPI *MFCI_DEVICE_ID_FN)(
   OUT CHAR16 **String,
-  OUT UINTN *StringSize
+  OUT UINTN *StringSize OPTIONAL
   );
 
 /**
- * Get the Manufacturer Name
- *
- * @param[out] Manufacturer
- * @param[out] ManufacturerSize
- *
- * It is the callers responsibility to free the buffer returned using FreePool()
- *
- * @return EFI_STATUS
- */
+  Function that returns Manufacturer Name of the device and string size upon on return.
+  It is the callers responsibility to free the buffer returned using FreePool().
+
+  @param[out] Manufacturer      The Manufacturer string to be returned.
+  @param[out] ManufacturerSize  The size of the Manufacturer string.
+
+  @retval EFI_SUCCESS           The Manufacturer string was successfully returned.
+  @retval EFI_UNSUPPORTED       The function is not supported.
+  @retval EFI_INVALID_PARAMETER The Manufacturer is NULL.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough memory to allocate the Manufacturer string.
+**/
 EFI_STATUS
 EFIAPI
 MfciIdSupportGetManufacturer (
@@ -58,16 +55,17 @@ MfciIdSupportGetManufacturer (
   );
 
 /**
- *
- * Get the Product Name
- *
- * @param[out] ProductName
- * @param[out] ProductNameSize
- *
- * It is the callers responsibility to free the buffer returned using FreePool()
- *
- * @return EFI_STATUS
- */
+  Function that returns the Product Name string and size upon on return.
+  It is the callers responsibility to free the buffer returned using FreePool().
+
+  @param[out] ProductName     The ProductName string to be returned.
+  @param[out] ProductNameSize The size of the ProductName string.
+
+  @retval EFI_SUCCESS           The ProductName string was successfully returned.
+  @retval EFI_UNSUPPORTED       The function is not supported.
+  @retval EFI_INVALID_PARAMETER The ProductName is NULL.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough memory to allocate the ProductName string.
+**/
 EFI_STATUS
 EFIAPI
 MfciIdSupportGetProductName (
@@ -76,15 +74,17 @@ MfciIdSupportGetProductName (
   );
 
 /**
- * Get the SerialNumber
- *
- * @param[out] SerialNumber
- * @param[out] SerialNumberSize
- *
- * It is the callers responsibility to free the buffer returned using FreePool()
- *
- * @return EFI_STATUS
- */
+  Function that returns the SerialNumber string and size upon on return.
+  It is the callers responsibility to free the buffer returned using FreePool().
+
+  @param[out] SerialNumber
+  @param[out] SerialNumberSize
+
+  @retval EFI_SUCCESS           The ProductName string was successfully returned.
+  @retval EFI_UNSUPPORTED       The function is not supported.
+  @retval EFI_INVALID_PARAMETER The ProductName is NULL.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough memory to allocate the ProductName string.
+**/
 EFI_STATUS
 EFIAPI
 MfciIdSupportGetSerialNumber (
@@ -93,15 +93,17 @@ MfciIdSupportGetSerialNumber (
   );
 
 /**
- * Get OEM1
- *
- * @param[out] Oem1
- * @param[out] Oem1Size
- *
- * It is the callers responsibility to free the buffer returned using FreePool()
- *
- * @return EFI_STATUS
- */
+  Function that returns the Oem1 string and size upon on return.
+  It is the callers responsibility to free the buffer returned using FreePool().
+
+  @param[out] Oem1      The OEM1 string to be returned.
+  @param[out] Oem1Size  The size of the OEM1 string.
+
+  @retval EFI_SUCCESS           The OEM1 string was successfully returned.
+  @retval EFI_UNSUPPORTED       The function is not supported.
+  @retval EFI_INVALID_PARAMETER The Oem1 is NULL.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough memory to allocate the OEM1 string.
+**/
 EFI_STATUS
 EFIAPI
 MfciIdSupportGetOem1 (
@@ -110,25 +112,27 @@ MfciIdSupportGetOem1 (
   );
 
 /**
- * Get OEM2
- *
- * @param[out] Oem2
- * @param[out] Oem2Size
- *
- * It is the callers responsibility to free the buffer returned using FreePool()
- *
- * @return EFI_STATUS
- */
+  Function that returns the Oem2 string and size upon on return.
+  It is the callers responsibility to free the buffer returned using FreePool().
+
+  @param[out] Oem2      The OEM2 string to be returned.
+  @param[out] Oem2Size  The size of the OEM2 string.
+
+  @retval EFI_SUCCESS           The OEM2 string was successfully returned.
+  @retval EFI_UNSUPPORTED       The function is not supported.
+  @retval EFI_INVALID_PARAMETER The Oem2 is NULL.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough memory to allocate the OEM2 string.
+**/
 EFI_STATUS
 EFIAPI
 MfciIdSupportGetOem2 (
   OUT CHAR16  **Oem2,
   OUT UINTN   *Oem2Size  OPTIONAL
   );
 
-/**
- * the following helps iterate over the functions and set the corresponding target variable names
- */
+//
+// The following helps iterate over the functions and set the corresponding target variable names.
+//
 
 // define a structure that pairs up the function pointer with the UEFI variable name
 typedef struct {
@@ -138,12 +142,7 @@ typedef struct {
 
 // populate the array of structures that pair up the functions with variable names
 #define MFCI_TARGET_VAR_COUNT  5
-STATIC CONST MFCI_DEVICE_ID_FN_TO_VAR_NAME_MAP  gDeviceIdFnToTargetVarNameMap[MFCI_TARGET_VAR_COUNT] = {
-  { MfciIdSupportGetManufacturer, MFCI_MANUFACTURER_VARIABLE_NAME },
-  { MfciIdSupportGetProductName,  MFCI_PRODUCT_VARIABLE_NAME      },
-  { MfciIdSupportGetSerialNumber, MFCI_SERIALNUMBER_VARIABLE_NAME },
-  { MfciIdSupportGetOem1,         MFCI_OEM_01_VARIABLE_NAME       },
-  { MfciIdSupportGetOem2,         MFCI_OEM_02_VARIABLE_NAME       }
-};
-
-#endif //__MFCI_DEVICE_ID_SUPPORT_LIB_H__
+
+extern CONST MFCI_DEVICE_ID_FN_TO_VAR_NAME_MAP  gDeviceIdFnToTargetVarNameMap[MFCI_TARGET_VAR_COUNT];
+
+#endif //MFCI_DEVICE_ID_SUPPORT_LIB_H_

MfciPkg/Library/MfciDeviceIdSupportLibNull/MfciDeviceIdSupportLibNull.c

@@ -13,6 +13,8 @@
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
 
+CONST MFCI_DEVICE_ID_FN_TO_VAR_NAME_MAP  gDeviceIdFnToTargetVarNameMap[MFCI_TARGET_VAR_COUNT] = { 0 };
+
 /**
  * Get the Manufacturer Name
  *

MfciPkg/Library/MfciDeviceIdSupportLibSmbios/MfciDeviceIdSupportLibSmbios.c

@@ -21,6 +21,15 @@
 
 #define ID_NOT_FOUND  "Not Found"
 
+// populate the array of structures that pair up the functions with variable names
+CONST MFCI_DEVICE_ID_FN_TO_VAR_NAME_MAP  gDeviceIdFnToTargetVarNameMap[MFCI_TARGET_VAR_COUNT] = {
+  { MfciIdSupportGetManufacturer, MFCI_MANUFACTURER_VARIABLE_NAME },
+  { MfciIdSupportGetProductName,  MFCI_PRODUCT_VARIABLE_NAME      },
+  { MfciIdSupportGetSerialNumber, MFCI_SERIALNUMBER_VARIABLE_NAME },
+  { MfciIdSupportGetOem1,         MFCI_OEM_01_VARIABLE_NAME       },
+  { MfciIdSupportGetOem2,         MFCI_OEM_02_VARIABLE_NAME       }
+};
+
 // Note: This protocol will guarantee to be met by the Depex and located at the
 // constructor of this library, thus no null-pointer check in library code flow.
 EFI_SMBIOS_PROTOCOL  *mSmbiosProtocol;

MfciPkg/MfciDxe/MfciTargeting.c

@@ -21,6 +21,29 @@
 
 #include "MfciDxe.h"
 
+/**
+  The strings of the names in the MFCI Policy name/value pairs
+**/
+CONST CHAR16  gPolicyBlobFieldName[MFCI_POLICY_FIELD_COUNT][MFCI_POLICY_FIELD_MAX_LEN] = {
+  L"Target\\Manufacturer",
+  L"Target\\Product",
+  L"Target\\SerialNumber",
+  L"Target\\OEM_01",
+  L"Target\\OEM_02",
+  L"Target\\Nonce", // this is nonce targeted by the binary policy blob
+  L"UEFI\\Policy"
+};
+
+CONST CHAR16  gPolicyTargetFieldVarNames[TARGET_POLICY_COUNT][MFCI_VAR_NAME_MAX_LENGTH] = {
+  MFCI_MANUFACTURER_VARIABLE_NAME,
+  MFCI_PRODUCT_VARIABLE_NAME,
+  MFCI_SERIALNUMBER_VARIABLE_NAME,
+  MFCI_OEM_01_VARIABLE_NAME,
+  MFCI_OEM_02_VARIABLE_NAME
+  // the platform has 2 nonce variables, one for verifying the current policy, another for verifying a next policy
+  // this complexity is handled elsewhere
+};
+
 STATIC
 EFI_STATUS
 GetOemField (

MfciPkg/MfciDxe/Test/MfciMultipleCertsHostTest.c

@@ -45,6 +45,8 @@
 #define UNIT_TEST_NAME     "Mfci Multiple Certificates Host Test"
 #define UNIT_TEST_VERSION  "0.1"
 
+CONST CHAR16  gPolicyTargetFieldVarNames[TARGET_POLICY_COUNT][MFCI_VAR_NAME_MAX_LENGTH] = { 0 };
+
 BOOLEAN
 EFIAPI
 GetRandomNumber64 (

MfciPkg/Private/MfciPolicyFields.h

@@ -46,28 +46,13 @@ typedef enum {
 /**
   The strings of the names in the MFCI Policy name/value pairs
 **/
-STATIC CONST CHAR16  gPolicyBlobFieldName[MFCI_POLICY_FIELD_COUNT][MFCI_POLICY_FIELD_MAX_LEN] = {
-  L"Target\\Manufacturer",
-  L"Target\\Product",
-  L"Target\\SerialNumber",
-  L"Target\\OEM_01",
-  L"Target\\OEM_02",
-  L"Target\\Nonce", // this is nonce targeted by the binary policy blob
-  L"UEFI\\Policy"
-};
+extern CONST CHAR16  gPolicyBlobFieldName[MFCI_POLICY_FIELD_COUNT][MFCI_POLICY_FIELD_MAX_LEN];
 
 /**
   A helper that maps static MFCI Policy targeting fields to their corresponding UEFI variable names
 **/
 #define TARGET_POLICY_COUNT  5
-STATIC CONST CHAR16  gPolicyTargetFieldVarNames[TARGET_POLICY_COUNT][MFCI_VAR_NAME_MAX_LENGTH] = {
-  MFCI_MANUFACTURER_VARIABLE_NAME,
-  MFCI_PRODUCT_VARIABLE_NAME,
-  MFCI_SERIALNUMBER_VARIABLE_NAME,
-  MFCI_OEM_01_VARIABLE_NAME,
-  MFCI_OEM_02_VARIABLE_NAME
-  // the platform has 2 nonce variables, one for verifying the current policy, another for verifying a next policy
-  // this complexity is handled elsewhere
-};
+
+extern CONST CHAR16  gPolicyTargetFieldVarNames[TARGET_POLICY_COUNT][MFCI_VAR_NAME_MAX_LENGTH];
 
 #endif //__MFCI_POLICY_FIELDS_H__

MfciPkg/UnitTests/MfciPkgHostTest.dsc

@@ -49,7 +49,7 @@
 
 [LibraryClasses]
   MfciPolicyParsingLib|MfciPkg/Private/Library/MfciPolicyParsingLibNull/MfciPolicyParsingLibNull.inf
-  MfciDeviceIdSupportLib|MfciPkg/Library/MfciDeviceIdSupportLibNull/MfciDeviceIdSupportLibNull.inf
+  MfciDeviceIdSupportLib|MfciPkg/Library/MfciDeviceIdSupportLibSmbios/MfciDeviceIdSupportLibSmbios.inf
   VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
   UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
   UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf

MfciPkg/UnitTests/MfciPolicyParsingUnitTest/MfciPolicyParsingUnitTestApp.inf

@@ -21,12 +21,16 @@
 
 [Sources]
   MfciPolicyParsingUnitTestApp.c
+  ../../MfciDxe/MfciTargeting.c
 
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MfciPkg/MfciPkg.dec
 
+[Guids]
+  gMfciVendorGuid   ## CONSUMES
+
 [Pcd]
   gMfciPkgTokenSpaceGuid.PcdMfciPkcs7CertBufferXdr    ## CONSUMES
   gMfciPkgTokenSpaceGuid.PcdMfciPkcs7RequiredLeafEKU  ## CONSUMES