[PATCH 0/3] Support EFI_PEI_CORE_FV_LOCATION_PPI

[PATCH 0/3] Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chasel, Chiu]

PI spec 1.7 section 6.3.9 has defined a PPI to support the scenario
that PEI Foundation not in BFV. EFI_PEI_CORE_FV_LOCATION_PPI reports
the FV which contains PEI Foundation and should be passed by SEC as
part of PPI list. Otherwise PEI Foundation shall assume that it
resides in BFV.

Patch1: Add EFI_PEI_CORE_FV_LOCATION_PPI definition.
Patch2: Support PeiCore not in BFV scenario when shadowing.
Patch3: SecCore to find PeiCore from either non-BFV or BFV.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>

Chasel, Chiu (3):
  MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI
  MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI
  UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58 +++++++++++++++++++++++++++++++++++++++++++++-------------
 UefiCpuPkg/SecCore/SecMain.c            | 35 +++++++++++++++++++++++++++++------
 MdeModulePkg/Core/Pei/PeiMain.h         |  3 ++-
 MdeModulePkg/Core/Pei/PeiMain.inf       |  3 ++-
 MdePkg/Include/Ppi/PeiCoreFvLocation.h  | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 MdePkg/MdePkg.dec                       | 11 +++++++++--
 UefiCpuPkg/SecCore/SecCore.inf          |  3 ++-
 UefiCpuPkg/SecCore/SecMain.h            |  3 ++-
 8 files changed, 144 insertions(+), 25 deletions(-)
 create mode 100644 MdePkg/Include/Ppi/PeiCoreFvLocation.h

-- 
2.13.3.windows.1

[PATCH 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chasel, Chiu]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524

Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on
PI spec 1.7, Section 6.3.9.
This PPI can support the secnario that PEI Foundation
not in BFV.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 MdePkg/Include/Ppi/PeiCoreFvLocation.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 MdePkg/MdePkg.dec                      | 11 +++++++++--
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
new file mode 100644
index 0000000000..c7bbbfb265
--- /dev/null
+++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
@@ -0,0 +1,53 @@
+/** @file
+  Header file for Pei Core FV Location PPI.
+
+  This PPI contains a pointer to the firmware volume which contains the PEI Foundation.
+  If the PEI Foundation does not reside in the BFV, then SEC must pass this PPI as a part
+  of the PPI list provided to the PEI Foundation Entry Point, otherwise the PEI Foundation
+  shall assume that it resides within the BFV.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+  @par Revision Reference:
+  This PPI is defined in UEFI Platform Initialization Specification 1.7 Volume 1:
+  Standards
+
+**/
+
+
+#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
+#define _EFI_PEI_CORE_FV_LOCATION_H_
+
+///
+/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI
+///
+#define EFI_PEI_CORE_FV_LOCATION_GUID \
+  { \
+    0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 } \
+  }
+
+///
+/// Forward declaration for EFI_PEI_CORE_FV_LOCATION_PPI
+///
+typedef struct _EFI_PEI_CORE_FV_LOCATION_PPI EFI_PEI_CORE_FV_LOCATION_PPI;
+
+///
+/// This PPI provides location of EFI PeiCoreFv.
+///
+struct _EFI_PEI_CORE_FV_LOCATION_PPI {
+  ///
+  /// Pointer to the first byte of the firmware volume which contains the PEI Foundation.
+  ///
+  VOID    *PeiCoreFvLocation;
+};
+
+extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
+
+#endif // _EFI_PEI_CORE_FV_LOCATION_H_
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index a485408310..c859b4a511 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2,9 +2,9 @@
 # This Package provides all definitions, library classes and libraries instances.
 #
 # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of
-# EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
+# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
 #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
 # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
@@ -929,6 +929,13 @@
   ## Include/Ppi/SecHobData.h
   gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5, 0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
 
+  #
+  # PPIs defined in PI 1.7.
+  #
+
+  ## Include/Ppi/PeiCoreFvLocation.h
+  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
+
 [Protocols]
   ## Include/Protocol/Pcd.h
   gPcdProtocolGuid               = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 0x90, 0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
-- 
2.13.3.windows.1

Re: [PATCH 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Zeng, Star]

On 2019/2/12 21:19, Chasel, Chiu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on
> PI spec 1.7, Section 6.3.9.
> This PPI can support the secnario that PEI Foundation
> not in BFV.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>

Reviewed-by: Star Zeng <star.zeng@intel.com>

> ---
>   MdePkg/Include/Ppi/PeiCoreFvLocation.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   MdePkg/MdePkg.dec                      | 11 +++++++++--
>   2 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> new file mode 100644
> index 0000000000..c7bbbfb265
> --- /dev/null
> +++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> @@ -0,0 +1,53 @@
> +/** @file
> +  Header file for Pei Core FV Location PPI.
> +
> +  This PPI contains a pointer to the firmware volume which contains the PEI Foundation.
> +  If the PEI Foundation does not reside in the BFV, then SEC must pass this PPI as a part
> +  of the PPI list provided to the PEI Foundation Entry Point, otherwise the PEI Foundation
> +  shall assume that it resides within the BFV.
> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +  @par Revision Reference:
> +  This PPI is defined in UEFI Platform Initialization Specification 1.7 Volume 1:
> +  Standards
> +
> +**/
> +
> +
> +#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
> +#define _EFI_PEI_CORE_FV_LOCATION_H_
> +
> +///
> +/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI
> +///
> +#define EFI_PEI_CORE_FV_LOCATION_GUID \
> +  { \
> +    0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 } \
> +  }
> +
> +///
> +/// Forward declaration for EFI_PEI_CORE_FV_LOCATION_PPI
> +///
> +typedef struct _EFI_PEI_CORE_FV_LOCATION_PPI EFI_PEI_CORE_FV_LOCATION_PPI;
> +
> +///
> +/// This PPI provides location of EFI PeiCoreFv.
> +///
> +struct _EFI_PEI_CORE_FV_LOCATION_PPI {
> +  ///
> +  /// Pointer to the first byte of the firmware volume which contains the PEI Foundation.
> +  ///
> +  VOID    *PeiCoreFvLocation;
> +};
> +
> +extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
> +
> +#endif // _EFI_PEI_CORE_FV_LOCATION_H_
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index a485408310..c859b4a511 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -2,9 +2,9 @@
>   # This Package provides all definitions, library classes and libraries instances.
>   #
>   # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of
> -# EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
> +# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
>   #
> -# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
>   # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>   # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>   #
> @@ -929,6 +929,13 @@
>     ## Include/Ppi/SecHobData.h
>     gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5, 0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
>   
> +  #
> +  # PPIs defined in PI 1.7.
> +  #
> +
> +  ## Include/Ppi/PeiCoreFvLocation.h
> +  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
> +
>   [Protocols]
>     ## Include/Protocol/Pcd.h
>     gPcdProtocolGuid               = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 0x90, 0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
> 

Re: [PATCH 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Ni, Ray]

Chasel,
I don't think the below forward definition is needed.
 typedef struct _EFI_PEI_CORE_FV_LOCATION_PPI EFI_PEI_CORE_FV_LOCATION_PPI;
It is needed when the PPI contains methods and the methods requires This pointer
as the first parameter.


> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Chasel,
> Chiu
> Sent: Tuesday, February 12, 2019 9:20 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [edk2] [PATCH 1/3] MdePkg: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on PI spec 1.7,
> Section 6.3.9.
> This PPI can support the secnario that PEI Foundation not in BFV.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  MdePkg/Include/Ppi/PeiCoreFvLocation.h | 53
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  MdePkg/MdePkg.dec                      | 11 +++++++++--
>  2 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> new file mode 100644
> index 0000000000..c7bbbfb265
> --- /dev/null
> +++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> @@ -0,0 +1,53 @@
> +/** @file
> +  Header file for Pei Core FV Location PPI.
> +
> +  This PPI contains a pointer to the firmware volume which contains the PEI
> Foundation.
> +  If the PEI Foundation does not reside in the BFV, then SEC must pass
> + this PPI as a part  of the PPI list provided to the PEI Foundation
> + Entry Point, otherwise the PEI Foundation  shall assume that it resides
> within the BFV.
> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>  This
> + program and the accompanying materials  are licensed and made
> + available under the terms and conditions of the BSD License  which
> + accompanies this distribution.  The full text of the license may be
> + found at  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +  @par Revision Reference:
> +  This PPI is defined in UEFI Platform Initialization Specification 1.7 Volume 1:
> +  Standards
> +
> +**/
> +
> +
> +#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
> +#define _EFI_PEI_CORE_FV_LOCATION_H_
> +
> +///
> +/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI /// #define
> +EFI_PEI_CORE_FV_LOCATION_GUID \
> +  { \
> +    0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0,
> +0xca, 0xf4 } \
> +  }
> +
> +///
> +/// Forward declaration for EFI_PEI_CORE_FV_LOCATION_PPI /// typedef
> +struct _EFI_PEI_CORE_FV_LOCATION_PPI
> EFI_PEI_CORE_FV_LOCATION_PPI;
> +
> +///
> +/// This PPI provides location of EFI PeiCoreFv.
> +///
> +struct _EFI_PEI_CORE_FV_LOCATION_PPI {
> +  ///
> +  /// Pointer to the first byte of the firmware volume which contains the PEI
> Foundation.
> +  ///
> +  VOID    *PeiCoreFvLocation;
> +};
> +
> +extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
> +
> +#endif // _EFI_PEI_CORE_FV_LOCATION_H_
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> a485408310..c859b4a511 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -2,9 +2,9 @@
>  # This Package provides all definitions, library classes and libraries instances.
>  #
>  # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of -#
> EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
> +# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
>  #
> -# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2007 - 2019, Intel Corporation. All rights
> +reserved.<BR>
>  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>  # (C)
> Copyright 2016 Hewlett Packard Enterprise Development LP<BR>  # @@ -
> 929,6 +929,13 @@
>    ## Include/Ppi/SecHobData.h
>    gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5,
> 0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
> 
> +  #
> +  # PPIs defined in PI 1.7.
> +  #
> +
> +  ## Include/Ppi/PeiCoreFvLocation.h
> +  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8,
> 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
> +
>  [Protocols]
>    ## Include/Protocol/Pcd.h
>    gPcdProtocolGuid               = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 0x90,
> 0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
> --
> 2.13.3.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

[PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chasel, Chiu]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524

