Re: [Patch V3] IntelSiliconPkg/Library:Add BaseConfigBlockLib Library

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Tsao, Ethan <ethan.tsao@intel.com>
> Sent: Monday, November 11, 2019 2:05 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [Patch V3] IntelSiliconPkg/Library:Add BaseConfigBlockLib Library
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2318
> 
> Establish one copy of Config blocks library class and instance
> in IntelSiliconPkg and remove copies from other silicon packages
> , like KabyLakeSiliconPkg, CoffelakeSiliconPkg.
> 
> Signed-off-by: Ethan Tsao <ethan.tsao@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> ---
> 
> Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlo
> ckLib.c                      | 146 ------------------------------------------------------------------
> --------------------------------------------------------------------------------
> 
> Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlo
> ckLib.inf                    |  29 -----------------------------
>  Silicon/Intel/{KabylakeSiliconPkg =>
> IntelSiliconPkg}/Library/BaseConfigBlockLib/BaseConfigBlockLib.c   |  10 ++---
> -----
>  Silicon/Intel/{KabylakeSiliconPkg =>
> IntelSiliconPkg}/Library/BaseConfigBlockLib/BaseConfigBlockLib.inf |   0
>  4 files changed, 2 insertions(+), 183 deletions(-)
> 
> diff --git
> a/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigB
> lockLib.c
> b/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfig
> BlockLib.c
> deleted file mode 100644
> index 369dab97ee..0000000000
> ---
> a/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigB
> lockLib.c
> +++ /dev/null
> @@ -1,146 +0,0 @@
> -/** @file
> -  Library functions for Config Block management.
> -
> -  Copyright (c) 2019 Intel Corporation. All rights reserved. <BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#include <ConfigBlock.h>
> -#include <Library/ConfigBlockLib.h>
> -#include <Library/BaseMemoryLib.h>
> -#include <Library/MemoryAllocationLib.h>
> -#include <Library/DebugLib.h>
> -
> -/**
> -  Create config block table
> -
> -  @param[in]     TotalSize                    - Max size to be allocated for the Config
> Block Table
> -  @param[out]    ConfigBlockTableAddress      - On return, points to a pointer
> to the beginning of Config Block Table Address
> -
> -  @retval EFI_INVALID_PARAMETER - Invalid Parameter
> -  @retval EFI_OUT_OF_RESOURCES  - Out of resources
> -  @retval EFI_SUCCESS           - Successfully created Config Block Table at
> ConfigBlockTableAddress
> -**/
> -EFI_STATUS
> -EFIAPI
> -CreateConfigBlockTable (
> -  IN     UINT16    TotalSize,
> -  OUT    VOID      **ConfigBlockTableAddress
> -  )
> -{
> -  CONFIG_BLOCK_TABLE_HEADER *ConfigBlkTblAddrPtr;
> -  UINT32                    ConfigBlkTblHdrSize;
> -
> -  ConfigBlkTblHdrSize = (UINT32)(sizeof (CONFIG_BLOCK_TABLE_HEADER));
> -
> -  if (TotalSize <= (ConfigBlkTblHdrSize + sizeof (CONFIG_BLOCK_HEADER))) {
> -    DEBUG ((DEBUG_ERROR, "Invalid Parameter\n"));
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  ConfigBlkTblAddrPtr = (CONFIG_BLOCK_TABLE_HEADER
> *)AllocateZeroPool (TotalSize);
> -  if (ConfigBlkTblAddrPtr == NULL) {
> -    DEBUG ((DEBUG_ERROR, "Could not allocate memory.\n"));
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -  ConfigBlkTblAddrPtr->NumberOfBlocks = 0;
> -  ConfigBlkTblAddrPtr->Header.GuidHob.Header.HobLength = TotalSize;
> -  ConfigBlkTblAddrPtr->AvailableSize = TotalSize - ConfigBlkTblHdrSize;
> -
> -  *ConfigBlockTableAddress = (VOID *)ConfigBlkTblAddrPtr;
> -
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Add config block into config block table structure
> -
> -  @param[in]     ConfigBlockTableAddress      - A pointer to the beginning of
> Config Block Table Address
> -  @param[out]    ConfigBlockAddress           - On return, points to a pointer to
> the beginning of Config Block Address
> -
> -  @retval EFI_OUT_OF_RESOURCES - Config Block Table is full and cannot
> add new Config Block or
> -                                 Config Block Offset Table is full and cannot add new Config
> Block.
> -  @retval EFI_SUCCESS          - Successfully added Config Block
> -**/
> -EFI_STATUS
> -EFIAPI
> -AddConfigBlock (
> -  IN     VOID      *ConfigBlockTableAddress,
> -  OUT    VOID      **ConfigBlockAddress
> -  )
> -{
> -  CONFIG_BLOCK              *TempConfigBlk;
> -  CONFIG_BLOCK_TABLE_HEADER *ConfigBlkTblAddrPtr;
> -  CONFIG_BLOCK              *ConfigBlkAddrPtr;
> -  UINT16                    ConfigBlkSize;
> -
> -  ConfigBlkTblAddrPtr = (CONFIG_BLOCK_TABLE_HEADER
> *)ConfigBlockTableAddress;
> -  ConfigBlkAddrPtr = (CONFIG_BLOCK *)(*ConfigBlockAddress);
> -  ConfigBlkSize = ConfigBlkAddrPtr->Header.GuidHob.Header.HobLength;
> -  DEBUG ((DEBUG_INFO, "Config Block GUID: %g / Config Block Size: 0x%x
> bytes\n", &(ConfigBlkAddrPtr->Header.GuidHob.Name), ConfigBlkSize));
> -  if ((ConfigBlkSize % 4) != 0) {
> -    DEBUG ((DEBUG_ERROR, "Config Block must be multiples of 4 bytes\n"));
> -    return EFI_INVALID_PARAMETER;
> -  }
> -  if (ConfigBlkTblAddrPtr->AvailableSize < ConfigBlkSize) {
> -    DEBUG ((DEBUG_ERROR, "Config Block Table is full and cannot add new
> Config Block.\n"));
> -    DEBUG ((DEBUG_ERROR, "Available Config Block Size: 0x%x bytes /
> Requested Config Block Size: 0x%x bytes\n", ConfigBlkTblAddrPtr-
> >AvailableSize, ConfigBlkSize));
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  TempConfigBlk = (CONFIG_BLOCK *)((UINTN)ConfigBlkTblAddrPtr +
> (UINTN)(ConfigBlkTblAddrPtr->Header.GuidHob.Header.HobLength -
> ConfigBlkTblAddrPtr->AvailableSize));
> -  CopyMem (&TempConfigBlk->Header, &ConfigBlkAddrPtr->Header,
> sizeof(CONFIG_BLOCK_HEADER));
> -
> -  ConfigBlkTblAddrPtr->NumberOfBlocks++;
> -  ConfigBlkTblAddrPtr->AvailableSize = ConfigBlkTblAddrPtr->AvailableSize -
> ConfigBlkSize;
> -
> -  *ConfigBlockAddress = (VOID *) TempConfigBlk;
> -  DEBUG ((DEBUG_INFO, "Config Block Address: 0x%x / Available Config
> Block Size: 0x%x bytes\n", (UINT32)(UINTN)*ConfigBlockAddress,
> ConfigBlkTblAddrPtr->AvailableSize));
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Retrieve a specific Config Block data by GUID
> -
> -  @param[in]      ConfigBlockTableAddress      - A pointer to the beginning of
> Config Block Table Address
> -  @param[in]      ConfigBlockGuid              - A pointer to the GUID uses to
> search specific Config Block
> -  @param[out]     ConfigBlockAddress           - On return, points to a pointer to
> the beginning of Config Block Address
> -
> -  @retval EFI_NOT_FOUND         - Could not find the Config Block
> -  @retval EFI_SUCCESS           - Config Block found and return
> -**/
> -EFI_STATUS
> -EFIAPI
> -GetConfigBlock (
> -  IN     VOID      *ConfigBlockTableAddress,
> -  IN     EFI_GUID  *ConfigBlockGuid,
> -  OUT    VOID      **ConfigBlockAddress
> -  )
> -{
> -  UINT16                    OffsetIndex;
> -  CONFIG_BLOCK              *TempConfigBlk;
> -  CONFIG_BLOCK_TABLE_HEADER *ConfigBlkTblAddrPtr;
> -  UINT32                    ConfigBlkTblHdrSize;
> -  UINT32                    ConfigBlkOffset;
> -  UINT16                    NumOfBlocks;
> -
> -  ConfigBlkTblHdrSize = (UINT32)(sizeof (CONFIG_BLOCK_TABLE_HEADER));
> -  ConfigBlkTblAddrPtr = (CONFIG_BLOCK_TABLE_HEADER
> *)ConfigBlockTableAddress;
> -  NumOfBlocks = ConfigBlkTblAddrPtr->NumberOfBlocks;
> -
> -  ConfigBlkOffset = 0;
> -  for (OffsetIndex = 0; OffsetIndex < NumOfBlocks; OffsetIndex++) {
> -    if ((ConfigBlkTblHdrSize + ConfigBlkOffset) > (ConfigBlkTblAddrPtr-
> >Header.GuidHob.Header.HobLength)) {
> -      break;
> -    }
> -    TempConfigBlk = (CONFIG_BLOCK *)((UINTN)ConfigBlkTblAddrPtr +
> (UINTN)ConfigBlkTblHdrSize + (UINTN)ConfigBlkOffset);
> -    if (CompareGuid (&(TempConfigBlk->Header.GuidHob.Name),
> ConfigBlockGuid)) {
> -      *ConfigBlockAddress = (VOID *)TempConfigBlk;
> -      return EFI_SUCCESS;
> -    }
> -    ConfigBlkOffset = ConfigBlkOffset + TempConfigBlk-
> >Header.GuidHob.Header.HobLength;
> -  }
> -  DEBUG ((DEBUG_ERROR, "Could not find the config block.\n"));
> -  return EFI_NOT_FOUND;
> -}
> diff --git
> a/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigB
> lockLib.inf
> b/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfig
> BlockLib.inf
> deleted file mode 100644
> index a7def2481d..0000000000
> ---
> a/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigB
> lockLib.inf
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -## @file
> -# Component INF file for the BaseConfigBlock library.
> -#
> -# Copyright (c) 2019 Intel Corporation. All rights reserved. <BR>
> -#
> -# SPDX-License-Identifier: BSD-2-Clause-Patent
> -#
> -##
> -
> -[Defines]
> -INF_VERSION = 0x00010017
> -BASE_NAME = BaseConfigBlockLib
> -FILE_GUID = 1EC07EA8-7808-4e06-9D79-309AE331D2D5
> -VERSION_STRING = 1.0
> -MODULE_TYPE = BASE
> -LIBRARY_CLASS = ConfigBlockLib
> -
> -
> -[Packages]
> -MdePkg/MdePkg.dec
> -CoffeelakeSiliconPkg/SiPkg.dec
> -
> -[Sources]
> -BaseConfigBlockLib.c
> -
> -[LibraryClasses]
> -DebugLib
> -BaseMemoryLib
> -MemoryAllocationLib
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlo
> ckLib.c
> b/Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLi
> b.c
> similarity index 84%
> rename from
> Silicon/Intel/KabylakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlock
> Lib.c
> rename to
> Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLib.c
> index b9bd6f3a48..77eb2244c2 100644
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlo
> ckLib.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLi
> b.c
> @@ -34,13 +34,11 @@ CreateConfigBlockTable (
>    ConfigBlkTblHdrSize = (UINT32)(sizeof (CONFIG_BLOCK_TABLE_HEADER));
> 
>    if (TotalSize <= (ConfigBlkTblHdrSize + sizeof (CONFIG_BLOCK_HEADER))) {
> -    DEBUG ((DEBUG_ERROR, "Invalid Parameter\n"));
>      return EFI_INVALID_PARAMETER;
>    }
> 
>    ConfigBlkTblAddrPtr = (CONFIG_BLOCK_TABLE_HEADER *)AllocateZeroPool
> (TotalSize);
>    if (ConfigBlkTblAddrPtr == NULL) {
> -    DEBUG ((DEBUG_ERROR, "Could not allocate memory.\n"));
>      return EFI_OUT_OF_RESOURCES;
>    }
>    ConfigBlkTblAddrPtr->NumberOfBlocks = 0;
> @@ -77,14 +75,11 @@ AddConfigBlock (
>    ConfigBlkTblAddrPtr = (CONFIG_BLOCK_TABLE_HEADER
> *)ConfigBlockTableAddress;
>    ConfigBlkAddrPtr = (CONFIG_BLOCK *)(*ConfigBlockAddress);
>    ConfigBlkSize = ConfigBlkAddrPtr->Header.GuidHob.Header.HobLength;
> -  DEBUG ((DEBUG_INFO, "Config Block GUID: %g / Config Block Size: 0x%x
> bytes\n", &(ConfigBlkAddrPtr->Header.GuidHob.Name), ConfigBlkSize));
> +
>    if ((ConfigBlkSize % 4) != 0) {
> -    DEBUG ((DEBUG_ERROR, "Config Block must be multiples of 4 bytes\n"));
>      return EFI_INVALID_PARAMETER;
>    }
>    if (ConfigBlkTblAddrPtr->AvailableSize < ConfigBlkSize) {
> -    DEBUG ((DEBUG_ERROR, "Config Block Table is full and cannot add new
> Config Block.\n"));
> -    DEBUG ((DEBUG_ERROR, "Available Config Block Size: 0x%x bytes /
> Requested Config Block Size: 0x%x bytes\n", ConfigBlkTblAddrPtr-
> >AvailableSize, ConfigBlkSize));
>      return EFI_OUT_OF_RESOURCES;
>    }
> 
> @@ -95,7 +90,6 @@ AddConfigBlock (
>    ConfigBlkTblAddrPtr->AvailableSize = ConfigBlkTblAddrPtr->AvailableSize -
> ConfigBlkSize;
> 
>    *ConfigBlockAddress = (VOID *) TempConfigBlk;
> -  DEBUG ((DEBUG_INFO, "Config Block Address: 0x%x / Available Config
> Block Size: 0x%x bytes\n", (UINT32)(UINTN)*ConfigBlockAddress,
> ConfigBlkTblAddrPtr->AvailableSize));
>    return EFI_SUCCESS;
>  }
> 
> @@ -140,6 +134,6 @@ GetConfigBlock (
>      }
>      ConfigBlkOffset = ConfigBlkOffset + TempConfigBlk-
> >Header.GuidHob.Header.HobLength;
>    }
> -  DEBUG ((DEBUG_ERROR, "Could not find the config block.\n"));
> +
>    return EFI_NOT_FOUND;
>  }
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlo
> ckLib.inf
> b/Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLi
> b.inf
> similarity index 100%
> rename from
> Silicon/Intel/KabylakeSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlock
> Lib.inf
> rename to
> Silicon/Intel/IntelSiliconPkg/Library/BaseConfigBlockLib/BaseConfigBlockLib.i
> nf
> --
> 2.16.2.windows.1