Reviewed-by: jiewen....@intel.com

> -----Original Message-----
> From: Chiu, Chasel
> Sent: Thursday, October 18, 2018 2:52 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen....@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desim...@intel.com>; Chiu, Chasel <chasel.c...@intel.com>
> Subject: [PATCH v3] IntelFsp2Pkg: Support FSP Dispatch mode
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1241
> 
> Add support for both API (original mode) and DISPATCH mode:
> 1. Add FspMode field from reserved byte of Global
>    Data Structure to tell which mode is selected by boot
>    loader. If boot loader invoking FSP-M API this field
>    will remain as default 0 (API mode), otherwise platform
>    FSP should set this field to 1 (Dispatch mode) when
>    initializing Global Data Structure.
> 2. gFspInApiModePpiGuid will be instaled when FSP running in API
>    mode and modules only for API mode should have this in depex.
> 3. If it is DISPATCH mode, FSP will return to PEI dispatcher,
>    not directly return to boot loader.
> 4. DISPATCH mode supports DXE NotifyPhase drivers so FSP
>    will not wait for PEI NotifyPhase callbacks, instead it
>    will install gFspReadyForNotifyPhasePpiGuid PPI for
>    platform to complete late initialization before transferring
>    to DXE.
> 
> Test: Verified FSP API and DISPATCH modes on 2 internal
>       platforms and both boot successfully.
> 
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Desimone Nathaniel L <nathaniel.l.desim...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.c...@intel.com>
> ---
>  IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c   | 59
> +++++++++-------
>  IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf |  3 +-
>  IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf            |  2 +-
>  IntelFsp2Pkg/FspSecCore/SecMain.c                  |  5 ++
>  IntelFsp2Pkg/Include/FspGlobalData.h               |  8 ++-
>  IntelFsp2Pkg/IntelFsp2Pkg.dec                      | 12 ++++
>  .../Library/BaseFspCommonLib/FspCommonLib.c        | 22 +++---
>  .../Library/BaseFspPlatformLib/FspPlatformNotify.c | 78
> +++++++++++++---------
>  8 files changed, 122 insertions(+), 67 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c
> b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c
> index 52435fa0b2..cbea3a7eaa 100644
> --- a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c
> +++ b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c
> @@ -48,6 +48,12 @@ CONST EFI_PEI_PPI_DESCRIPTOR gEndOfPeiSignalPpi
> = {
>    NULL
>  };
> 
> +CONST EFI_PEI_PPI_DESCRIPTOR gFspReadyForNotifyPhasePpi = {
> +  (EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> +  &gFspReadyForNotifyPhasePpiGuid,
> +  NULL
> +};
> +
>  /**
> 
>     This function waits for FSP notify.
> @@ -88,13 +94,15 @@ WaitForNotify (
>    //
>    FspWaitForNotify ();
> 
> -  //
> -  // Should not come here
> -  //
> -  while (TRUE) {
> -    DEBUG ((DEBUG_ERROR, "No FSP API should be called after FSP is
> DONE!\n"));
> -    SetFspApiReturnStatus (EFI_UNSUPPORTED);
> -    Pei2LoaderSwitchStack ();
> +  if (GetFspGlobalDataPointer ()->FspMode == FSP_IN_API_MODE) {
> +    //
> +    // Should not come here
> +    //
> +    while (TRUE) {
> +      DEBUG ((DEBUG_ERROR, "No FSP API should be called after FSP is
> DONE!\n"));
> +      SetFspApiReturnStatus (EFI_UNSUPPORTED);
> +      Pei2LoaderSwitchStack ();
> +    }
>    }
> 
>    return EFI_SUCCESS;
> @@ -121,22 +129,27 @@ FspNotifyPhasePeimEntryPoint (
> 
>    DEBUG ((DEBUG_INFO | DEBUG_INIT, "The entry of
> FspNotificationPeim\n"));
> 
> -  //
> -  // Locate old DXE IPL PPI
> -  //
> -  Status = PeiServicesLocatePpi (
> -            &gEfiDxeIplPpiGuid,
> -            0,
> -            &OldDescriptor,
> -            &OldDxeIplPpi
> -            );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  //
> -  // Re-install the DXE IPL PPI to wait for notify
> -  //
> -  Status = PeiServicesReInstallPpi (OldDescriptor, &mInstallDxeIplPpi);
> -  ASSERT_EFI_ERROR (Status);
> +  if (GetFspGlobalDataPointer ()->FspMode == FSP_IN_API_MODE) {
> +    //
> +    // Locate old DXE IPL PPI
> +    //
> +    Status = PeiServicesLocatePpi (
> +              &gEfiDxeIplPpiGuid,
> +              0,
> +              &OldDescriptor,
> +              &OldDxeIplPpi
> +              );
> +    ASSERT_EFI_ERROR (Status);
> +
> +    //
> +    // Re-install the DXE IPL PPI to wait for notify
> +    //
> +    Status = PeiServicesReInstallPpi (OldDescriptor, &mInstallDxeIplPpi);
> +    ASSERT_EFI_ERROR (Status);
> +  } else {
> +    Status = PeiServicesInstallPpi (&gFspReadyForNotifyPhasePpi);
> +    ASSERT_EFI_ERROR (Status);
> +  }
> 
>    return EFI_SUCCESS;
>  }
> diff --git a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
> b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
> index a0bc37580d..b0f6768cfa 100644
> --- a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
> +++ b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  # Component information file for the FSP notify phase PEI module.
>  #
> -#  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
>  #  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
> @@ -37,6 +37,7 @@
>  [Ppis]
>    gEfiDxeIplPpiGuid                       ## PRODUCES
>    gEfiEndOfPeiSignalPpiGuid               ## PRODUCES
> +  gFspReadyForNotifyPhasePpiGuid          ## PRODUCES
> 
>  [Protocols]
>    gEfiPciEnumerationCompleteProtocolGuid  ## PRODUCES
> diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> index c657862deb..c61af10b8a 100644
> --- a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> @@ -65,4 +65,4 @@
> 
>  [Ppis]
>    gEfiTemporaryRamSupportPpiGuid
> ## PRODUCES
> -
> +  gFspInApiModePpiGuid
> ## PRODUCES
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c
> b/IntelFsp2Pkg/FspSecCore/SecMain.c
> index 5a43ba1967..37fd4dfdeb 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecMain.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
> @@ -20,6 +20,11 @@ EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI
> gSecTemporaryRamSupportPpi = {
> 
>  EFI_PEI_PPI_DESCRIPTOR            mPeiSecPlatformInformationPpi[]
> = {
>    {
> +    EFI_PEI_PPI_DESCRIPTOR_PPI,
> +    &gFspInApiModePpiGuid,
> +    NULL
> +  },
> +  {
>      (EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
>      &gEfiTemporaryRamSupportPpiGuid,
>      &gSecTemporaryRamSupportPpi
> diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
> b/IntelFsp2Pkg/Include/FspGlobalData.h
> index 7de26606a7..2a03129f4a 100644
> --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2018, 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
> @@ -16,6 +16,9 @@
> 
>  #include <FspEas.h>
> 
> +#define FSP_IN_API_MODE      0
> +#define FSP_IN_DISPATCH_MODE 1
> +
>  #pragma pack(1)
> 
>  typedef enum {
> @@ -54,7 +57,8 @@ typedef struct  {
>     VOID               *MemoryInitUpdPtr;
>     VOID               *SiliconInitUpdPtr;
>     UINT8              ApiIdx;
> -   UINT8              Reserved3[31];
> +   UINT8              FspMode; // 0: FSP in API mode; 1: FSP in
> DISPATCH mode
> +   UINT8              Reserved3[30];
>     UINT32             PerfSig;
>     UINT16             PerfLen;
>     UINT16             Reserved4;
> diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dec b/IntelFsp2Pkg/IntelFsp2Pkg.dec
> index b2ee4b6ef5..5b037d65e2 100644
> --- a/IntelFsp2Pkg/IntelFsp2Pkg.dec
> +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dec
> @@ -43,6 +43,18 @@
>    ##  @libraryclass  Provides FSP platform sec related actions.
>    FspSecPlatformLib|Include/Library/FspSecPlatformLib.h
> 
> +[Ppis]
> +  #
> +  # PPI to indicate FSP is ready to enter notify phase
> +  # This provides flexibility for any late initialization that must be done 
> right
> before entering notify phase.
> +  #
> +  gFspReadyForNotifyPhasePpiGuid        = { 0xcd167c1e, 0x6e0b,
> 0x42b3, { 0x82, 0xf6, 0xe3, 0xe9, 0x6, 0x19, 0x98, 0x10}}
> +
> +  #
> +  # PPI as dependency on some modules which only required for API mode
> +  #
> +  gFspInApiModePpiGuid                  = { 0xa1eeab87, 0xc859,
> 0x479d, {0x89, 0xb5, 0x14, 0x61, 0xf4, 0x06, 0x1a, 0x3e}}
> +
>  [Guids]
>    #
>    # GUID defined in package
> diff --git a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> index d0128304e6..64ffba1d16 100644
> --- a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> +++ b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> @@ -519,14 +519,16 @@ FspApiReturnStatusReset (
> 
>    LoopUntilReset = TRUE;
>    DEBUG ((DEBUG_INFO, "FSP returning control to Bootloader with reset
> required return status %x\n",FspResetType));
> -  ///
> -  /// Below code is not an infinite loop.The control will go back to API
> calling function in BootLoader each time BootLoader
> -  /// calls the FSP API without honoring the reset request by FSP
> -  ///
> -  do {
> -    SetFspApiReturnStatus ((EFI_STATUS)FspResetType);
> -    Pei2LoaderSwitchStack ();
> -    DEBUG ((DEBUG_ERROR, "!!!ERROR: FSP has requested BootLoader
> for reset. But BootLoader has not honored the reset\n"));
> -    DEBUG ((DEBUG_ERROR, "!!!ERROR: Please add support in
> BootLoader to honor the reset request from FSP\n"));
> -  } while (LoopUntilReset);
> +  if (GetFspGlobalDataPointer ()->FspMode == FSP_IN_API_MODE) {
> +    ///
> +    /// Below code is not an infinite loop.The control will go back to API
> calling function in BootLoader each time BootLoader
> +    /// calls the FSP API without honoring the reset request by FSP
> +    ///
> +    do {
> +      SetFspApiReturnStatus ((EFI_STATUS)FspResetType);
> +      Pei2LoaderSwitchStack ();
> +      DEBUG ((DEBUG_ERROR, "!!!ERROR: FSP has requested BootLoader
> for reset. But BootLoader has not honored the reset\n"));
> +      DEBUG ((DEBUG_ERROR, "!!!ERROR: Please add support in
> BootLoader to honor the reset request from FSP\n"));
> +    } while (LoopUntilReset);
> +  }
>  }
> diff --git a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> index ac1fc1ac0b..cbc3b472c6 100644
> --- a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> +++ b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2018, 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
> @@ -117,6 +117,9 @@ FspSiliconInitDone2 (
>    IN EFI_STATUS Status
>    )
>  {
> +  volatile EFI_STATUS FspStatus;
> +
> +  FspStatus = Status;
>    //
>    // Convert to FSP EAS defined API return codes
>    //
> @@ -139,13 +142,15 @@ FspSiliconInitDone2 (
>    DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() - [Status:
> 0x%08X] - End\n", Status));
>    PERF_END_EX (&gFspPerformanceDataGuid, "EventRec", NULL, 0,
> FSP_STATUS_CODE_SILICON_INIT | FSP_STATUS_CODE_COMMON_CODE |
> FSP_STATUS_CODE_API_EXIT);
>    REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT);
> -  do {
> -    SetFspApiReturnStatus (Status);
> -    Pei2LoaderSwitchStack ();
> -    if (Status != EFI_SUCCESS) {
> -      DEBUG ((DEBUG_ERROR, "!!!ERROR: FspSiliconInitApi() - [Status:
> 0x%08X] - Error encountered during previous API and cannot proceed
> further\n", Status));
> -    }
> -  } while (Status != EFI_SUCCESS);
> +  if (GetFspGlobalDataPointer ()->FspMode == FSP_IN_API_MODE) {
> +    do {
> +      SetFspApiReturnStatus (Status);
> +      Pei2LoaderSwitchStack ();
> +      if (Status != EFI_SUCCESS) {
> +        DEBUG ((DEBUG_ERROR, "!!!ERROR: FspSiliconInitApi() - [Status:
> 0x%08X] - Error encountered during previous API and cannot proceed
> further\n", Status));
> +      }
> +    } while (FspStatus != EFI_SUCCESS);
> +  }
>  }
> 
>  /**
> @@ -162,6 +167,9 @@ FspMemoryInitDone2 (
>    )
>  {
>    FSP_GLOBAL_DATA   *FspData;
> +  volatile EFI_STATUS FspStatus;
> +
> +  FspStatus = Status;
>    //
>    // Calling use FspMemoryInit API
>    // Update HOB and return the control directly
> @@ -199,13 +207,15 @@ FspMemoryInitDone2 (
>    PERF_START_EX(&gFspPerformanceDataGuid, "EventRec", NULL,
> (FspData->PerfData[2] & FSP_PERFORMANCE_DATA_TIMER_MASK),
> FSP_STATUS_CODE_MEMORY_INIT | FSP_STATUS_CODE_COMMON_CODE |
> FSP_STATUS_CODE_API_ENTRY);
>    PERF_END_EX(&gFspPerformanceDataGuid, "EventRec", NULL, 0,
> FSP_STATUS_CODE_MEMORY_INIT | FSP_STATUS_CODE_COMMON_CODE |
> FSP_STATUS_CODE_API_EXIT);
>    REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> FSP_STATUS_CODE_MEMORY_INIT | FSP_STATUS_CODE_COMMON_CODE |
> FSP_STATUS_CODE_API_EXIT);
> -  do {
> -    SetFspApiReturnStatus (Status);
> -    Pei2LoaderSwitchStack ();
> -    if (Status != EFI_SUCCESS) {
> -      DEBUG ((DEBUG_ERROR, "!!!ERROR: FspMemoryInitApi() - [Status:
> 0x%08X] - Error encountered during previous API and cannot proceed
> further\n", Status));
> -    }
> -  } while (Status != EFI_SUCCESS);
> +  if (GetFspGlobalDataPointer ()->FspMode == FSP_IN_API_MODE) {
> +    do {
> +      SetFspApiReturnStatus (Status);
> +      Pei2LoaderSwitchStack ();
> +      if (Status != EFI_SUCCESS) {
> +        DEBUG ((DEBUG_ERROR, "!!!ERROR: FspMemoryInitApi() -
> [Status: 0x%08X] - Error encountered during previous API and cannot
> proceed further\n", Status));
> +      }
> +    } while (FspStatus != EFI_SUCCESS);
> +  }
> 
>    //
>    // The TempRamExitApi is called
> @@ -238,6 +248,9 @@ FspTempRamExitDone2 (
>    )
>  {
>    //
> +  volatile EFI_STATUS FspStatus;
> +
> +  FspStatus = Status;
>    // Convert to FSP EAS defined API return codes
>    //
>    switch (Status) {
> @@ -259,13 +272,15 @@ FspTempRamExitDone2 (
>    SetFspMeasurePoint (FSP_PERF_ID_API_TEMP_RAM_EXIT_EXIT);
>    PERF_END_EX(&gFspPerformanceDataGuid, "EventRec", NULL, 0,
> FSP_STATUS_CODE_TEMP_RAM_EXIT |
> FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT);
>    REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> FSP_STATUS_CODE_TEMP_RAM_EXIT |
> FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT);
> -  do {
> -    SetFspApiReturnStatus (Status);
> -    Pei2LoaderSwitchStack ();
> -    if (Status != EFI_SUCCESS) {
> -      DEBUG ((DEBUG_ERROR, "!!!ERROR: TempRamExitApi() - [Status:
> 0x%08X] - Error encountered during previous API and cannot proceed
> further\n", Status));
> -    }
> -  } while (Status != EFI_SUCCESS);
> +  if (GetFspGlobalDataPointer ()->FspMode == FSP_IN_API_MODE) {
> +    do {
> +      SetFspApiReturnStatus (Status);
> +      Pei2LoaderSwitchStack ();
> +      if (Status != EFI_SUCCESS) {
> +        DEBUG ((DEBUG_ERROR, "!!!ERROR: TempRamExitApi() - [Status:
> 0x%08X] - Error encountered during previous API and cannot proceed
> further\n", Status));
> +      }
> +    } while (FspStatus != EFI_SUCCESS);
> +  }
>    SetPhaseStatusCode (FSP_STATUS_CODE_SILICON_INIT);
>    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_SILICON_INIT_ENTRY);
>    PERF_START_EX(&gFspPerformanceDataGuid, "EventRec", NULL, 0,
> FSP_STATUS_CODE_SILICON_INIT | FSP_STATUS_CODE_COMMON_CODE |
> FSP_STATUS_CODE_API_ENTRY);
> @@ -289,6 +304,7 @@ FspWaitForNotify (
>    UINT32                     NotificationValue;
>    UINT32                     NotificationCount;
>    UINT8                      Count;
> +  volatile EFI_STATUS        FspStatus;
> 
>    NotificationCount = 0;
>    while (NotificationCount < sizeof(mFspNotifySequence) / sizeof(UINT32))
> {
> @@ -341,15 +357,17 @@ FspWaitForNotify (
>        PERF_END_EX(&gFspPerformanceDataGuid, "EventRec", NULL, 0,
> FSP_STATUS_CODE_END_OF_FIRMWARE_NOTIFICATION |
> FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT);
>        REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> FSP_STATUS_CODE_END_OF_FIRMWARE_NOTIFICATION |
> FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT);
>      }
> -    do {
> -      SetFspApiReturnStatus(Status);
> -      Pei2LoaderSwitchStack();
> -      if (Status != EFI_SUCCESS) {
> -        DEBUG ((DEBUG_ERROR, "!!!ERROR: NotifyPhaseApi()
> [Phase: %08X] - Failed - [Status: 0x%08X]\n", NotificationValue, Status));
> -      }
> -    } while (Status != EFI_SUCCESS);
> +    if (GetFspGlobalDataPointer ()->FspMode == FSP_IN_API_MODE) {
> +      FspStatus = Status;
> +      do {
> +        SetFspApiReturnStatus(Status);
> +        Pei2LoaderSwitchStack();
> +        if (Status != EFI_SUCCESS) {
> +          DEBUG ((DEBUG_ERROR, "!!!ERROR: NotifyPhaseApi()
> [Phase: %08X] - Failed - [Status: 0x%08X]\n", NotificationValue, Status));
> +        }
> +      } while (FspStatus != EFI_SUCCESS);
> +    }
>    }
> -
>    //
>    // Control goes back to the PEI Core and it dispatches further PEIMs.
>    // DXEIPL is the final one to transfer control back to the boot loader.
> --
> 2.13.3.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to