Re: [PATCH] IntelFsp2Pkg: Add function to get bootloader stack pointer

["Zeng, Star" <star.zeng@intel.com>]

On 2019/1/23 13:36, Chasel, Chiu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1391
> 
> FSP on-going enhancement for stack utilization required
> to know bootloader stack pointer and this pointer can be
> retrieved by first input parameter address when FSP-M
> entry API invoked by bootloader.
> 
> Test: Verified on internal platform and booting successfully
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>

Minor comments below, with them addressed, Reviewed-by: Star Zeng 
<star.zeng@intel.com>.

> ---
>   IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c | 27 ++++++++++++++++++++++-----
>   IntelFsp2Pkg/Include/Library/FspCommonLib.h          | 21 ++++++++++++++++-----
>   2 files changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> index 64ffba1d16..2f2861281c 100644
> --- a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> +++ b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> @@ -1,6 +1,6 @@
>   /** @file
>   
> -  Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
>     This program and the accompanying materials
>     are licensed and made available under the terms and conditions of the BSD License
>     which accompanies this distribution.  The full text of the license may be found at
> @@ -87,9 +87,9 @@ GetFspGlobalDataPointer (
>   }
>   
>   /**
> -  This function gets back the FSP API first parameter passed by the bootlaoder.
> +  This function gets back the FSP API first parameter passed by the bootloader.
>   
> -  @retval ApiParameter FSP API first parameter passed by the bootlaoder.
> +  @retval ApiParameter FSP API first parameter passed by the bootloader.
>   **/
>   UINT32
>   EFIAPI
> @@ -104,9 +104,26 @@ GetFspApiParameter (
>   }
>   
>   /**
> -  This function gets back the FSP API second parameter passed by the bootlaoder.
> +  This function returns the FSP entry stack pointer from address of the first API parameter
>   
> -  @retval ApiParameter FSP API second parameter passed by the bootlaoder.
> +  @retval FSP entry stack pointer.
> +**/
> +VOID*
> +EFIAPI
> +GetFspEntryStack (
> +  VOID
> +  )
> +{
> +  FSP_GLOBAL_DATA  *FspData;
> +
> +  FspData  = GetFspGlobalDataPointer ();
> +  return (VOID*)(FspData->CoreStack + CONTEXT_STACK_OFFSET(ApiParam[0]));
> +}
> +
> +/**
> +  This function gets back the FSP API second parameter passed by the bootloader.
> +
> +  @retval ApiParameter FSP API second parameter passed by the bootloader.
>   **/
>   UINT32
>   EFIAPI
> diff --git a/IntelFsp2Pkg/Include/Library/FspCommonLib.h b/IntelFsp2Pkg/Include/Library/FspCommonLib.h
> index 1c99be3b1f..e98b180fca 100644
> --- a/IntelFsp2Pkg/Include/Library/FspCommonLib.h
> +++ b/IntelFsp2Pkg/Include/Library/FspCommonLib.h
> @@ -1,6 +1,6 @@
>   /** @file
>   
> -  Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
>     This program and the accompanying materials
>     are licensed and made available under the terms and conditions of the BSD License
>     which accompanies this distribution.  The full text of the license may be found at
> @@ -40,9 +40,9 @@ GetFspGlobalDataPointer (
>     );
>   
>   /**
> -  This function gets back the FSP API first parameter passed by the bootlaoder.
> +  This function gets back the FSP API first parameter passed by the bootloader.
>   
> -  @retval ApiParameter FSP API first parameter passed by the bootlaoder.
> +  @retval ApiParameter FSP API first parameter passed by the bootloader.
>   **/
>   UINT32
>   EFIAPI
> @@ -51,9 +51,9 @@ GetFspApiParameter (
>     );
>   
>   /**
> -  This function gets back the FSP API second parameter passed by the bootlaoder.
> +  This function gets back the FSP API second parameter passed by the bootloader.
>   
> -  @retval ApiParameter FSP API second parameter passed by the bootlaoder.
> +  @retval ApiParameter FSP API second parameter passed by the bootloader.
>   **/
>   UINT32
>   EFIAPI
> @@ -61,6 +61,17 @@ GetFspApiParameter2 (
>     VOID
>     );
>   
> +

Remove this new line?

> +/**
> +  This function returns the FSP entry stack pointer from address of the first API parameter
> +
> +  @retval FSP entry stack pointer.
> +**/
> +VOID*
> +EFIAPI
> +GetFspEntryStack (
> +  VOID
> +  );

Add a new line?


Thanks,
Star

>   /**
>     This function sets the FSP API parameter in the stack.
>   
>