public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Nate DeSimone" <nathaniel.l.desimone@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Chiu, Chasel" <chasel.chiu@intel.com>
Cc: "Ma, Maurice" <maurice.ma@intel.com>, "Zeng, Star" <star.zeng@intel.com>
Subject: Re: [edk2-devel] [PATCH v8] IntelFsp2Pkg: Support Multi-Phase SiInit and debug handlers.
Date: Thu, 14 May 2020 05:06:26 +0000	[thread overview]
Message-ID: <BL0PR11MB34896DC43C4D8AD81B5A2EC3CDBC0@BL0PR11MB3489.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20200513145019.13504-1-chasel.chiu@intel.com>

Hi Chasel,

Two minor comments.

#1) The following comment:

  // Usually OnSeparateStack can be reset when FSP-M returning control back to bootloader and after
  // that FSP again has a chance to run on the same bootloader stack.

This is not quite accurate. Once we switch to the separate stack, we can never come back to bootloader stack. The reason why is because all the PEI core global data (List of PEIMs, PPI list, etc.) is stored in the stack. Because those data structure use several pointers, all the PEI core state needs to be relocated/fixed up if the stack base changes.

The only reason this works OK is because the PeiCore() function never returns.

#2) I think Reserved4[26] should be Reserved4[20]. Please see inline.

With those changes...

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