When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI
should be checked to see if PeiCore not in BFV, otherwise
just shadowing PeiCore from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58 +++++++++++++++++++++++++++++++++++++++++++++-------------
 MdeModulePkg/Core/Pei/PeiMain.h         |  3 ++-
 MdeModulePkg/Core/Pei/PeiMain.inf       |  3 ++-
 3 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index 4da80a8222..408f24c216 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -1,7 +1,7 @@
 /** @file
   Pei Core Main Entry Point
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -80,23 +80,55 @@ ShadowPeiCore (
   IN PEI_CORE_INSTANCE  *PrivateData
   )
 {
-  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
-  EFI_PHYSICAL_ADDRESS EntryPoint;
-  EFI_STATUS           Status;
-  UINT32               AuthenticationState;
+  EFI_PEI_FILE_HANDLE          PeiCoreFileHandle;
+  EFI_PHYSICAL_ADDRESS         EntryPoint;
+  EFI_STATUS                   Status;
+  UINT32                       AuthenticationState;
+  UINTN                        Index;
+  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
 
   PeiCoreFileHandle = NULL;
 
   //
-  // Find the PEI Core in the BFV
+  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI indicated FV or BFV
   //
-  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
-                                       PrivateData->Fv[0].FvPpi,
-                                       EFI_FV_FILETYPE_PEI_CORE,
-                                       PrivateData->Fv[0].FvHandle,
-                                       &PeiCoreFileHandle
-                                       );
-  ASSERT_EFI_ERROR (Status);
+  Status = PeiServicesLocatePpi (
+             &gEfiPeiCoreFvLocationPpiGuid,
+             0,
+             NULL,
+             (VOID **) &PeiCoreFvLocationPpi
+             );
+  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation != NULL)) {
+    //
+    // If PeiCoreFvLocation present, the PEI Core should be found from indicated FV.
+    //
+    for (Index = 0; Index < PrivateData->FvCount; Index ++) {
+      if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32) PeiCoreFvLocationPpi->PeiCoreFvLocation) {
+        continue;
+      }
+      Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
+                                               PrivateData->Fv[Index].FvPpi,
+                                               EFI_FV_FILETYPE_PEI_CORE,
+                                               PrivateData->Fv[Index].FvHandle,
+                                               &PeiCoreFileHandle
+                                               );
+      if (!EFI_ERROR (Status)) {
+        break;
+      }
+    }
+    ASSERT (Index < PrivateData->FvCount);
+  } else {
+    //
+    // Find PEI Core from BFV
+    //
+    Status = PrivateData->Fv[0].FvPpi->FindFileByType (
+                                         PrivateData->Fv[0].FvPpi,
+                                         EFI_FV_FILETYPE_PEI_CORE,
+                                         PrivateData->Fv[0].FvHandle,
+                                         &PeiCoreFileHandle
+                                         );
+    ASSERT_EFI_ERROR (Status);
+  }
 
   //
   // Shadow PEI Core into memory so it will run faster
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
index 322e7cd845..38542ab072 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -1,7 +1,7 @@
 /** @file
   Definition of Pei Core Structures and Services
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -49,6 +49,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Guid/FirmwareFileSystem2.h>
 #include <Guid/FirmwareFileSystem3.h>
 #include <Guid/AprioriFileName.h>
+#include <Ppi/PeiCoreFvLocation.h>
 
 ///
 /// It is an FFS type extension used for PeiFindFileEx. It indicates current
diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf
index 140c4444b1..5bab2aab8c 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.inf
+++ b/MdeModulePkg/Core/Pei/PeiMain.inf
@@ -6,7 +6,7 @@
 # 2) Dispatch PEIM from discovered FV.
 # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
 #
-# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -102,6 +102,7 @@
   gEfiTemporaryRamDonePpiGuid                   ## SOMETIMES_CONSUMES
   gEfiPeiReset2PpiGuid                          ## SOMETIMES_CONSUMES
   gEfiSecHobDataPpiGuid                         ## SOMETIMES_CONSUMES
+  gEfiPeiCoreFvLocationPpiGuid                  ## SOMETIMES_CONSUMES
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize                  ## CONSUMES
-- 
2.13.3.windows.1

Re: [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Wang, Jian J]

Chasel,


> -----Original Message-----
> From: Chiu, Chasel
> Sent: Tuesday, February 12, 2019 9:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
> Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI
> should be checked to see if PeiCore not in BFV, otherwise
> just shadowing PeiCore from BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++-------------
>  MdeModulePkg/Core/Pei/PeiMain.h         |  3 ++-
>  MdeModulePkg/Core/Pei/PeiMain.inf       |  3 ++-
>  3 files changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 4da80a8222..408f24c216 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Pei Core Main Entry Point
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -80,23 +80,55 @@ ShadowPeiCore (
>    IN PEI_CORE_INSTANCE  *PrivateData
>    )
>  {
> -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> -  EFI_PHYSICAL_ADDRESS EntryPoint;
> -  EFI_STATUS           Status;
> -  UINT32               AuthenticationState;
> +  EFI_PEI_FILE_HANDLE          PeiCoreFileHandle;
> +  EFI_PHYSICAL_ADDRESS         EntryPoint;
> +  EFI_STATUS                   Status;
> +  UINT32                       AuthenticationState;
> +  UINTN                        Index;
> +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> 
>    PeiCoreFileHandle = NULL;
> 
>    //
> -  // Find the PEI Core in the BFV
> +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI indicated
> FV or BFV
>    //
> -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> -                                       PrivateData->Fv[0].FvPpi,
> -                                       EFI_FV_FILETYPE_PEI_CORE,
> -                                       PrivateData->Fv[0].FvHandle,
> -                                       &PeiCoreFileHandle
> -                                       );
> -  ASSERT_EFI_ERROR (Status);
> +  Status = PeiServicesLocatePpi (
> +             &gEfiPeiCoreFvLocationPpiGuid,
> +             0,
> +             NULL,
> +             (VOID **) &PeiCoreFvLocationPpi
> +             );
> +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation !=
> NULL)) {
> +    //
> +    // If PeiCoreFvLocation present, the PEI Core should be found from indicated
> FV.
> +    //
> +    for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> +      if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> PeiCoreFvLocationPpi->PeiCoreFvLocation) {

I think the UINT32 type cast is not necessary. FvHandle and PeiCoreFvLocation are
actually type of VOID*. Do you encounter any compiler error here?

Regards,
Jian

> +        continue;
> +      }
> +      Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> +                                               PrivateData->Fv[Index].FvPpi,
> +                                               EFI_FV_FILETYPE_PEI_CORE,
> +                                               PrivateData->Fv[Index].FvHandle,
> +                                               &PeiCoreFileHandle
> +                                               );
> +      if (!EFI_ERROR (Status)) {
> +        break;
> +      }
> +    }
> +    ASSERT (Index < PrivateData->FvCount);
> +  } else {
> +    //
> +    // Find PEI Core from BFV
> +    //
> +    Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> +                                         PrivateData->Fv[0].FvPpi,
> +                                         EFI_FV_FILETYPE_PEI_CORE,
> +                                         PrivateData->Fv[0].FvHandle,
> +                                         &PeiCoreFileHandle
> +                                         );
> +    ASSERT_EFI_ERROR (Status);
> +  }
> 
>    //
>    // Shadow PEI Core into memory so it will run faster
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h
> index 322e7cd845..38542ab072 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Definition of Pei Core Structures and Services
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -49,6 +49,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Guid/FirmwareFileSystem2.h>
>  #include <Guid/FirmwareFileSystem3.h>
>  #include <Guid/AprioriFileName.h>
> +#include <Ppi/PeiCoreFvLocation.h>
> 
>  ///
>  /// It is an FFS type extension used for PeiFindFileEx. It indicates current
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> b/MdeModulePkg/Core/Pei/PeiMain.inf
> index 140c4444b1..5bab2aab8c 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> @@ -6,7 +6,7 @@
>  # 2) Dispatch PEIM from discovered FV.
>  # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
>  #
> -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD
> License
> @@ -102,6 +102,7 @@
>    gEfiTemporaryRamDonePpiGuid                   ## SOMETIMES_CONSUMES
>    gEfiPeiReset2PpiGuid                          ## SOMETIMES_CONSUMES
>    gEfiSecHobDataPpiGuid                         ## SOMETIMES_CONSUMES
> +  gEfiPeiCoreFvLocationPpiGuid                  ## SOMETIMES_CONSUMES
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize                  ##
> CONSUMES
> --
> 2.13.3.windows.1

Re: [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chiu, Chasel]

No issue, I will remove UINT32 casting. Thanks!

> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, February 13, 2019 9:14 AM
> To: Chiu, Chasel <chasel.chiu@intel.com>; edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> Chasel,
> 
> 
> > -----Original Message-----
> > From: Chiu, Chasel
> > Sent: Tuesday, February 12, 2019 9:20 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Chiu,
> > Chasel <chasel.chiu@intel.com>
> > Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> > EFI_PEI_CORE_FV_LOCATION_PPI
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> >
> > When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be
> > checked to see if PeiCore not in BFV, otherwise just shadowing PeiCore
> > from BFV.
> >
> > Test: Verified on internal platform and booting successfully.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > ---
> >  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> > +++++++++++++++++++++++++++++++++++++++++++++-------------
> >  MdeModulePkg/Core/Pei/PeiMain.h         |  3 ++-
> >  MdeModulePkg/Core/Pei/PeiMain.inf       |  3 ++-
> >  3 files changed, 49 insertions(+), 15 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > index 4da80a8222..408f24c216 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Pei Core Main Entry Point
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > +reserved.<BR>
> >  This program and the accompanying materials  are licensed and made
> > available under the terms and conditions of the BSD License  which
> > accompanies this distribution.  The full text of the license may be
> > found at @@ -80,23 +80,55 @@ ShadowPeiCore (
> >    IN PEI_CORE_INSTANCE  *PrivateData
> >    )
> >  {
> > -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> > -  EFI_PHYSICAL_ADDRESS EntryPoint;
> > -  EFI_STATUS           Status;
> > -  UINT32               AuthenticationState;
> > +  EFI_PEI_FILE_HANDLE          PeiCoreFileHandle;
> > +  EFI_PHYSICAL_ADDRESS         EntryPoint;
> > +  EFI_STATUS                   Status;
> > +  UINT32                       AuthenticationState;
> > +  UINTN                        Index;
> > +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> >
> >    PeiCoreFileHandle = NULL;
> >
> >    //
> > -  // Find the PEI Core in the BFV
> > +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI
> > + indicated
> > FV or BFV
> >    //
> > -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > -                                       PrivateData->Fv[0].FvPpi,
> > -                                       EFI_FV_FILETYPE_PEI_CORE,
> > -                                       PrivateData->Fv[0].FvHandle,
> > -                                       &PeiCoreFileHandle
> > -                                       );
> > -  ASSERT_EFI_ERROR (Status);
> > +  Status = PeiServicesLocatePpi (
> > +             &gEfiPeiCoreFvLocationPpiGuid,
> > +             0,
> > +             NULL,
> > +             (VOID **) &PeiCoreFvLocationPpi
> > +             );
> > +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation
> > + !=
> > NULL)) {
> > +    //
> > +    // If PeiCoreFvLocation present, the PEI Core should be found
> > + from indicated
> > FV.
> > +    //
> > +    for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> > +      if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> > PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> 
> I think the UINT32 type cast is not necessary. FvHandle and PeiCoreFvLocation
> are actually type of VOID*. Do you encounter any compiler error here?
> 
> Regards,
> Jian
> 
> > +        continue;
> > +      }
> > +      Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> > +                                               PrivateData->Fv[Index].FvPpi,
> > +                                               EFI_FV_FILETYPE_PEI_CORE,
> > +                                               PrivateData->Fv[Index].FvHandle,
> > +                                               &PeiCoreFileHandle
> > +                                               );
> > +      if (!EFI_ERROR (Status)) {
> > +        break;
> > +      }
> > +    }
> > +    ASSERT (Index < PrivateData->FvCount);  } else {
> > +    //
> > +    // Find PEI Core from BFV
> > +    //
> > +    Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > +                                         PrivateData->Fv[0].FvPpi,
> > +                                         EFI_FV_FILETYPE_PEI_CORE,
> > +                                         PrivateData->Fv[0].FvHandle,
> > +                                         &PeiCoreFileHandle
> > +                                         );
> > +    ASSERT_EFI_ERROR (Status);
> > +  }
> >
> >    //
> >    // Shadow PEI Core into memory so it will run faster diff --git
> > a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h
> > index 322e7cd845..38542ab072 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain.h
> > +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Definition of Pei Core Structures and Services
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > +reserved.<BR>
> >  This program and the accompanying materials  are licensed and made
> > available under the terms and conditions of the BSD License  which
> > accompanies this distribution.  The full text of the license may be
> > found at @@ -49,6 +49,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS
> OF
> > ANY KIND, EITHER EXPRESS OR IMPLIED.
> >  #include <Guid/FirmwareFileSystem2.h>  #include
> > <Guid/FirmwareFileSystem3.h>  #include <Guid/AprioriFileName.h>
> > +#include <Ppi/PeiCoreFvLocation.h>
> >
> >  ///
> >  /// It is an FFS type extension used for PeiFindFileEx. It indicates
> > current diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> > b/MdeModulePkg/Core/Pei/PeiMain.inf
> > index 140c4444b1..5bab2aab8c 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> > +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> > @@ -6,7 +6,7 @@
> >  # 2) Dispatch PEIM from discovered FV.
> >  # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
> >  #
> > -# Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > +# Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > +reserved.<BR>
> >  #
> >  #  This program and the accompanying materials  #  are licensed and
> > made available under the terms and conditions of the BSD License @@
> > -102,6 +102,7 @@
> >    gEfiTemporaryRamDonePpiGuid                   ##
> SOMETIMES_CONSUMES
> >    gEfiPeiReset2PpiGuid                          ## SOMETIMES_CONSUMES
> >    gEfiSecHobDataPpiGuid                         ## SOMETIMES_CONSUMES
> > +  gEfiPeiCoreFvLocationPpiGuid                  ## SOMETIMES_CONSUMES
> >
> >  [Pcd]
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize
> ##
> > CONSUMES
> > --
> > 2.13.3.windows.1

Re: [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Zeng, Star]

With this addressed, Reviewed-by: Star Zeng <star.zeng@intel.com>.

-----Original Message-----
From: Chiu, Chasel 
Sent: Wednesday, February 13, 2019 11:59 AM
To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI


No issue, I will remove UINT32 casting. Thanks!

> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, February 13, 2019 9:14 AM
> To: Chiu, Chasel <chasel.chiu@intel.com>; edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, 
> Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH 2/3] MdeModulePkg/PeiMain: Support 
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> Chasel,
> 
> 
> > -----Original Message-----
> > From: Chiu, Chasel
> > Sent: Tuesday, February 12, 2019 9:20 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A 
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star 
> > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Chiu, 
> > Chasel <chasel.chiu@intel.com>
> > Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support 
> > EFI_PEI_CORE_FV_LOCATION_PPI
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> >
> > When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be 
> > checked to see if PeiCore not in BFV, otherwise just shadowing 
> > PeiCore from BFV.
> >
> > Test: Verified on internal platform and booting successfully.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > ---
> >  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> > +++++++++++++++++++++++++++++++++++++++++++++-------------
> >  MdeModulePkg/Core/Pei/PeiMain.h         |  3 ++-
> >  MdeModulePkg/Core/Pei/PeiMain.inf       |  3 ++-
> >  3 files changed, 49 insertions(+), 15 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > index 4da80a8222..408f24c216 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Pei Core Main Entry Point
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights 
> > reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights 
> > +reserved.<BR>
> >  This program and the accompanying materials  are licensed and made 
> > available under the terms and conditions of the BSD License  which 
> > accompanies this distribution.  The full text of the license may be 
> > found at @@ -80,23 +80,55 @@ ShadowPeiCore (
> >    IN PEI_CORE_INSTANCE  *PrivateData
> >    )
> >  {
> > -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> > -  EFI_PHYSICAL_ADDRESS EntryPoint;
> > -  EFI_STATUS           Status;
> > -  UINT32               AuthenticationState;
> > +  EFI_PEI_FILE_HANDLE          PeiCoreFileHandle;
> > +  EFI_PHYSICAL_ADDRESS         EntryPoint;
> > +  EFI_STATUS                   Status;
> > +  UINT32                       AuthenticationState;
> > +  UINTN                        Index;
> > +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> >
> >    PeiCoreFileHandle = NULL;
> >
> >    //
> > -  // Find the PEI Core in the BFV
> > +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI 
> > + indicated
> > FV or BFV
> >    //
> > -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > -                                       PrivateData->Fv[0].FvPpi,
> > -                                       EFI_FV_FILETYPE_PEI_CORE,
> > -                                       PrivateData->Fv[0].FvHandle,
> > -                                       &PeiCoreFileHandle
> > -                                       );
> > -  ASSERT_EFI_ERROR (Status);
> > +  Status = PeiServicesLocatePpi (
> > +             &gEfiPeiCoreFvLocationPpiGuid,
> > +             0,
> > +             NULL,
> > +             (VOID **) &PeiCoreFvLocationPpi
> > +             );
> > +  if (!EFI_ERROR (Status) && 
> > + (PeiCoreFvLocationPpi->PeiCoreFvLocation
> > + !=
> > NULL)) {
> > +    //
> > +    // If PeiCoreFvLocation present, the PEI Core should be found 
> > + from indicated
> > FV.
> > +    //
> > +    for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> > +      if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> > PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> 
> I think the UINT32 type cast is not necessary. FvHandle and 
> PeiCoreFvLocation are actually type of VOID*. Do you encounter any compiler error here?
> 
> Regards,
> Jian
> 
> > +        continue;
> > +      }
> > +      Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> > +                                               PrivateData->Fv[Index].FvPpi,
> > +                                               EFI_FV_FILETYPE_PEI_CORE,
> > +                                               PrivateData->Fv[Index].FvHandle,
> > +                                               &PeiCoreFileHandle
> > +                                               );
> > +      if (!EFI_ERROR (Status)) {
> > +        break;
> > +      }
> > +    }
> > +    ASSERT (Index < PrivateData->FvCount);  } else {
> > +    //
> > +    // Find PEI Core from BFV
> > +    //
> > +    Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > +                                         PrivateData->Fv[0].FvPpi,
> > +                                         EFI_FV_FILETYPE_PEI_CORE,
> > +                                         PrivateData->Fv[0].FvHandle,
> > +                                         &PeiCoreFileHandle
> > +                                         );
> > +    ASSERT_EFI_ERROR (Status);
> > +  }
> >
> >    //
> >    // Shadow PEI Core into memory so it will run faster diff --git 
> > a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h
> > index 322e7cd845..38542ab072 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain.h
> > +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Definition of Pei Core Structures and Services
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights 
> > reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights 
> > +reserved.<BR>
> >  This program and the accompanying materials  are licensed and made 
> > available under the terms and conditions of the BSD License  which 
> > accompanies this distribution.  The full text of the license may be 
> > found at @@ -49,6 +49,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS
> OF
> > ANY KIND, EITHER EXPRESS OR IMPLIED.
> >  #include <Guid/FirmwareFileSystem2.h>  #include 
> > <Guid/FirmwareFileSystem3.h>  #include <Guid/AprioriFileName.h>
> > +#include <Ppi/PeiCoreFvLocation.h>
> >
> >  ///
> >  /// It is an FFS type extension used for PeiFindFileEx. It 
> > indicates current diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> > b/MdeModulePkg/Core/Pei/PeiMain.inf
> > index 140c4444b1..5bab2aab8c 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> > +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> > @@ -6,7 +6,7 @@
> >  # 2) Dispatch PEIM from discovered FV.
> >  # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
> >  #
> > -# Copyright (c) 2006 - 2018, Intel Corporation. All rights 
> > reserved.<BR>
> > +# Copyright (c) 2006 - 2019, Intel Corporation. All rights 
> > +reserved.<BR>
> >  #
> >  #  This program and the accompanying materials  #  are licensed and 
> > made available under the terms and conditions of the BSD License @@
> > -102,6 +102,7 @@
> >    gEfiTemporaryRamDonePpiGuid                   ##
> SOMETIMES_CONSUMES
> >    gEfiPeiReset2PpiGuid                          ## SOMETIMES_CONSUMES
> >    gEfiSecHobDataPpiGuid                         ## SOMETIMES_CONSUMES
> > +  gEfiPeiCoreFvLocationPpiGuid                  ## SOMETIMES_CONSUMES
> >
> >  [Pcd]
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize
> ##
> > CONSUMES
> > --
> > 2.13.3.windows.1

Re: [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Ni, Ray]

1. please move the #include adjacent to other #include<Ppi/*>
2. Can you explain why 

> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Tuesday, February 12, 2019 9:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
> Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be
> checked to see if PeiCore not in BFV, otherwise just shadowing PeiCore from
> BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++-------------
>  MdeModulePkg/Core/Pei/PeiMain.h         |  3 ++-
>  MdeModulePkg/Core/Pei/PeiMain.inf       |  3 ++-
>  3 files changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 4da80a8222..408f24c216 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Pei Core Main Entry Point
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials  are licensed and made
> available under the terms and conditions of the BSD License  which
> accompanies this distribution.  The full text of the license may be found at
> @@ -80,23 +80,55 @@ ShadowPeiCore (
>    IN PEI_CORE_INSTANCE  *PrivateData
>    )
>  {
> -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> -  EFI_PHYSICAL_ADDRESS EntryPoint;
> -  EFI_STATUS           Status;
> -  UINT32               AuthenticationState;
> +  EFI_PEI_FILE_HANDLE          PeiCoreFileHandle;
> +  EFI_PHYSICAL_ADDRESS         EntryPoint;
> +  EFI_STATUS                   Status;
> +  UINT32                       AuthenticationState;
> +  UINTN                        Index;
> +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> 
>    PeiCoreFileHandle = NULL;
> 
>    //
> -  // Find the PEI Core in the BFV
> +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI
> + indicated FV or BFV
>    //
> -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> -                                       PrivateData->Fv[0].FvPpi,
> -                                       EFI_FV_FILETYPE_PEI_CORE,
> -                                       PrivateData->Fv[0].FvHandle,
> -                                       &PeiCoreFileHandle
> -                                       );
> -  ASSERT_EFI_ERROR (Status);
> +  Status = PeiServicesLocatePpi (
> +             &gEfiPeiCoreFvLocationPpiGuid,
> +             0,
> +             NULL,
> +             (VOID **) &PeiCoreFvLocationPpi
> +             );
> +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation !=
> NULL)) {
> +    //
> +    // If PeiCoreFvLocation present, the PEI Core should be found from
> indicated FV.
> +    //
> +    for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> +      if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> +        continue;
> +      }
> +      Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> +                                               PrivateData->Fv[Index].FvPpi,
> +                                               EFI_FV_FILETYPE_PEI_CORE,
> +                                               PrivateData->Fv[Index].FvHandle,
> +                                               &PeiCoreFileHandle
> +                                               );
> +      if (!EFI_ERROR (Status)) {
> +        break;

1. Is it valid that PEI_CORE cannot be found in the PeiCoreFvLocation?
If no, why not assert here?

> +      }
> +    }
> +    ASSERT (Index < PrivateData->FvCount);  } else {
> +    //
> +    // Find PEI Core from BFV
> +    //
> +    Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> +                                         PrivateData->Fv[0].FvPpi,
> +                                         EFI_FV_FILETYPE_PEI_CORE,
> +                                         PrivateData->Fv[0].FvHandle,
> +                                         &PeiCoreFileHandle
> +                                         );
> +    ASSERT_EFI_ERROR (Status);
> +  }
> 
>    //
>    // Shadow PEI Core into memory so it will run faster diff --git
> a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h index 322e7cd845..38542ab072
> 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Definition of Pei Core Structures and Services
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials  are licensed and made
> available under the terms and conditions of the BSD License  which
> accompanies this distribution.  The full text of the license may be found at
> @@ -49,6 +49,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Guid/FirmwareFileSystem2.h>
>  #include <Guid/FirmwareFileSystem3.h>
>  #include <Guid/AprioriFileName.h>
> +#include <Ppi/PeiCoreFvLocation.h>

2. please move the #include adjacent to other #include<Ppi/*>

> 
>  ///
>  /// It is an FFS type extension used for PeiFindFileEx. It indicates current diff
> --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> b/MdeModulePkg/Core/Pei/PeiMain.inf
> index 140c4444b1..5bab2aab8c 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> @@ -6,7 +6,7 @@
>  # 2) Dispatch PEIM from discovered FV.
>  # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
>  #
> -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2019, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  This program and the accompanying materials  #  are licensed and made
> available under the terms and conditions of the BSD License @@ -102,6
> +102,7 @@
>    gEfiTemporaryRamDonePpiGuid                   ## SOMETIMES_CONSUMES
>    gEfiPeiReset2PpiGuid                          ## SOMETIMES_CONSUMES
>    gEfiSecHobDataPpiGuid                         ## SOMETIMES_CONSUMES
> +  gEfiPeiCoreFvLocationPpiGuid                  ## SOMETIMES_CONSUMES
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize
> ## CONSUMES
> --
> 2.13.3.windows.1

Re: [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Ni, Ray]

Sorry, please ignore my other review mail and just check this mail.
3 minor comments below.

> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Tuesday, February 12, 2019 9:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
> Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be
> checked to see if PeiCore not in BFV, otherwise just shadowing PeiCore from
> BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++-------------
>  MdeModulePkg/Core/Pei/PeiMain.h         |  3 ++-
>  MdeModulePkg/Core/Pei/PeiMain.inf       |  3 ++-
>  3 files changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 4da80a8222..408f24c216 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Pei Core Main Entry Point
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials  are licensed and made
> available under the terms and conditions of the BSD License  which
> accompanies this distribution.  The full text of the license may be found at
> @@ -80,23 +80,55 @@ ShadowPeiCore (
>    IN PEI_CORE_INSTANCE  *PrivateData
>    )
>  {
> -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> -  EFI_PHYSICAL_ADDRESS EntryPoint;
> -  EFI_STATUS           Status;
> -  UINT32               AuthenticationState;
> +  EFI_PEI_FILE_HANDLE          PeiCoreFileHandle;
> +  EFI_PHYSICAL_ADDRESS         EntryPoint;
> +  EFI_STATUS                   Status;
> +  UINT32                       AuthenticationState;
> +  UINTN                        Index;
> +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> 
>    PeiCoreFileHandle = NULL;
> 
>    //
> -  // Find the PEI Core in the BFV
> +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI
> + indicated FV or BFV
>    //
> -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> -                                       PrivateData->Fv[0].FvPpi,
> -                                       EFI_FV_FILETYPE_PEI_CORE,
> -                                       PrivateData->Fv[0].FvHandle,
> -                                       &PeiCoreFileHandle
> -                                       );
> -  ASSERT_EFI_ERROR (Status);
> +  Status = PeiServicesLocatePpi (
> +             &gEfiPeiCoreFvLocationPpiGuid,
> +             0,
> +             NULL,
> +             (VOID **) &PeiCoreFvLocationPpi
> +             );
> +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation !=
> NULL)) {
> +    //
> +    // If PeiCoreFvLocation present, the PEI Core should be found from
> indicated FV.
> +    //
> +    for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> +      if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> +        continue;
> +      }
> +      Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> +                                               PrivateData->Fv[Index].FvPpi,
> +                                               EFI_FV_FILETYPE_PEI_CORE,
> +                                               PrivateData->Fv[Index].FvHandle,
> +                                               &PeiCoreFileHandle
> +                                               );
> +      if (!EFI_ERROR (Status)) {
> +        break;
1. Is it valid that PEI_CORE cannot be found in the PeiCoreFvLocation?
If no, why not assert here?

> +      }
> +    }
> +    ASSERT (Index < PrivateData->FvCount);  } else {
> +    //
> +    // Find PEI Core from BFV
> +    //
> +    Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> +                                         PrivateData->Fv[0].FvPpi,
> +                                         EFI_FV_FILETYPE_PEI_CORE,
> +                                         PrivateData->Fv[0].FvHandle,
> +                                         &PeiCoreFileHandle
> +                                         );
> +    ASSERT_EFI_ERROR (Status);

2. Seems to have code duplication here. Can we have a local PeiCoreFvIndex to be 0 as default?
And when the gEfiPeiCoreFvLocationPpiGuid PPI exists, PeiCoreFvIndex is updated to the correct value.
So that FindFileByType() is only used once in source code with first parameter
PrivateData->Fv[PeiCoreFvIndex].FvPpi.

> +  }
> 
>    //
>    // Shadow PEI Core into memory so it will run faster diff --git
> a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h index 322e7cd845..38542ab072
> 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Definition of Pei Core Structures and Services
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials  are licensed and made
> available under the terms and conditions of the BSD License  which
> accompanies this distribution.  The full text of the license may be found at
> @@ -49,6 +49,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Guid/FirmwareFileSystem2.h>
>  #include <Guid/FirmwareFileSystem3.h>
>  #include <Guid/AprioriFileName.h>
> +#include <Ppi/PeiCoreFvLocation.h>

3. please move the #include adjacent to other #include<Ppi/*>

> 
>  ///
>  /// It is an FFS type extension used for PeiFindFileEx. It indicates current diff
> --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> b/MdeModulePkg/Core/Pei/PeiMain.inf
> index 140c4444b1..5bab2aab8c 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> @@ -6,7 +6,7 @@
>  # 2) Dispatch PEIM from discovered FV.
>  # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
>  #
> -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2019, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  This program and the accompanying materials  #  are licensed and made
> available under the terms and conditions of the BSD License @@ -102,6
> +102,7 @@
>    gEfiTemporaryRamDonePpiGuid                   ## SOMETIMES_CONSUMES
>    gEfiPeiReset2PpiGuid                          ## SOMETIMES_CONSUMES
>    gEfiSecHobDataPpiGuid                         ## SOMETIMES_CONSUMES
> +  gEfiPeiCoreFvLocationPpiGuid                  ## SOMETIMES_CONSUMES
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize
> ## CONSUMES
> --
> 2.13.3.windows.1

Re: [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chiu, Chasel]

> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, February 14, 2019 4:20 PM
> To: Chiu, Chasel <chasel.chiu@intel.com>; edk2-devel@lists.01.org
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> Sorry, please ignore my other review mail and just check this mail.
> 3 minor comments below.
> 
> > -----Original Message-----
> > From: Chiu, Chasel <chasel.chiu@intel.com>
> > Sent: Tuesday, February 12, 2019 9:20 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Chiu,
> > Chasel <chasel.chiu@intel.com>
> > Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> > EFI_PEI_CORE_FV_LOCATION_PPI
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> >
> > When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be
> > checked to see if PeiCore not in BFV, otherwise just shadowing PeiCore
> > from BFV.
> >
> > Test: Verified on internal platform and booting successfully.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > ---
> >  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> > +++++++++++++++++++++++++++++++++++++++++++++-------------
> >  MdeModulePkg/Core/Pei/PeiMain.h         |  3 ++-
> >  MdeModulePkg/Core/Pei/PeiMain.inf       |  3 ++-
> >  3 files changed, 49 insertions(+), 15 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > index 4da80a8222..408f24c216 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Pei Core Main Entry Point
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > +reserved.<BR>
> >  This program and the accompanying materials  are licensed and made
> > available under the terms and conditions of the BSD License  which
> > accompanies this distribution.  The full text of the license may be
> > found at @@ -80,23 +80,55 @@ ShadowPeiCore (
> >    IN PEI_CORE_INSTANCE  *PrivateData
> >    )
> >  {
> > -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> > -  EFI_PHYSICAL_ADDRESS EntryPoint;
> > -  EFI_STATUS           Status;
> > -  UINT32               AuthenticationState;
> > +  EFI_PEI_FILE_HANDLE          PeiCoreFileHandle;
> > +  EFI_PHYSICAL_ADDRESS         EntryPoint;
> > +  EFI_STATUS                   Status;
> > +  UINT32                       AuthenticationState;
> > +  UINTN                        Index;
> > +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> >
> >    PeiCoreFileHandle = NULL;
> >
> >    //
> > -  // Find the PEI Core in the BFV
> > +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI
> > + indicated FV or BFV
> >    //
> > -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > -                                       PrivateData->Fv[0].FvPpi,
> > -                                       EFI_FV_FILETYPE_PEI_CORE,
> > -                                       PrivateData->Fv[0].FvHandle,
> > -                                       &PeiCoreFileHandle
> > -                                       );
> > -  ASSERT_EFI_ERROR (Status);
> > +  Status = PeiServicesLocatePpi (
> > +             &gEfiPeiCoreFvLocationPpiGuid,
> > +             0,
> > +             NULL,
> > +             (VOID **) &PeiCoreFvLocationPpi
> > +             );
> > +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation
> > + !=
> > NULL)) {
> > +    //
> > +    // If PeiCoreFvLocation present, the PEI Core should be found
> > + from
> > indicated FV.
> > +    //
> > +    for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> > +      if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> > PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> > +        continue;
> > +      }
> > +      Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> > +                                               PrivateData->Fv[Index].FvPpi,
> > +                                               EFI_FV_FILETYPE_PEI_CORE,
> > +                                               PrivateData->Fv[Index].FvHandle,
> > +                                               &PeiCoreFileHandle
> > +                                               );
> > +      if (!EFI_ERROR (Status)) {
> > +        break;
> 1. Is it valid that PEI_CORE cannot be found in the PeiCoreFvLocation?
> If no, why not assert here?
Agree. I will update the code.

> 
> > +      }
> > +    }
> > +    ASSERT (Index < PrivateData->FvCount);  } else {
> > +    //
> > +    // Find PEI Core from BFV
> > +    //
> > +    Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > +                                         PrivateData->Fv[0].FvPpi,
> > +                                         EFI_FV_FILETYPE_PEI_CORE,
> > +                                         PrivateData->Fv[0].FvHandle,
> > +                                         &PeiCoreFileHandle
> > +                                         );
> > +    ASSERT_EFI_ERROR (Status);
> 
> 2. Seems to have code duplication here. Can we have a local PeiCoreFvIndex to
> be 0 as default?
> And when the gEfiPeiCoreFvLocationPpiGuid PPI exists, PeiCoreFvIndex is
> updated to the correct value.
> So that FindFileByType() is only used once in source code with first parameter
> PrivateData->Fv[PeiCoreFvIndex].FvPpi.
Good suggestion! I will optimize this code.


> 
> > +  }
> >
> >    //
> >    // Shadow PEI Core into memory so it will run faster diff --git
> > a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h
> > index 322e7cd845..38542ab072
> > 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain.h
> > +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Definition of Pei Core Structures and Services
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > +reserved.<BR>
> >  This program and the accompanying materials  are licensed and made
> > available under the terms and conditions of the BSD License  which
> > accompanies this distribution.  The full text of the license may be
> > found at @@ -49,6 +49,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS
> OF
> > ANY KIND, EITHER EXPRESS OR IMPLIED.
> >  #include <Guid/FirmwareFileSystem2.h>  #include
> > <Guid/FirmwareFileSystem3.h>  #include <Guid/AprioriFileName.h>
> > +#include <Ppi/PeiCoreFvLocation.h>
> 
> 3. please move the #include adjacent to other #include<Ppi/*>
Yes.

> 
> >
> >  ///
> >  /// It is an FFS type extension used for PeiFindFileEx. It indicates
> > current diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> > b/MdeModulePkg/Core/Pei/PeiMain.inf
> > index 140c4444b1..5bab2aab8c 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> > +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> > @@ -6,7 +6,7 @@
> >  # 2) Dispatch PEIM from discovered FV.
> >  # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
> >  #
> > -# Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > +# Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > +reserved.<BR>
> >  #
> >  #  This program and the accompanying materials  #  are licensed and
> > made available under the terms and conditions of the BSD License @@
> > -102,6
> > +102,7 @@
> >    gEfiTemporaryRamDonePpiGuid                   ##
> SOMETIMES_CONSUMES
> >    gEfiPeiReset2PpiGuid                          ## SOMETIMES_CONSUMES
> >    gEfiSecHobDataPpiGuid                         ## SOMETIMES_CONSUMES
> > +  gEfiPeiCoreFvLocationPpiGuid                  ## SOMETIMES_CONSUMES
> >
> >  [Pcd]
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize
> > ## CONSUMES
> > --
> > 2.13.3.windows.1

[PATCH 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Chasel, Chiu]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524

EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform
when PeiCore not in BFV so SecCore has to search PeiCore
either from the FV location provided by
EFI_PEI_CORE_FV_LOCATION_PPI or from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 UefiCpuPkg/SecCore/SecMain.c   | 35 +++++++++++++++++++++++++++++------
 UefiCpuPkg/SecCore/SecCore.inf |  3 ++-
 UefiCpuPkg/SecCore/SecMain.h   |  3 ++-
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
index b24e190617..b99072599d 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -1,7 +1,7 @@
 /** @file
   C functions in SEC
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -228,26 +228,49 @@ SecStartupPhase2(
 {
   EFI_SEC_PEI_HAND_OFF        *SecCoreData;
   EFI_PEI_PPI_DESCRIPTOR      *PpiList;
+  EFI_PEI_PPI_DESCRIPTOR      *PpiListTmp;
   UINT32                      Index;
   EFI_PEI_PPI_DESCRIPTOR      *AllSecPpiList;
   EFI_PEI_CORE_ENTRY_POINT    PeiCoreEntryPoint;
 
+  PeiCoreEntryPoint = NULL;
   SecCoreData   = (EFI_SEC_PEI_HAND_OFF *) Context;
   AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) SecCoreData->PeiTemporaryRamBase;
+
   //
   // Find Pei Core entry point. It will report SEC and Pei Core debug information if remote debug
   // is enabled.
   //
-  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) SecCoreData->BootFirmwareVolumeBase, &PeiCoreEntryPoint);
-  if (PeiCoreEntryPoint == NULL)
-  {
-    CpuDeadLoop ();
+  PpiList = SecPlatformMain (SecCoreData);
+  PpiListTmp = PpiList;
+  for (;;) {
+    if (CompareGuid (PpiListTmp->Guid, &gEfiPeiCoreFvLocationPpiGuid) && (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)->PeiCoreFvLocation != 0)) {
+      FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) ((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)->PeiCoreFvLocation, &PeiCoreEntryPoint);
+      if (PeiCoreEntryPoint != NULL) {
+        break;
+      }
+    }
+    if ((PpiListTmp->Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) == EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) {
+      //
+      // Continue until the end of the PPI List.
+      //
+      break;
+    }
+    PpiListTmp++;
+  }
+  //
+  // If EFI_PEI_CORE_FV_LOCATION_PPI not found or no PeiCore found by the pointer in provided PPI, try to locate PeiCore from BFV.
+  //
+  if (PeiCoreEntryPoint == NULL) {
+    FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) SecCoreData->BootFirmwareVolumeBase, &PeiCoreEntryPoint);
+    if (PeiCoreEntryPoint == NULL) {
+      CpuDeadLoop ();
+    }
   }
 
   //
   // Perform platform specific initialization before entering PeiCore.
   //
-  PpiList = SecPlatformMain (SecCoreData);
   if (PpiList != NULL) {
     //
     // Remove the terminal flag from the terminal PPI
diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
index 442f663911..fc1485b5cb 100644
--- a/UefiCpuPkg/SecCore/SecCore.inf
+++ b/UefiCpuPkg/SecCore/SecCore.inf
@@ -7,7 +7,7 @@
 #  protected mode, setup flat memory model, enable temporary memory and
 #  call into SecStartup().
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
 #  which accompanies this distribution.  The full text of the license may be found at
@@ -73,6 +73,7 @@
   ## NOTIFY
   ## SOMETIMES_CONSUMES
   gPeiSecPerformancePpiGuid
+  gEfiPeiCoreFvLocationPpiGuid
 
 [Guids]
   ## SOMETIMES_PRODUCES   ## HOB
diff --git a/UefiCpuPkg/SecCore/SecMain.h b/UefiCpuPkg/SecCore/SecMain.h
index 83244af119..80045d34f4 100644
--- a/UefiCpuPkg/SecCore/SecMain.h
+++ b/UefiCpuPkg/SecCore/SecMain.h
@@ -1,7 +1,7 @@
 /** @file
   Master header file for SecCore.
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -20,6 +20,7 @@
 #include <Ppi/SecPlatformInformation2.h>
 #include <Ppi/TemporaryRamDone.h>
 #include <Ppi/SecPerformance.h>
+#include <Ppi/PeiCoreFvLocation.h>
 
 #include <Guid/FirmwarePerformance.h>
 
-- 
2.13.3.windows.1

Re: [PATCH 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

[Laszlo Ersek]

On 02/12/19 14:19, Chasel, Chiu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform
> when PeiCore not in BFV so SecCore has to search PeiCore
> either from the FV location provided by
> EFI_PEI_CORE_FV_LOCATION_PPI or from BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  UefiCpuPkg/SecCore/SecMain.c   | 35 +++++++++++++++++++++++++++++------
>  UefiCpuPkg/SecCore/SecCore.inf |  3 ++-
>  UefiCpuPkg/SecCore/SecMain.h   |  3 ++-
>  3 files changed, 33 insertions(+), 8 deletions(-)

Thank you for the CC. OVMF doesn't consume this module, so I'll pass on
this review.

Thanks,
Laszlo