Re: [edk2-platforms] [PATCH 4/5] WhiskeylakeOpenBoardPkg: Add BIOS INFO PEIM

["Kubacki, Michael A" <michael.a.kubacki@intel.com>]

Same feedback as patch #2:

BiosInfo.c:

BiosInfoEntryPoint () function description should be updated. The function installs the BiosInfo PPI
 but also produces the BiosInfo HOB.

It also looks like you can remove the following macros for now:
#define INDEXPORT_TO_ADDRESS(x)             (x)
#define DATAPORT_TO_ADDRESS(x)              ((x) << 16)
#define PORTWIDTH_TO_ADDRESS(x)             ((x) << 32)
#define PORTBITNUMBER_TO_ADDRESS(x)         ((x) << 40)
#define PORTINDEXNUMBER_TO_ADDRESS(x)       ((x) << 48)

> -----Original Message-----
> From: Agyeman, Prince <prince.agyeman@intel.com>
> Sent: Thursday, October 3, 2019 2:13 PM
> To: devel@edk2.groups.io
> Cc: Sinha, Ankit <ankit.sinha@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Kubacki, Michael A
> <michael.a.kubacki@intel.com>
> Subject: [edk2-platforms] [PATCH 4/5] WhiskeylakeOpenBoardPkg: Add BIOS
> INFO PEIM
> 
> Added BIOS info PEIM to publish Bios Info HOB. This PEIM currently publishes
> the microcode FV info.
> 
> Cc: Ankit Sinha <ankit.sinha@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Kubacki Michael A <michael.a.kubacki@intel.com>
> 
> Signed-off-by: Prince Agyeman <prince.agyeman@intel.com>
> ---
>  .../WhiskeylakeURvp/BiosInfo/BiosInfo.c       | 93 +++++++++++++++++++
>  .../WhiskeylakeURvp/BiosInfo/BiosInfo.inf     | 48 ++++++++++
>  .../WhiskeylakeURvp/OpenBoardPkg.dsc          |  2 +
>  .../WhiskeylakeURvp/OpenBoardPkg.fdf          |  1 +
>  4 files changed, 144 insertions(+)
>  create mode 100644
> Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosI
> nfo.c
>  create mode 100644
> Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosI
> nfo.inf
> 
> diff --git
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/Bi
> osInfo.c
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/Bi
> osInfo.c
> new file mode 100644
> index 0000000000..6a058a0fc2
> --- /dev/null
> +++
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/Bi
> +++ osInfo.c
> @@ -0,0 +1,93 @@
> +/** @file
> +  Driver for BIOS Info support.
> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#include <PiPei.h>
> +#include <Library/BiosInfo.h>
> +#include <Library/PeiServicesLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PcdLib.h>
> +#include <IndustryStandard/FirmwareInterfaceTable.h>
> +
> +#define INDEXPORT_TO_ADDRESS(x)             (x)
> +#define DATAPORT_TO_ADDRESS(x)              ((x) << 16)
> +#define PORTWIDTH_TO_ADDRESS(x)             ((x) << 32)
> +#define PORTBITNUMBER_TO_ADDRESS(x)         ((x) << 40)
> +#define PORTINDEXNUMBER_TO_ADDRESS(x)       ((x) << 48)
> +
> +//
> +// Internal
> +//
> +#pragma pack (1)
> +
> +typedef struct {
> +  BIOS_INFO_HEADER  Header;
> +  BIOS_INFO_STRUCT  Entry[1];
> +} BIOS_INFO;
> +#pragma pack ()
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED BIOS_INFO  mBiosInfo = {
> +  {
> +    BIOS_INFO_SIGNATURE,
> +    1,
> +    0,
> +  },
> +  {
> +    {
> +      FIT_TYPE_01_MICROCODE,
> +      BIOS_INFO_STRUCT_ATTRIBUTE_MICROCODE_WHOLE_REGION,
> +      0x0100,
> +      FixedPcdGet32 (PcdFlashMicrocodeFvSize),
> +      FixedPcdGet32 (PcdFlashMicrocodeFvBase)
> +    }
> +  }
> +};
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR
> mBiosInfoPpiList
> += {
> +  EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> +  &gBiosInfoGuid,
> +  &mBiosInfo
> +};
> +
> +/**
> +  Installs BiosInfo Ppi.
> +
> +  @param  FileHandle  Handle of the file being invoked.
> +  @param  PeiServices Describes the list of possible PEI Services.
> +
> +  @retval EFI_SUCCESS   Install the BiosInfo Ppi successfully.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +BiosInfoEntryPoint (
> +  IN       EFI_PEI_FILE_HANDLE  FileHandle,
> +  IN CONST EFI_PEI_SERVICES     **PeiServices
> +  )
> +{
> +  EFI_STATUS  Status;
> +  VOID        *HobData;
> +
> +  //
> +  // Install PPI, so that other PEI module can add dependency.
> +  //
> +  Status = PeiServicesInstallPpi (&mBiosInfoPpiList);  ASSERT_EFI_ERROR
> + (Status);
> +
> +  //
> +  // Build hob, so that DXE module can also get the data.
> +  //
> +  HobData = BuildGuidHob (&gBiosInfoGuid, sizeof (mBiosInfo));  ASSERT
> + (HobData != NULL);  if (HobData == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +  CopyMem (HobData, &mBiosInfo, sizeof (mBiosInfo));
> +
> +  return EFI_SUCCESS;
> +}
> diff --git
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/Bi
> osInfo.inf
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/Bi
> osInfo.inf
> new file mode 100644
> index 0000000000..f268de00f5
> --- /dev/null
> +++
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/Bi
> +++ osInfo.inf
> @@ -0,0 +1,48 @@
> +### @file
> +#  Module Information description file for BIOS Info Driver # #
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> #
> +SPDX-License-Identifier: BSD-2-Clause-Patent ###
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010017
> +  BASE_NAME                      = BiosInfo
> +  FILE_GUID                      = A842B2D2-5C88-44E9-A9E2-4830F26662B7
> +  VERSION_STRING                 = 1.0
> +  MODULE_TYPE                    = PEIM
> +  ENTRY_POINT                    = BiosInfoEntryPoint
> +#
> +# The following information is for reference only and not required by the
> build tools.
> +#
> +# VALID_ARCHITECTURES IA32 X64
> +#
> +
> +[LibraryClasses]
> +  PeimEntryPoint
> +  PeiServicesLib
> +  HobLib
> +  BaseMemoryLib
> +  DebugLib
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  IntelSiliconPkg/IntelSiliconPkg.dec
> +  CoffeelakeSiliconPkg/SiPkg.dec
> +  CoffeeLakeFspBinPkg/CoffeeLakeFspBinPkg.dec
> +  BoardModulePkg/BoardModulePkg.dec
> +  MinPlatformPkg/MinPlatformPkg.dec
> +
> +[Pcd]
> +  gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase                    ##
> CONSUMES
> +  gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize                    ## CONSUMES
> +
> +[Sources]
> +  BiosInfo.c
> +
> +[Guids]
> +  gBiosInfoGuid                                 ## PRODUCES
> +
> +[Depex]
> +  TRUE
> diff --git
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pkg.dsc
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pkg.dsc
> index eea809140c..00a89f4507 100644
> ---
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pkg.dsc
> +++
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pk
> +++ g.dsc
> @@ -246,6 +246,8 @@
> 
> SiliconPolicyUpdateLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyUp
> dateLibNull/SiliconPolicyUpdateLibNull.inf
>    }
> 
> +  $(PROJECT)/BiosInfo/BiosInfo.inf
> +
>    #
>    # Security
>    #
> diff --git
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pkg.fdf
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pkg.fdf
> index 611078e4b4..867cee8a64 100644
> ---
> a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pkg.fdf
> +++
> b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoard
> Pk
> +++ g.fdf
> @@ -237,6 +237,7 @@ INF
> $(PLATFORM_PACKAGE)/PlatformInit/ReportFv/ReportFvPei.inf
>  INF
> $(PLATFORM_PACKAGE)/PlatformInit/PlatformInitPei/PlatformInitPreMem.i
> nf
>  INF IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
>  INF
> $(PLATFORM_PACKAGE)/PlatformInit/SiliconPolicyPei/SiliconPolicyPeiPreMe
> m.inf
> +INF $(PROJECT)/BiosInfo/BiosInfo.inf
> 
>  [FV.FvPostMemoryUncompact]
>  BlockSize          = $(FLASH_BLOCK_SIZE)
> --
> 2.19.1.windows.1