The only opportunity we have to fixup the PEI global state is during the flow that shadows PEI to DRAM, since that requires us to relocate PEI core anyway. So once OneSeperateStack becomes 1... it can never go back to 0 without a platform reset.
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu,
> Chasel
> Sent: Wednesday, May 13, 2020 7:50 AM
> To: devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2-devel] [PATCH v8] IntelFsp2Pkg: Support Multi-Phase SiInit
> and debug handlers.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2698
> 
> To enhance FSP silicon initialization flexibility an optional Multi-Phase API is
> introduced and FSP header needs update for new API offset. Also new
> SecCore module created for FspMultiPhaseSiInit API
> 
> New ARCH_UPD introduced for enhancing FSP debug message flexibility now
> bootloader can pass its own debug handler function pointer and FSP will call
> the function to handle debug message.
> To support calling bootloader functions, a FspGlobalData field added to
> indicate if FSP needs to switch stack when FSP running on separate stack
> from bootloader.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
> 
> V8:
> Add OnSeparateStack FspGlobalData field to indicate if FSP running on
> bootloader stack or not. When FSP calling bootloader functions switching
> stack is necessary when FSP running on separate stack.
> 
>  IntelFsp2Pkg/FspSecCore/SecFsp.c                                   |  12 +++++++++++-
>  IntelFsp2Pkg/FspSecCore/SecFspApiChk.c                             |   6 +++---
>  IntelFsp2Pkg/FspSecCore/SecMain.c                                  |  10 +++++++++-
>  IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c |  19
> ++++++++++++++++++-
>  IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf                          |  52
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm                   |  99
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++
>  IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm                |   5
> ++++-
>  IntelFsp2Pkg/Include/FspEas/FspApi.h                               | 130
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++---
>  IntelFsp2Pkg/Include/FspGlobalData.h                               |  11 ++++++++---
>  IntelFsp2Pkg/Include/Guid/FspHeaderFile.h                          |  10 ++++++++--
>  IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h                   |  16
> +++++++++++++++-
>  11 files changed, 354 insertions(+), 16 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/SecFsp.c
> b/IntelFsp2Pkg/FspSecCore/SecFsp.c
> index 446d1730e9..216f7bb6c5 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecFsp.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecFsp.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -161,6 +161,16 @@ FspGlobalDataInit (
>    SetFspSiliconInitUpdDataPointer (NULL);
> 
>    //
> +  // Initialize OnSeparateStack value.
> +  //
> +  if (PcdGet8 (PcdFspHeapSizePercentage) != 0) {
> +    //
> +    // FSP is running on its own stack and may need switching stack when
> calling bootloader functions.
> +    //
> +    GetFspGlobalDataPointer ()->OnSeparateStack = 1;  }
> +
> +  //
>    // Initialize serial port
>    // It might have been done in ProcessLibraryConstructorList(), however,
>    // the FSP global data is not initialized at that time. So do it again diff --git
> a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> index 8e0595fe9a..1334959005 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -59,7 +59,7 @@ FspApiCallingCheck (
>          Status = EFI_UNSUPPORTED;
>        }
>      }
> -  } else if (ApiIdx == FspSiliconInitApiIndex) {
> +  } else if ((ApiIdx == FspSiliconInitApiIndex) || (ApiIdx ==
> + FspMultiPhaseSiInitApiIndex)) {
>      //
>      // FspSiliconInit check
>      //
> @@ -68,7 +68,7 @@ FspApiCallingCheck (
>      } else {
>        if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
>          Status = EFI_UNSUPPORTED;
> -      } else if (EFI_ERROR (FspUpdSignatureCheck (ApiIdx, ApiParam))) {
> +      } else if (EFI_ERROR (FspUpdSignatureCheck
> + (FspSiliconInitApiIndex, ApiParam))) {
>          Status = EFI_INVALID_PARAMETER;
>        }
>      }
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c
> b/IntelFsp2Pkg/FspSecCore/SecMain.c
> index 7169afc6c7..300f93ebab 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecMain.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -221,6 +221,14 @@ SecTemporaryRamSupport (
>    UINTN             CurrentStack;
>    UINTN             FspStackBase;
> 
> +  //
> +  // Override OnSeparateStack to 1 because this function will switch
> + stack to permanent memory which  // makes FSP running on different
> stack from bootloader temp ram stack.
> +  // Usually OnSeparateStack can be reset when FSP-M returning control
> + back to bootloader and after  // that FSP again has a chance to run on the
> same bootloader stack.
> +  //
> +  GetFspGlobalDataPointer ()->OnSeparateStack = 1;
> +
>    if (PcdGet8 (PcdFspHeapSizePercentage) == 0) {
> 
>      CurrentStack = AsmReadEsp();
> diff --git
> a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
> b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
> index f7945b5240..df8c5d121f 100644
> --- a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
> +++ b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Null instance of Platform Sec Lib.
> 
> -  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -25,3 +25,20 @@ FspUpdSignatureCheck (  {
>    return EFI_SUCCESS;
>  }
> +
> +/**
> +  This function handles FspMultiPhaseSiInitApi.
> +
> +  @param[in]  ApiIdx           Internal index of the FSP API.
> +  @param[in]  ApiParam         Parameter of the FSP API.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +FspMultiPhaseSiInitApiHandler (
> +  IN UINT32   ApiIdx,
> +  IN VOID     *ApiParam
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> diff --git a/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> b/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> new file mode 100644
> index 0000000000..0a24eb2a8b
> --- /dev/null
> +++ b/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> @@ -0,0 +1,52 @@
> +## @file
> +#  Sec Core for FSP to support MultiPhase (SeparatePhase) SiInitialization.
> +#
> +#  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> # #
> +SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = Fsp22SecCoreS
> +  FILE_GUID                      = DF0FCD70-264A-40BF-BBD4-06C76DB19CB1
> +  MODULE_TYPE                    = SEC
> +  VERSION_STRING                 = 1.0
> +
> +#
> +# The following information is for reference only and not required by the
> build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32
> +#
> +
> +[Sources]
> +  SecFspApiChk.c
> +  SecFsp.h
> +
> +[Sources.IA32]
> +  Ia32/Stack.nasm
> +  Ia32/Fsp22ApiEntryS.nasm
> +  Ia32/FspApiEntryCommon.nasm
> +  Ia32/FspHelper.nasm
> +
> +[Binaries.Ia32]
> +  RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  IntelFsp2Pkg/IntelFsp2Pkg.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  BaseLib
> +  PciCf8Lib
> +  SerialPortLib
> +  FspSwitchStackLib
> +  FspCommonLib
> +  FspSecPlatformLib
> +
> +[Ppis]
> +  gEfiTemporaryRamSupportPpiGuid                              ## PRODUCES
> +
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
> new file mode 100644
> index 0000000000..c5e73a635b
> --- /dev/null
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
> @@ -0,0 +1,99 @@
> +;; @file
> +;  Provide FSP API entry points.
> +;
> +; Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> ;
> +SPDX-License-Identifier: BSD-2-Clause-Patent ;;
> +
> +    SECTION .text
> +
> +;
> +; Following functions will be provided in C ; extern
> +ASM_PFX(FspApiCommon) extern
> ASM_PFX(FspMultiPhaseSiInitApiHandler)
> +
> +;----------------------------------------------------------------------
> +------
> +; NotifyPhase API
> +;
> +; This FSP API will notify the FSP about the different phases in the
> +boot ; process ;
> +;----------------------------------------------------------------------
> +------
> +global ASM_PFX(NotifyPhaseApi)
> +ASM_PFX(NotifyPhaseApi):
> +  mov    eax,  2 ; FSP_API_INDEX.NotifyPhaseApiIndex
> +  jmp    ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------
> +------
> +; FspSiliconInit API
> +;
> +; This FSP API initializes the CPU and the chipset including the IO ;
> +controllers in the chipset to enable normal operation of these devices.
> +;
> +;----------------------------------------------------------------------
> +------
> +global ASM_PFX(FspSiliconInitApi)
> +ASM_PFX(FspSiliconInitApi):
> +  mov    eax,  5 ; FSP_API_INDEX.FspSiliconInitApiIndex
> +  jmp    ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------
> +------
> +; FspMultiPhaseSiInitApi API
> +;
> +; This FSP API provides multi-phase silicon initialization, which
> +brings greater ; modularity beyond the existing FspSiliconInit() API.
> +; Increased modularity is achieved by adding an extra API to FSP-S.
> +; This allows the bootloader to add board specific initialization steps
> +throughout ; the SiliconInit flow as needed.
> +;
> +;----------------------------------------------------------------------
> +------
> +global ASM_PFX(FspMultiPhaseSiInitApi)
> +ASM_PFX(FspMultiPhaseSiInitApi):
> +  mov    eax,  6 ; FSP_API_INDEX.FspMultiPhaseSiInitApiIndex
> +  jmp    ASM_PFX(FspApiCommon)
> +
> +;----------------------------------------------------------------------
> +------
> +; FspApiCommonContinue API
> +;
> +; This is the FSP API common entry point to resume the FSP execution ;
> +;----------------------------------------------------------------------
> +------
> +global ASM_PFX(FspApiCommonContinue)
> +ASM_PFX(FspApiCommonContinue):
> +  ;
> +  ; Handle FspMultiPhaseSiInitApiIndex API
> +  ;
> +  cmp    eax, 6
> +  jnz    NotMultiPhaseSiInitApi
> +
> +  pushad
> +  push   DWORD [esp + (4 * 8 + 4)]  ; push ApiParam
> +  push   eax                ; push ApiIdx
> +  call   ASM_PFX(FspMultiPhaseSiInitApiHandler)
> +  add    esp, 8
> +  mov    dword  [esp + (4 * 7)], eax
> +  popad
> +  ret
> +
> +NotMultiPhaseSiInitApi:
> +  jmp $
> +  ret
> +
> +;----------------------------------------------------------------------
> +------
> +; TempRamInit API
> +;
> +; Empty function for WHOLEARCHIVE build option ;
> +;----------------------------------------------------------------------
> +------
> +global ASM_PFX(TempRamInitApi)
> +ASM_PFX(TempRamInitApi):
> +  jmp $
> +  ret
> +
> +;----------------------------------------------------------------------
> +------
> +; Module Entrypoint API
> +;----------------------------------------------------------------------
> +------
> +global ASM_PFX(_ModuleEntryPoint)
> +ASM_PFX(_ModuleEntryPoint):
> +  jmp $
> +
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
> index bb4451b145..26ae7d9fd3 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
> @@ -1,7 +1,7 @@
>  ;; @file
>  ;  Provide FSP API entry points.
>  ;
> -; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2016 - 2020, Intel Corporation. All rights
> +reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent  ;;
> 
> @@ -62,6 +62,9 @@ FspApiCommon2:
>    cmp    eax, 3   ; FspMemoryInit API
>    jz     FspApiCommon3
> 
> +  cmp    eax, 6   ; FspMultiPhaseSiInitApiIndex API
> +  jz     FspApiCommon3
> +
>    call   ASM_PFX(AsmGetFspInfoHeader)
>    jmp    ASM_PFX(Loader2PeiSwitchStack)
> 
> diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> index dcf489dbe6..ed40f9538c 100644
> --- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> +++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> @@ -1,8 +1,8 @@
>  /** @file
>    Intel FSP API definition from Intel Firmware Support Package External
> -  Architecture Specification v2.0.
> +  Architecture Specification v2.0 - v2.2
> 
> -  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -10,6 +10,8 @@
>  #ifndef _FSP_API_H_
>  #define _FSP_API_H_
> 
> +#include <Pi/PiStatusCode.h>
> +
>  ///
>  /// FSP Reset Status code
>  /// These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status Code @@ -
> 24,6 +26,65 @@
>  #define FSP_STATUS_RESET_REQUIRED_8            0x40000008
>  /// @}
> 
> +///
> +/// FSP Event related definition.
> +///
> +#define FSP_EVENT_CODE   0xF5000000
> +#define FSP_POST_CODE    (FSP_EVENT_CODE | 0x00F80000)
> +
> +/*
> +  FSP may optionally include the capability of generating events messages to
> aid in the debugging of firmware issues.
> +  These events fall under three catagories: Error, Progress, and Debug.
> +The event reporting mechanism follows the
> +  status code services described in section 6 and 7 of the PI Specification v1.7
> Volume 3.
> +
> +  @param[in] Type                   Indicates the type of event being reported.
> +                                    See MdePkg/Include/Pi/PiStatusCode.h for the definition
> of EFI_STATUS_CODE_TYPE.
> +  @param[in] Value                  Describes the current status of a hardware or
> software entity.
> +                                    This includes information about the class and subclass that
> is used to classify the entity as well as an operation.
> +                                    For progress events, the operation is the current activity.
> For error events, it is the exception.
> +                                    For debug events, it is not defined at this time.
> +                                    See MdePkg/Include/Pi/PiStatusCode.h for the definition
> of EFI_STATUS_CODE_VALUE.
> +  @param[in] Instance               The enumeration of a hardware or software
> entity within the system.
> +                                    A system may contain multiple entities that match a
> class/subclass pairing. The instance differentiates between them.
> +                                    An instance of 0 indicates that instance information is
> unavailable, not meaningful, or not relevant.
> +                                    Valid instance numbers start with 1.
> +  @param[in] *CallerId              This parameter can be used to identify the
> sub-module within the FSP generating the event.
> +                                    This parameter may be NULL.
> +  @param[in] *Data                  This optional parameter may be used to pass
> additional data. The contents can have event-specific data.
> +                                    For example, the FSP provides a
> EFI_STATUS_CODE_STRING_DATA instance to this parameter when sending
> debug messages.
> +                                    This parameter is NULL when no additional data is
> provided.
> +
> +  @retval EFI_SUCCESS               The event was handled successfully.
> +  @retval EFI_INVALID_PARAMETER     Input parameters are invalid.
> +  @retval EFI_DEVICE_ERROR          The event handler failed.
> +*/
> +typedef
> +EFI_STATUS
> +(EFIAPI *FSP_EVENT_HANDLER) (
> +  IN          EFI_STATUS_CODE_TYPE   Type,
> +  IN          EFI_STATUS_CODE_VALUE  Value,
> +  IN          UINT32                 Instance,
> +  IN OPTIONAL EFI_GUID               *CallerId,
> +  IN OPTIONAL EFI_STATUS_CODE_DATA   *Data
> +  );
> +
> +/*
> +  Handler for FSP-T debug log messages, provided by the bootloader.
> +
> +  @param[in] DebugMessage           A pointer to the debug message to be
> written to the log.
> +  @param[in] MessageLength          Number of bytes to written to the debug
> log.
> +
> +  @retval UINT32                    The return value indicates the number of bytes
> actually written to
> +                                    the debug log. If the return value is less than
> MessageLength,
> +                                    an error occurred.
> +*/
> +typedef
> +UINT32
> +(EFIAPI *FSP_DEBUG_HANDLER) (
> +  IN CHAR8*                 DebugMessage,
> +  IN UINT32                 MessageLength
> +  );
> +
>  #pragma pack(1)
>  ///
>  /// FSP_UPD_HEADER Configuration.
> @@ -77,7 +138,12 @@ typedef struct {
>    /// Current boot mode.
>    ///
>    UINT32                      BootMode;
> -  UINT8                       Reserved1[8];
> +  ///
> +  /// Optional event handler for the bootloader to be informed of events
> occurring during FSP execution.
> +  /// This value is only valid if Revision is >= 2.
> +  ///
> +  FSP_EVENT_HANDLER           *FspEventHandler;
> +  UINT8                       Reserved1[4];
>  } FSPM_ARCH_UPD;
> 
>  ///
> @@ -147,6 +213,40 @@ typedef struct {
>    FSP_INIT_PHASE     Phase;
>  } NOTIFY_PHASE_PARAMS;
> 
> +///
> +/// Action definition for FspMultiPhaseSiInit API /// typedef enum {
> +  EnumMultiPhaseGetNumberOfPhases  = 0x0,
> +  EnumMultiPhaseExecutePhase       = 0x1
> +} FSP_MULTI_PHASE_ACTION;
> +
> +///
> +/// Data structure returned by FSP when bootloader calling ///
> +FspMultiPhaseSiInit API with action 0
> (EnumMultiPhaseGetNumberOfPhases)
> +/// typedef struct {
> +  UINT32                         NumberOfPhases;
> +  UINT32                         PhasesExecuted;
> +} FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS;
> +
> +///
> +/// FspMultiPhaseSiInit function parameter.
> +///
> +/// For action 0 (EnumMultiPhaseGetNumberOfPhases):
> +///   - PhaseIndex must be 0.
> +///   - MultiPhaseParamPtr should point to an instance of
> FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS.
> +///
> +/// For action 1 (EnumMultiPhaseExecutePhase):
> +///   - PhaseIndex will be the phase that will be executed by FSP.
> +///   - MultiPhaseParamPtr shall be NULL.
> +///
> +typedef struct {
> +  IN     FSP_MULTI_PHASE_ACTION  MultiPhaseAction;
> +  IN     UINT32                  PhaseIndex;
> +  IN OUT VOID                    *MultiPhaseParamPtr;
> +} FSP_MULTI_PHASE_PARAMS;
> +
>  #pragma pack()
> 
>  /**
> @@ -279,4 +379,28 @@ EFI_STATUS
>    IN  VOID    *FspsUpdDataPtr
>    );
> 
> +/**
> +  This FSP API is expected to be called after FspSiliconInit but before
> FspNotifyPhase.
> +  This FSP API provides multi-phase silicon initialization; which
> +brings greater modularity
> +  beyond the existing FspSiliconInit() API. Increased modularity is
> +achieved by adding an
> +  extra API to FSP-S. This allows the bootloader to add board specific
> +initialization steps
> +  throughout the SiliconInit flow as needed.
> +
> +  @param[in,out] FSP_MULTI_PHASE_PARAMS   For action -
> EnumMultiPhaseGetNumberOfPhases:
> +                                            FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr
> will contain
> +                                            how many phases supported by FSP.
> +                                          For action - EnumMultiPhaseExecutePhase:
> +                                            FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr
> shall be NULL.
> +  @retval EFI_SUCCESS                     FSP execution environment was initialized
> successfully.
> +  @retval EFI_INVALID_PARAMETER           Input parameters are invalid.
> +  @retval EFI_UNSUPPORTED                 The FSP calling conditions were not
> met.
> +  @retval EFI_DEVICE_ERROR                FSP initialization failed.
> +  @retval FSP_STATUS_RESET_REQUIREDx      A reset is required. These
> status codes will not be returned during S3.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *FSP_MULTI_PHASE_SI_INIT) (
> +  IN FSP_MULTI_PHASE_PARAMS     *MultiPhaseSiInitParamPtr
> +);
> +
>  #endif
> diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
> b/IntelFsp2Pkg/Include/FspGlobalData.h
> index 1896b0240a..34a3793ace 100644
> --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -22,6 +22,7 @@ typedef enum {
>    FspMemoryInitApiIndex,
>    TempRamExitApiIndex,
>    FspSiliconInitApiIndex,
> +  FspMultiPhaseSiInitApiIndex,
>    FspApiIndexMax
>  } FSP_API_INDEX;
> 
> @@ -52,10 +53,14 @@ typedef struct  {
>     VOID               *SiliconInitUpdPtr;
>     UINT8              ApiIdx;
>     UINT8              FspMode; // 0: FSP in API mode; 1: FSP in DISPATCH mode
> -   UINT8              Reserved3[30];
> +   UINT8              OnSeparateStack;
> +   UINT8              Reserved3;
> +   UINT32             NumberOfPhases;
> +   UINT32             PhasesExecuted;
> +   UINT8              Reserved4[26];

I think this should be Reserved4[20];

>     UINT32             PerfSig;
>     UINT16             PerfLen;
> -   UINT16             Reserved4;
> +   UINT16             Reserved5;
>     UINT32             PerfIdx;
>     UINT64             PerfData[32];
>  } FSP_GLOBAL_DATA;
> diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> index 16f43a1273..3474bac1de 100644
> --- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> +++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
> @@ -1,8 +1,8 @@
>  /** @file
>    Intel FSP Header File definition from Intel Firmware Support Package
> External
> -  Architecture Specification v2.0.
> +  Architecture Specification v2.0 and above.
> 
> -  Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -110,6 +110,12 @@ typedef struct {
>    /// Byte 0x44: The offset for the API to initialize the CPU and chipset.
>    ///
>    UINT32  FspSiliconInitEntryOffset;
> +  ///
> +  /// Byte 0x48: Offset for the API for the optional Multi-Phase processor
> and chipset initialization.
> +  ///            This value is only valid if FSP HeaderRevision is >= 5.
> +  ///            If the value is set to 0x00000000, then this API is not available in
> this component.
> +  ///
> +  UINT32  FspMultiPhaseSiInitEntryOffset;
>  } FSP_INFO_HEADER;
> 
>  ///
> diff --git a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
> b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
> index 4d01b5f6d9..51a0309aed 100644
> --- a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
> +++ b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2015 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -79,4 +79,18 @@ FspUpdSignatureCheck (
>    IN VOID     *ApiParam
>    );
> 
> +/**
> +  This function handles FspMultiPhaseSiInitApi.
> +
> +  @param[in]  ApiIdx           Internal index of the FSP API.
> +  @param[in]  ApiParam         Parameter of the FSP API.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +FspMultiPhaseSiInitApiHandler (
> +  IN UINT32   ApiIdx,
> +  IN VOID     *ApiParam
> +  );
> +
>  #endif
> --
> 2.13.3.windows.1
> 
> 
> 


  reply	other threads:[~2020-05-14  5:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 14:50 [PATCH v8] IntelFsp2Pkg: Support Multi-Phase SiInit and debug handlers Chiu, Chasel
2020-05-14  5:06 ` Nate DeSimone [this message]
2020-05-14 12:57   ` [edk2-devel] " Chiu, Chasel
     [not found] <160E9DFD1594EC48.15934@groups.io>
2020-05-14  0:35 ` Chiu, Chasel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BL0PR11MB34896DC43C4D8AD81B5A2EC3CDBC0@BL0PR11MB3489.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox