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

2019-10-07 Thread Kubacki, Michael A
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 
> Sent: Thursday, October 3, 2019 2:13 PM
> To: devel@edk2.groups.io
> Cc: Sinha, Ankit ; Desimone, Nathaniel L
> ; Kubacki, Michael A
> 
> 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 
> Cc: Nate DeSimone 
> Cc: Kubacki Michael A 
> 
> Signed-off-by: Prince Agyeman 
> ---
>  .../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 00..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.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#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,
> +  ,
> +  
> +};
> +
> +/**
> +  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 ();  ASSERT_EFI_ERROR
> + (Status);
> +
> +  //
> +  // Build hob, so that DXE module can also get the data.
> +  //
> +  HobData = BuildGuidHob (, sizeof (mBiosInfo));  ASSERT
> + (HobData != NULL);  if (HobData == NULL) {
> +return EFI_OUT_OF_RESOURCES;
> +  }
> +  CopyMem (HobData, , 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 00..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. #
> +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 

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

2019-10-07 Thread Nate DeSimone
Same comment as PATCH 2 & 3, it seems like this PEIM could be used generically 
by any Whiskeylake board so I would prefer one copy of it in 
WhiskeylakeOpenBoardPkg.

-Original Message-
From: Agyeman, Prince  
Sent: Thursday, October 3, 2019 2:13 PM
To: devel@edk2.groups.io
Cc: Sinha, Ankit ; Desimone, Nathaniel L 
; Kubacki, Michael A 

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 
Cc: Nate DeSimone 
Cc: Kubacki Michael A 

Signed-off-by: Prince Agyeman 
---
 .../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/BiosInfo.c
 create mode 100644 
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.inf

diff --git 
a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.c 
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.c
new file mode 100644
index 00..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.
+  SPDX-License-Identifier: BSD-2-Clause-Patent **/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#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,
+  ,
+  
+};
+
+/**
+  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 ();  ASSERT_EFI_ERROR 
+ (Status);
+
+  //
+  // Build hob, so that DXE module can also get the data.
+  //
+  HobData = BuildGuidHob (, sizeof (mBiosInfo));  ASSERT 
+ (HobData != NULL);  if (HobData == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+  CopyMem (HobData, , sizeof (mBiosInfo));
+
+  return EFI_SUCCESS;
+}
diff --git 
a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.inf 
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.inf
new file mode 100644
index 00..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. #
+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]
+  

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

2019-10-03 Thread Agyeman, Prince
Added BIOS info PEIM to publish Bios Info
HOB. This PEIM currently publishes the microcode
FV info.

Cc: Ankit Sinha 
Cc: Nate DeSimone 
Cc: Kubacki Michael A 

Signed-off-by: Prince Agyeman 
---
 .../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/BiosInfo.c
 create mode 100644 
Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.inf

diff --git 
a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.c 
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.c
new file mode 100644
index 00..6a058a0fc2
--- /dev/null
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.c
@@ -0,0 +1,93 @@
+/** @file
+  Driver for BIOS Info support.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#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,
+  ,
+  
+};
+
+/**
+  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 ();
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Build hob, so that DXE module can also get the data.
+  //
+  HobData = BuildGuidHob (, sizeof (mBiosInfo));
+  ASSERT (HobData != NULL);
+  if (HobData == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
+  CopyMem (HobData, , sizeof (mBiosInfo));
+
+  return EFI_SUCCESS;
+}
diff --git 
a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.inf 
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.inf
new file mode 100644
index 00..f268de00f5
--- /dev/null
+++ 
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/BiosInfo/BiosInfo.inf
@@ -0,0 +1,48 @@
+### @file
+#  Module Information description file for BIOS Info Driver
+#
+#  Copyright (c) 2019, Intel Corporation. All rights reserved.
+#  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/OpenBoardPkg.dsc 
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc
index eea809140c..00a89f4507 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc
+++