public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done
       [not found] <20161108094525.35036-1-richard.marian.thomaiyar@intel.com>
@ 2016-11-08 22:39 ` Mudusuru, Giri P
  2016-11-09  6:31   ` Thomaiyar, Richard Marian
  2016-11-10  5:11   ` Mudusuru, Giri P
  2016-11-10  5:23 ` Yao, Jiewen
  1 sibling, 2 replies; 4+ messages in thread
From: Mudusuru, Giri P @ 2016-11-08 22:39 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian, edk2-devel@lists.01.org
  Cc: Ma, Maurice, Yao, Jiewen

Hi Richard,

In the below snippet the debug message is based on the API status and will not be display if the switch stack already happened right? Also since it is in a do while loop "cannot proceed further" is not necessarily accurate.

> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -
> -  Pei2LoaderSwitchStack();
> -
> +  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);
Thanks,
-Giri

> -----Original Message-----
> From: Thomaiyar, Richard Marian
> Sent: Tuesday, November 8, 2016 1:45 AM
> To: edk2-devel@lists.01.org
> Cc: Ma, Maurice <maurice.ma@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Mudusuru, Giri P <giri.p.mudusuru@intel.com>;
> Thomaiyar, Richard Marian <richard.marian.thomaiyar@intel.com>
> Subject: [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done
> 
> Added FspMemoryInitDone2, FspTempRamExitDone2, FspSiliconInitDone2
> to return error status to Boot Loader for FSP API calls.
> To maintain backward compatibility existing functions
> (FspMemoryInitDone, FspTempRamExitDone, FspSiliconInitDone)
> declaration left untouched.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Richard Thomaiyar <richard.marian.thomaiyar@intel.com>
> ---
>  IntelFsp2Pkg/Include/Library/FspPlatformLib.h      |  35 +++++
>  .../Library/BaseFspPlatformLib/FspPlatformNotify.c | 151 +++++++++++++++++-
> ---
>  2 files changed, 163 insertions(+), 23 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> index 61e77bd..1f39601 100644
> --- a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> +++ b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> @@ -93,4 +93,39 @@ FspWaitForNotify (
>    VOID
>    );
> 
> +/**
> +  This function transfer control back to BootLoader after FspSiliconInit.
> +
> +  @param[in] Status return status for the FspSiliconInit.
> +**/
> +VOID
> +EFIAPI
> +FspSiliconInitDone2 (
> +  IN EFI_STATUS Status
> +  );
> +
> +/**
> +  This function returns control to BootLoader after MemoryInitApi.
> +
> +  @param[in] Status return status for the MemoryInitApi.
> +  @param[in,out] HobListPtr The address of HobList pointer.
> +**/
> +VOID
> +EFIAPI
> +FspMemoryInitDone2 (
> +  IN EFI_STATUS Status,
> +  IN OUT VOID   **HobListPtr
> +  );
> +
> +/**
> +  This function returns control to BootLoader after TempRamExitApi.
> +
> +  @param[in] Status return status for the TempRamExitApi.
> +**/
> +VOID
> +EFIAPI
> +FspTempRamExitDone2 (
> +  IN EFI_STATUS Status
> +  );
> +
>  #endif
> diff --git a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> index 755e84f..ac1fc1a 100644
> --- a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> +++ b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> @@ -108,35 +108,56 @@ FspNotificationHandler (
>  /**
>    This function transfer control back to BootLoader after FspSiliconInit.
> 
> +  @param[in] Status return status for the FspSiliconInit.
> +
>  **/
>  VOID
>  EFIAPI
> -FspSiliconInitDone (
> -  VOID
> +FspSiliconInitDone2 (
> +  IN EFI_STATUS Status
>    )
>  {
>    //
> +  // Convert to FSP EAS defined API return codes
> +  //
> +  switch (Status) {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() Invalid Error -
> [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
> +  //
>    // This is the end of the FspSiliconInit API
>    // Give control back to the boot loader
>    //
>    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_SILICON_INIT_EXIT);
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() - End\n"));
> +  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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -
> -  Pei2LoaderSwitchStack();
> -
> +  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);
>  }
> 
>  /**
>    This function returns control to BootLoader after MemoryInitApi.
> 
> -  @param[in,out] HobListPtr The address of HobList pointer.
> +  @param[in] Status return status for the MemoryInitApi.
> +  @param[in,out] HobListPtr The address of HobList pointer, if NULL, will get
> value from GetFspApiParameter2 ()
>  **/
>  VOID
>  EFIAPI
> -FspMemoryInitDone (
> +FspMemoryInitDone2 (
> +  IN EFI_STATUS Status,
>    IN OUT VOID   **HobListPtr
>    )
>  {
> @@ -145,15 +166,32 @@ FspMemoryInitDone (
>    // Calling use FspMemoryInit API
>    // Update HOB and return the control directly
>    //
> +  if (HobListPtr == NULL) {
> +    HobListPtr = (VOID **)GetFspApiParameter2 ();
> +  }
>    if (HobListPtr != NULL) {
>      *HobListPtr = (VOID *) GetHobList ();
>    }
> -
> +  //
> +  // Convert to FSP EAS defined API return codes
> +  //
> +  switch (Status) {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +    case EFI_OUT_OF_RESOURCES:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() Invalid Error
> [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
>    //
>    // This is the end of the FspMemoryInit API
>    // Give control back to the boot loader
>    //
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() - End\n"));
> +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() - [Status: 0x%08X]
> - End\n", Status));
>    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_MEMORY_INIT_EXIT);
>    FspData = GetFspGlobalDataPointer ();
>    PERF_START_EX(&gFspPerformanceDataGuid, "EventRec", NULL, (FspData-
> >PerfData[0] & FSP_PERFORMANCE_DATA_TIMER_MASK),
> FSP_STATUS_CODE_TEMP_RAM_INIT | FSP_STATUS_CODE_COMMON_CODE|
> FSP_STATUS_CODE_API_ENTRY);
> @@ -161,8 +199,13 @@ FspMemoryInitDone (
>    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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -  Pei2LoaderSwitchStack ();
> +  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);
> 
>    //
>    // The TempRamExitApi is called
> @@ -185,25 +228,44 @@ FspMemoryInitDone (
>  /**
>    This function returns control to BootLoader after TempRamExitApi.
> 
> +  @param[in] Status return status for the TempRamExitApi.
> +
>  **/
>  VOID
>  EFIAPI
> -FspTempRamExitDone (
> -  VOID
> +FspTempRamExitDone2 (
> +  IN EFI_STATUS Status
>    )
>  {
> -
> +  //
> +  // Convert to FSP EAS defined API return codes
> +  //
> +  switch (Status) {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() Invalid Error -
> [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
>    //
>    // This is the end of the TempRamExit API
>    // Give control back to the boot loader
>    //
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() - End\n"));
> +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() - [Status: 0x%08X]
> - End\n", Status));
>    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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -  Pei2LoaderSwitchStack ();
> -
> +  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);
>    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);
> @@ -266,9 +328,7 @@ FspWaitForNotify (
>        }
>      }
> 
> -    SetFspApiReturnStatus(Status);
>      DEBUG ((DEBUG_INFO | DEBUG_INIT, "NotifyPhaseApi() - End  [Status:
> 0x%08X]\n", Status));
> -
>      SetFspMeasurePoint (FSP_PERF_ID_API_NOTIFY_POST_PCI_EXIT + Count);
> 
>      if ((NotificationCount - 1) == 0) {
> @@ -281,7 +341,13 @@ 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);
>      }
> -    Pei2LoaderSwitchStack();
> +    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);
>    }
> 
>    //
> @@ -290,3 +356,42 @@ FspWaitForNotify (
>    //
>  }
> 
> +/**
> +  This function transfer control back to BootLoader after FspSiliconInit.
> +
> +**/
> +VOID
> +EFIAPI
> +FspSiliconInitDone (
> +  VOID
> +  )
> +{
> +  FspSiliconInitDone2 (EFI_SUCCESS);
> +}
> +
> +/**
> +  This function returns control to BootLoader after MemoryInitApi.
> +
> +  @param[in,out] HobListPtr The address of HobList pointer.
> +**/
> +VOID
> +EFIAPI
> +FspMemoryInitDone (
> +  IN OUT VOID   **HobListPtr
> +  )
> +{
> +  FspMemoryInitDone2 (EFI_SUCCESS, HobListPtr);
> +}
> +
> +/**
> +  This function returns control to BootLoader after TempRamExitApi.
> +
> +**/
> +VOID
> +EFIAPI
> +FspTempRamExitDone (
> +  VOID
> +  )
> +{
> +  FspTempRamExitDone2 (EFI_SUCCESS);
> +}
> --
> 2.9.0.windows.1



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done
  2016-11-08 22:39 ` [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done Mudusuru, Giri P
@ 2016-11-09  6:31   ` Thomaiyar, Richard Marian
  2016-11-10  5:11   ` Mudusuru, Giri P
  1 sibling, 0 replies; 4+ messages in thread
From: Thomaiyar, Richard Marian @ 2016-11-09  6:31 UTC (permalink / raw)
  To: Mudusuru, Giri P, edk2-devel@lists.01.org; +Cc: Ma, Maurice, Yao, Jiewen

The debug message will be displayed only when FSP API returned error status, and when Boot Loader called the subsequent FSP API again. The idea here is to return the control back to Boot Loader if there is any error in the previous API Call, without proceeding further and not having a hang inside the FSP. 

Note: Regular FSP API call status will be displayed in debug message, before returning the control back to the Boot Loader. 
> +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() - [Status: 
> + 0x%08X] -
> End\n", Status));

Regards, 
Richard

-----Original Message-----
From: Mudusuru, Giri P 
Sent: Wednesday, November 9, 2016 4:09 AM
To: Thomaiyar, Richard Marian <richard.marian.thomaiyar@intel.com>; edk2-devel@lists.01.org
Cc: Ma, Maurice <maurice.ma@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: RE: [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done

Hi Richard,

In the below snippet the debug message is based on the API status and will not be display if the switch stack already happened right? Also since it is in a do while loop "cannot proceed further" is not necessarily accurate.

> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -
> -  Pei2LoaderSwitchStack();
> -
> +  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);
Thanks,
-Giri

> -----Original Message-----
> From: Thomaiyar, Richard Marian
> Sent: Tuesday, November 8, 2016 1:45 AM
> To: edk2-devel@lists.01.org
> Cc: Ma, Maurice <maurice.ma@intel.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Mudusuru, Giri P <giri.p.mudusuru@intel.com>; 
> Thomaiyar, Richard Marian <richard.marian.thomaiyar@intel.com>
> Subject: [PATCH] IntelFsp2Pkg: Support to return error status from FSP 
> API done
> 
> Added FspMemoryInitDone2, FspTempRamExitDone2, FspSiliconInitDone2 to 
> return error status to Boot Loader for FSP API calls.
> To maintain backward compatibility existing functions 
> (FspMemoryInitDone, FspTempRamExitDone, FspSiliconInitDone) 
> declaration left untouched.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Richard Thomaiyar <richard.marian.thomaiyar@intel.com>
> ---
>  IntelFsp2Pkg/Include/Library/FspPlatformLib.h      |  35 +++++
>  .../Library/BaseFspPlatformLib/FspPlatformNotify.c | 151 
> +++++++++++++++++-
> ---
>  2 files changed, 163 insertions(+), 23 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> index 61e77bd..1f39601 100644
> --- a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> +++ b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> @@ -93,4 +93,39 @@ FspWaitForNotify (
>    VOID
>    );
> 
> +/**
> +  This function transfer control back to BootLoader after FspSiliconInit.
> +
> +  @param[in] Status return status for the FspSiliconInit.
> +**/
> +VOID
> +EFIAPI
> +FspSiliconInitDone2 (
> +  IN EFI_STATUS Status
> +  );
> +
> +/**
> +  This function returns control to BootLoader after MemoryInitApi.
> +
> +  @param[in] Status return status for the MemoryInitApi.
> +  @param[in,out] HobListPtr The address of HobList pointer.
> +**/
> +VOID
> +EFIAPI
> +FspMemoryInitDone2 (
> +  IN EFI_STATUS Status,
> +  IN OUT VOID   **HobListPtr
> +  );
> +
> +/**
> +  This function returns control to BootLoader after TempRamExitApi.
> +
> +  @param[in] Status return status for the TempRamExitApi.
> +**/
> +VOID
> +EFIAPI
> +FspTempRamExitDone2 (
> +  IN EFI_STATUS Status
> +  );
> +
>  #endif
> diff --git 
> a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> index 755e84f..ac1fc1a 100644
> --- a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> +++ b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> @@ -108,35 +108,56 @@ FspNotificationHandler (
>  /**
>    This function transfer control back to BootLoader after FspSiliconInit.
> 
> +  @param[in] Status return status for the FspSiliconInit.
> +
>  **/
>  VOID
>  EFIAPI
> -FspSiliconInitDone (
> -  VOID
> +FspSiliconInitDone2 (
> +  IN EFI_STATUS Status
>    )
>  {
>    //
> +  // Convert to FSP EAS defined API return codes  //  switch (Status) 
> + {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() Invalid 
> + Error -
> [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
> +  //
>    // This is the end of the FspSiliconInit API
>    // Give control back to the boot loader
>    //
>    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_SILICON_INIT_EXIT);
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() - End\n"));
> +  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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -
> -  Pei2LoaderSwitchStack();
> -
> +  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);
>  }
> 
>  /**
>    This function returns control to BootLoader after MemoryInitApi.
> 
> -  @param[in,out] HobListPtr The address of HobList pointer.
> +  @param[in] Status return status for the MemoryInitApi.
> +  @param[in,out] HobListPtr The address of HobList pointer, if NULL, 
> + will get
> value from GetFspApiParameter2 ()
>  **/
>  VOID
>  EFIAPI
> -FspMemoryInitDone (
> +FspMemoryInitDone2 (
> +  IN EFI_STATUS Status,
>    IN OUT VOID   **HobListPtr
>    )
>  {
> @@ -145,15 +166,32 @@ FspMemoryInitDone (
>    // Calling use FspMemoryInit API
>    // Update HOB and return the control directly
>    //
> +  if (HobListPtr == NULL) {
> +    HobListPtr = (VOID **)GetFspApiParameter2 ();  }
>    if (HobListPtr != NULL) {
>      *HobListPtr = (VOID *) GetHobList ();
>    }
> -
> +  //
> +  // Convert to FSP EAS defined API return codes  //  switch (Status) 
> + {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +    case EFI_OUT_OF_RESOURCES:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() Invalid 
> + Error
> [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
>    //
>    // This is the end of the FspMemoryInit API
>    // Give control back to the boot loader
>    //
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() - End\n"));
> +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() - [Status: 
> + 0x%08X]
> - End\n", Status));
>    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_MEMORY_INIT_EXIT);
>    FspData = GetFspGlobalDataPointer ();
>    PERF_START_EX(&gFspPerformanceDataGuid, "EventRec", NULL, (FspData-
> >PerfData[0] & FSP_PERFORMANCE_DATA_TIMER_MASK),
> FSP_STATUS_CODE_TEMP_RAM_INIT | FSP_STATUS_CODE_COMMON_CODE| 
> FSP_STATUS_CODE_API_ENTRY); @@ -161,8 +199,13 @@ FspMemoryInitDone (
>    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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -  Pei2LoaderSwitchStack ();
> +  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);
> 
>    //
>    // The TempRamExitApi is called
> @@ -185,25 +228,44 @@ FspMemoryInitDone (
>  /**
>    This function returns control to BootLoader after TempRamExitApi.
> 
> +  @param[in] Status return status for the TempRamExitApi.
> +
>  **/
>  VOID
>  EFIAPI
> -FspTempRamExitDone (
> -  VOID
> +FspTempRamExitDone2 (
> +  IN EFI_STATUS Status
>    )
>  {
> -
> +  //
> +  // Convert to FSP EAS defined API return codes  //  switch (Status) 
> + {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() Invalid 
> + Error -
> [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
>    //
>    // This is the end of the TempRamExit API
>    // Give control back to the boot loader
>    //
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() - End\n"));
> +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() - [Status: 
> + 0x%08X]
> - End\n", Status));
>    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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -  Pei2LoaderSwitchStack ();
> -
> +  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);
>    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); @@ -266,9 +328,7 @@ FspWaitForNotify (
>        }
>      }
> 
> -    SetFspApiReturnStatus(Status);
>      DEBUG ((DEBUG_INFO | DEBUG_INIT, "NotifyPhaseApi() - End  [Status:
> 0x%08X]\n", Status));
> -
>      SetFspMeasurePoint (FSP_PERF_ID_API_NOTIFY_POST_PCI_EXIT + 
> Count);
> 
>      if ((NotificationCount - 1) == 0) { @@ -281,7 +341,13 @@ 
> 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);
>      }
> -    Pei2LoaderSwitchStack();
> +    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);
>    }
> 
>    //
> @@ -290,3 +356,42 @@ FspWaitForNotify (
>    //
>  }
> 
> +/**
> +  This function transfer control back to BootLoader after FspSiliconInit.
> +
> +**/
> +VOID
> +EFIAPI
> +FspSiliconInitDone (
> +  VOID
> +  )
> +{
> +  FspSiliconInitDone2 (EFI_SUCCESS);
> +}
> +
> +/**
> +  This function returns control to BootLoader after MemoryInitApi.
> +
> +  @param[in,out] HobListPtr The address of HobList pointer.
> +**/
> +VOID
> +EFIAPI
> +FspMemoryInitDone (
> +  IN OUT VOID   **HobListPtr
> +  )
> +{
> +  FspMemoryInitDone2 (EFI_SUCCESS, HobListPtr); }
> +
> +/**
> +  This function returns control to BootLoader after TempRamExitApi.
> +
> +**/
> +VOID
> +EFIAPI
> +FspTempRamExitDone (
> +  VOID
> +  )
> +{
> +  FspTempRamExitDone2 (EFI_SUCCESS);
> +}
> --
> 2.9.0.windows.1



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done
  2016-11-08 22:39 ` [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done Mudusuru, Giri P
  2016-11-09  6:31   ` Thomaiyar, Richard Marian
@ 2016-11-10  5:11   ` Mudusuru, Giri P
  1 sibling, 0 replies; 4+ messages in thread
From: Mudusuru, Giri P @ 2016-11-10  5:11 UTC (permalink / raw)
  To: Mudusuru, Giri P, Thomaiyar, Richard Marian,
	edk2-devel@lists.01.org
  Cc: Yao, Jiewen

Thanks Richard. Can you please add comments in the code to describe it? Other than that it looks good to me.

Reviewed-by: Giri P Mudusuru <giri.p.mudusuru@intel.com>


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Mudusuru, Giri P
> Sent: Tuesday, November 8, 2016 2:39 PM
> To: Thomaiyar, Richard Marian <richard.marian.thomaiyar@intel.com>; edk2-
> devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2] [PATCH] IntelFsp2Pkg: Support to return error status from
> FSP API done
> 
> Hi Richard,
> 
> In the below snippet the debug message is based on the API status and will not
> be display if the switch stack already happened right? Also since it is in a do while
> loop "cannot proceed further" is not necessarily accurate.
> 
> > -  SetFspApiReturnStatus (EFI_SUCCESS);
> > -
> > -  Pei2LoaderSwitchStack();
> > -
> > +  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);
> Thanks,
> -Giri
> 
> > -----Original Message-----
> > From: Thomaiyar, Richard Marian
> > Sent: Tuesday, November 8, 2016 1:45 AM
> > To: edk2-devel@lists.01.org
> > Cc: Ma, Maurice <maurice.ma@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Mudusuru, Giri P <giri.p.mudusuru@intel.com>;
> > Thomaiyar, Richard Marian <richard.marian.thomaiyar@intel.com>
> > Subject: [PATCH] IntelFsp2Pkg: Support to return error status from FSP API
> done
> >
> > Added FspMemoryInitDone2, FspTempRamExitDone2, FspSiliconInitDone2
> > to return error status to Boot Loader for FSP API calls.
> > To maintain backward compatibility existing functions
> > (FspMemoryInitDone, FspTempRamExitDone, FspSiliconInitDone)
> > declaration left untouched.
> >
> > Cc: Maurice Ma <maurice.ma@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Richard Thomaiyar <richard.marian.thomaiyar@intel.com>
> > ---
> >  IntelFsp2Pkg/Include/Library/FspPlatformLib.h      |  35 +++++
> >  .../Library/BaseFspPlatformLib/FspPlatformNotify.c | 151
> +++++++++++++++++-
> > ---
> >  2 files changed, 163 insertions(+), 23 deletions(-)
> >
> > diff --git a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> > b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> > index 61e77bd..1f39601 100644
> > --- a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> > +++ b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> > @@ -93,4 +93,39 @@ FspWaitForNotify (
> >    VOID
> >    );
> >
> > +/**
> > +  This function transfer control back to BootLoader after FspSiliconInit.
> > +
> > +  @param[in] Status return status for the FspSiliconInit.
> > +**/
> > +VOID
> > +EFIAPI
> > +FspSiliconInitDone2 (
> > +  IN EFI_STATUS Status
> > +  );
> > +
> > +/**
> > +  This function returns control to BootLoader after MemoryInitApi.
> > +
> > +  @param[in] Status return status for the MemoryInitApi.
> > +  @param[in,out] HobListPtr The address of HobList pointer.
> > +**/
> > +VOID
> > +EFIAPI
> > +FspMemoryInitDone2 (
> > +  IN EFI_STATUS Status,
> > +  IN OUT VOID   **HobListPtr
> > +  );
> > +
> > +/**
> > +  This function returns control to BootLoader after TempRamExitApi.
> > +
> > +  @param[in] Status return status for the TempRamExitApi.
> > +**/
> > +VOID
> > +EFIAPI
> > +FspTempRamExitDone2 (
> > +  IN EFI_STATUS Status
> > +  );
> > +
> >  #endif
> > diff --git a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> > b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> > index 755e84f..ac1fc1a 100644
> > --- a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> > +++ b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> > @@ -108,35 +108,56 @@ FspNotificationHandler (
> >  /**
> >    This function transfer control back to BootLoader after FspSiliconInit.
> >
> > +  @param[in] Status return status for the FspSiliconInit.
> > +
> >  **/
> >  VOID
> >  EFIAPI
> > -FspSiliconInitDone (
> > -  VOID
> > +FspSiliconInitDone2 (
> > +  IN EFI_STATUS Status
> >    )
> >  {
> >    //
> > +  // Convert to FSP EAS defined API return codes
> > +  //
> > +  switch (Status) {
> > +    case EFI_SUCCESS:
> > +    case EFI_INVALID_PARAMETER:
> > +    case EFI_UNSUPPORTED:
> > +    case EFI_DEVICE_ERROR:
> > +      break;
> > +    default:
> > +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() Invalid Error -
> > [Status: 0x%08X]\n", Status));
> > +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> > +      break;
> > +  }
> > +  //
> >    // This is the end of the FspSiliconInit API
> >    // Give control back to the boot loader
> >    //
> >    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_SILICON_INIT_EXIT);
> > -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() - End\n"));
> > +  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);
> > -  SetFspApiReturnStatus (EFI_SUCCESS);
> > -
> > -  Pei2LoaderSwitchStack();
> > -
> > +  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);
> >  }
> >
> >  /**
> >    This function returns control to BootLoader after MemoryInitApi.
> >
> > -  @param[in,out] HobListPtr The address of HobList pointer.
> > +  @param[in] Status return status for the MemoryInitApi.
> > +  @param[in,out] HobListPtr The address of HobList pointer, if NULL, will get
> > value from GetFspApiParameter2 ()
> >  **/
> >  VOID
> >  EFIAPI
> > -FspMemoryInitDone (
> > +FspMemoryInitDone2 (
> > +  IN EFI_STATUS Status,
> >    IN OUT VOID   **HobListPtr
> >    )
> >  {
> > @@ -145,15 +166,32 @@ FspMemoryInitDone (
> >    // Calling use FspMemoryInit API
> >    // Update HOB and return the control directly
> >    //
> > +  if (HobListPtr == NULL) {
> > +    HobListPtr = (VOID **)GetFspApiParameter2 ();
> > +  }
> >    if (HobListPtr != NULL) {
> >      *HobListPtr = (VOID *) GetHobList ();
> >    }
> > -
> > +  //
> > +  // Convert to FSP EAS defined API return codes
> > +  //
> > +  switch (Status) {
> > +    case EFI_SUCCESS:
> > +    case EFI_INVALID_PARAMETER:
> > +    case EFI_UNSUPPORTED:
> > +    case EFI_DEVICE_ERROR:
> > +    case EFI_OUT_OF_RESOURCES:
> > +      break;
> > +    default:
> > +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() Invalid Error
> > [Status: 0x%08X]\n", Status));
> > +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> > +      break;
> > +  }
> >    //
> >    // This is the end of the FspMemoryInit API
> >    // Give control back to the boot loader
> >    //
> > -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() - End\n"));
> > +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() - [Status:
> 0x%08X]
> > - End\n", Status));
> >    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_MEMORY_INIT_EXIT);
> >    FspData = GetFspGlobalDataPointer ();
> >    PERF_START_EX(&gFspPerformanceDataGuid, "EventRec", NULL, (FspData-
> > >PerfData[0] & FSP_PERFORMANCE_DATA_TIMER_MASK),
> > FSP_STATUS_CODE_TEMP_RAM_INIT |
> FSP_STATUS_CODE_COMMON_CODE|
> > FSP_STATUS_CODE_API_ENTRY);
> > @@ -161,8 +199,13 @@ FspMemoryInitDone (
> >    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);
> > -  SetFspApiReturnStatus (EFI_SUCCESS);
> > -  Pei2LoaderSwitchStack ();
> > +  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);
> >
> >    //
> >    // The TempRamExitApi is called
> > @@ -185,25 +228,44 @@ FspMemoryInitDone (
> >  /**
> >    This function returns control to BootLoader after TempRamExitApi.
> >
> > +  @param[in] Status return status for the TempRamExitApi.
> > +
> >  **/
> >  VOID
> >  EFIAPI
> > -FspTempRamExitDone (
> > -  VOID
> > +FspTempRamExitDone2 (
> > +  IN EFI_STATUS Status
> >    )
> >  {
> > -
> > +  //
> > +  // Convert to FSP EAS defined API return codes
> > +  //
> > +  switch (Status) {
> > +    case EFI_SUCCESS:
> > +    case EFI_INVALID_PARAMETER:
> > +    case EFI_UNSUPPORTED:
> > +    case EFI_DEVICE_ERROR:
> > +      break;
> > +    default:
> > +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() Invalid Error -
> > [Status: 0x%08X]\n", Status));
> > +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> > +      break;
> > +  }
> >    //
> >    // This is the end of the TempRamExit API
> >    // Give control back to the boot loader
> >    //
> > -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() - End\n"));
> > +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() - [Status:
> 0x%08X]
> > - End\n", Status));
> >    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);
> > -  SetFspApiReturnStatus (EFI_SUCCESS);
> > -  Pei2LoaderSwitchStack ();
> > -
> > +  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);
> >    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);
> > @@ -266,9 +328,7 @@ FspWaitForNotify (
> >        }
> >      }
> >
> > -    SetFspApiReturnStatus(Status);
> >      DEBUG ((DEBUG_INFO | DEBUG_INIT, "NotifyPhaseApi() - End  [Status:
> > 0x%08X]\n", Status));
> > -
> >      SetFspMeasurePoint (FSP_PERF_ID_API_NOTIFY_POST_PCI_EXIT + Count);
> >
> >      if ((NotificationCount - 1) == 0) {
> > @@ -281,7 +341,13 @@ 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);
> >      }
> > -    Pei2LoaderSwitchStack();
> > +    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);
> >    }
> >
> >    //
> > @@ -290,3 +356,42 @@ FspWaitForNotify (
> >    //
> >  }
> >
> > +/**
> > +  This function transfer control back to BootLoader after FspSiliconInit.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +FspSiliconInitDone (
> > +  VOID
> > +  )
> > +{
> > +  FspSiliconInitDone2 (EFI_SUCCESS);
> > +}
> > +
> > +/**
> > +  This function returns control to BootLoader after MemoryInitApi.
> > +
> > +  @param[in,out] HobListPtr The address of HobList pointer.
> > +**/
> > +VOID
> > +EFIAPI
> > +FspMemoryInitDone (
> > +  IN OUT VOID   **HobListPtr
> > +  )
> > +{
> > +  FspMemoryInitDone2 (EFI_SUCCESS, HobListPtr);
> > +}
> > +
> > +/**
> > +  This function returns control to BootLoader after TempRamExitApi.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +FspTempRamExitDone (
> > +  VOID
> > +  )
> > +{
> > +  FspTempRamExitDone2 (EFI_SUCCESS);
> > +}
> > --
> > 2.9.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done
       [not found] <20161108094525.35036-1-richard.marian.thomaiyar@intel.com>
  2016-11-08 22:39 ` [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done Mudusuru, Giri P
@ 2016-11-10  5:23 ` Yao, Jiewen
  1 sibling, 0 replies; 4+ messages in thread
From: Yao, Jiewen @ 2016-11-10  5:23 UTC (permalink / raw)
  To: Thomaiyar, Richard Marian, edk2-devel@lists.01.org

Reivewed-by: jiewen.yao@intel.com

> -----Original Message-----
> From: Thomaiyar, Richard Marian
> Sent: Tuesday, November 8, 2016 5:45 PM
> To: edk2-devel@lists.01.org
> Cc: Ma, Maurice <maurice.ma@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Mudusuru, Giri P <giri.p.mudusuru@intel.com>;
> Thomaiyar, Richard Marian <richard.marian.thomaiyar@intel.com>
> Subject: [PATCH] IntelFsp2Pkg: Support to return error status from FSP API
> done
> 
> Added FspMemoryInitDone2, FspTempRamExitDone2, FspSiliconInitDone2
> to return error status to Boot Loader for FSP API calls.
> To maintain backward compatibility existing functions
> (FspMemoryInitDone, FspTempRamExitDone, FspSiliconInitDone)
> declaration left untouched.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Richard Thomaiyar <richard.marian.thomaiyar@intel.com>
> ---
>  IntelFsp2Pkg/Include/Library/FspPlatformLib.h      |  35 +++++
>  .../Library/BaseFspPlatformLib/FspPlatformNotify.c | 151
> +++++++++++++++++----
>  2 files changed, 163 insertions(+), 23 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> index 61e77bd..1f39601 100644
> --- a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> +++ b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> @@ -93,4 +93,39 @@ FspWaitForNotify (
>    VOID
>    );
> 
> +/**
> +  This function transfer control back to BootLoader after FspSiliconInit.
> +
> +  @param[in] Status return status for the FspSiliconInit.
> +**/
> +VOID
> +EFIAPI
> +FspSiliconInitDone2 (
> +  IN EFI_STATUS Status
> +  );
> +
> +/**
> +  This function returns control to BootLoader after MemoryInitApi.
> +
> +  @param[in] Status return status for the MemoryInitApi.
> +  @param[in,out] HobListPtr The address of HobList pointer.
> +**/
> +VOID
> +EFIAPI
> +FspMemoryInitDone2 (
> +  IN EFI_STATUS Status,
> +  IN OUT VOID   **HobListPtr
> +  );
> +
> +/**
> +  This function returns control to BootLoader after TempRamExitApi.
> +
> +  @param[in] Status return status for the TempRamExitApi.
> +**/
> +VOID
> +EFIAPI
> +FspTempRamExitDone2 (
> +  IN EFI_STATUS Status
> +  );
> +
>  #endif
> diff --git a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> index 755e84f..ac1fc1a 100644
> --- a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> +++ b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c
> @@ -108,35 +108,56 @@ FspNotificationHandler (
>  /**
>    This function transfer control back to BootLoader after FspSiliconInit.
> 
> +  @param[in] Status return status for the FspSiliconInit.
> +
>  **/
>  VOID
>  EFIAPI
> -FspSiliconInitDone (
> -  VOID
> +FspSiliconInitDone2 (
> +  IN EFI_STATUS Status
>    )
>  {
>    //
> +  // Convert to FSP EAS defined API return codes
> +  //
> +  switch (Status) {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() Invalid
> Error - [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
> +  //
>    // This is the end of the FspSiliconInit API
>    // Give control back to the boot loader
>    //
>    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_SILICON_INIT_EXIT);
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() - End\n"));
> +  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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -
> -  Pei2LoaderSwitchStack();
> -
> +  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);
>  }
> 
>  /**
>    This function returns control to BootLoader after MemoryInitApi.
> 
> -  @param[in,out] HobListPtr The address of HobList pointer.
> +  @param[in] Status return status for the MemoryInitApi.
> +  @param[in,out] HobListPtr The address of HobList pointer, if NULL, will
> get value from GetFspApiParameter2 ()
>  **/
>  VOID
>  EFIAPI
> -FspMemoryInitDone (
> +FspMemoryInitDone2 (
> +  IN EFI_STATUS Status,
>    IN OUT VOID   **HobListPtr
>    )
>  {
> @@ -145,15 +166,32 @@ FspMemoryInitDone (
>    // Calling use FspMemoryInit API
>    // Update HOB and return the control directly
>    //
> +  if (HobListPtr == NULL) {
> +    HobListPtr = (VOID **)GetFspApiParameter2 ();
> +  }
>    if (HobListPtr != NULL) {
>      *HobListPtr = (VOID *) GetHobList ();
>    }
> -
> +  //
> +  // Convert to FSP EAS defined API return codes
> +  //
> +  switch (Status) {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +    case EFI_OUT_OF_RESOURCES:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() Invalid
> Error [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
>    //
>    // This is the end of the FspMemoryInit API
>    // Give control back to the boot loader
>    //
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() - End\n"));
> +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspMemoryInitApi() - [Status:
> 0x%08X] - End\n", Status));
>    SetFspMeasurePoint (FSP_PERF_ID_API_FSP_MEMORY_INIT_EXIT);
>    FspData = GetFspGlobalDataPointer ();
>    PERF_START_EX(&gFspPerformanceDataGuid, "EventRec", NULL,
> (FspData->PerfData[0] & FSP_PERFORMANCE_DATA_TIMER_MASK),
> FSP_STATUS_CODE_TEMP_RAM_INIT |
> FSP_STATUS_CODE_COMMON_CODE| FSP_STATUS_CODE_API_ENTRY);
> @@ -161,8 +199,13 @@ FspMemoryInitDone (
>    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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -  Pei2LoaderSwitchStack ();
> +  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);
> 
>    //
>    // The TempRamExitApi is called
> @@ -185,25 +228,44 @@ FspMemoryInitDone (
>  /**
>    This function returns control to BootLoader after TempRamExitApi.
> 
> +  @param[in] Status return status for the TempRamExitApi.
> +
>  **/
>  VOID
>  EFIAPI
> -FspTempRamExitDone (
> -  VOID
> +FspTempRamExitDone2 (
> +  IN EFI_STATUS Status
>    )
>  {
> -
> +  //
> +  // Convert to FSP EAS defined API return codes
> +  //
> +  switch (Status) {
> +    case EFI_SUCCESS:
> +    case EFI_INVALID_PARAMETER:
> +    case EFI_UNSUPPORTED:
> +    case EFI_DEVICE_ERROR:
> +      break;
> +    default:
> +      DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() Invalid
> Error - [Status: 0x%08X]\n", Status));
> +      Status = EFI_DEVICE_ERROR;  // Force to known error.
> +      break;
> +  }
>    //
>    // This is the end of the TempRamExit API
>    // Give control back to the boot loader
>    //
> -  DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() - End\n"));
> +  DEBUG ((DEBUG_INFO | DEBUG_INIT, "TempRamExitApi() - [Status:
> 0x%08X] - End\n", Status));
>    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);
> -  SetFspApiReturnStatus (EFI_SUCCESS);
> -  Pei2LoaderSwitchStack ();
> -
> +  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);
>    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);
> @@ -266,9 +328,7 @@ FspWaitForNotify (
>        }
>      }
> 
> -    SetFspApiReturnStatus(Status);
>      DEBUG ((DEBUG_INFO | DEBUG_INIT, "NotifyPhaseApi() - End
> [Status: 0x%08X]\n", Status));
> -
>      SetFspMeasurePoint (FSP_PERF_ID_API_NOTIFY_POST_PCI_EXIT +
> Count);
> 
>      if ((NotificationCount - 1) == 0) {
> @@ -281,7 +341,13 @@ 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);
>      }
> -    Pei2LoaderSwitchStack();
> +    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);
>    }
> 
>    //
> @@ -290,3 +356,42 @@ FspWaitForNotify (
>    //
>  }
> 
> +/**
> +  This function transfer control back to BootLoader after FspSiliconInit.
> +
> +**/
> +VOID
> +EFIAPI
> +FspSiliconInitDone (
> +  VOID
> +  )
> +{
> +  FspSiliconInitDone2 (EFI_SUCCESS);
> +}
> +
> +/**
> +  This function returns control to BootLoader after MemoryInitApi.
> +
> +  @param[in,out] HobListPtr The address of HobList pointer.
> +**/
> +VOID
> +EFIAPI
> +FspMemoryInitDone (
> +  IN OUT VOID   **HobListPtr
> +  )
> +{
> +  FspMemoryInitDone2 (EFI_SUCCESS, HobListPtr);
> +}
> +
> +/**
> +  This function returns control to BootLoader after TempRamExitApi.
> +
> +**/
> +VOID
> +EFIAPI
> +FspTempRamExitDone (
> +  VOID
> +  )
> +{
> +  FspTempRamExitDone2 (EFI_SUCCESS);
> +}
> --
> 2.9.0.windows.1



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-11-10  5:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20161108094525.35036-1-richard.marian.thomaiyar@intel.com>
2016-11-08 22:39 ` [PATCH] IntelFsp2Pkg: Support to return error status from FSP API done Mudusuru, Giri P
2016-11-09  6:31   ` Thomaiyar, Richard Marian
2016-11-10  5:11   ` Mudusuru, Giri P
2016-11-10  5:23 ` Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox