public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] IntelFsp2WrapprPkg: Check header revision for MultiPhase support.
@ 2022-10-25 20:55 Chiu, Chasel
  2022-10-25 22:19 ` Nate DeSimone
  0 siblings, 1 reply; 3+ messages in thread
From: Chiu, Chasel @ 2022-10-25 20:55 UTC (permalink / raw)
  To: devel; +Cc: Chasel Chiu, Nate DeSimone, Star Zeng

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

Earlier version of FSP header may not have MultiPhase fields present in
the FspInfoHeader so the handler should verify header revision before
accessing the MultiPhase fields from the header.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c b/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c
index 1248accf87..0f524910d5 100644
--- a/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c
+++ b/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c
@@ -73,15 +73,27 @@ CallFspMultiPhaseEntry (
   //
   // FSP_MULTI_PHASE_INIT and FSP_MULTI_PHASE_SI_INIT API functions having same prototype.
   //
-  UINTN       FspMultiPhaseApiEntry;
-  UINTN       FspMultiPhaseApiOffset;
-  EFI_STATUS  Status;
-  BOOLEAN     InterruptState;
+  UINTN                   FspMultiPhaseApiEntry;
+  UINTN                   FspMultiPhaseApiOffset;
+  EFI_STATUS              Status;
+  BOOLEAN                 InterruptState;
+  BOOLEAN                 IsVariableServiceRequest;
+  FSP_MULTI_PHASE_PARAMS  *FspMultiPhaseParamsPtr;
+
+  FspMultiPhaseParamsPtr   = (FSP_MULTI_PHASE_PARAMS *)FspMultiPhaseParams;
+  IsVariableServiceRequest = FALSE;
+  if ((FspMultiPhaseParamsPtr->MultiPhaseAction == EnumMultiPhaseGetVariableRequestInfo) ||
+      (FspMultiPhaseParamsPtr->MultiPhaseAction == EnumMultiPhaseCompleteVariableRequest))
+  {
+    IsVariableServiceRequest = TRUE;
+  }
 
   if (ComponentIndex == FspMultiPhaseMemInitApiIndex) {
     FspHeader = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32 (PcdFspmBaseAddress));
     if (FspHeader == NULL) {
       return EFI_DEVICE_ERROR;
+    } else if (FspHeader->SpecVersion < 0x24) {
+      return EFI_UNSUPPORTED;
     }
 
     FspMultiPhaseApiOffset = FspHeader->FspMultiPhaseMemInitEntryOffset;
@@ -89,6 +101,10 @@ CallFspMultiPhaseEntry (
     FspHeader = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32 (PcdFspsBaseAddress));
     if (FspHeader == NULL) {
       return EFI_DEVICE_ERROR;
+    } else if (FspHeader->SpecVersion < 0x22) {
+      return EFI_UNSUPPORTED;
+    } else if ((FspHeader->SpecVersion < 0x24) && (IsVariableServiceRequest == TRUE)) {
+      return EFI_UNSUPPORTED;
     }
 
     FspMultiPhaseApiOffset = FspHeader->FspMultiPhaseSiInitEntryOffset;
@@ -120,7 +136,8 @@ CallFspMultiPhaseEntry (
   @param[in] FspHobListPtr        - Pointer to FSP HobList (valid after FSP-M completed)
   @param[in] ComponentIndex       - FSP Component which executing MultiPhase initialization.
 
-  @retval EFI_UNSUPPORTED   FSP Wrapper cannot support the specific variable request
+  @retval EFI_UNSUPPORTED   FSP Wrapper cannot support the specific variable request,
+                            or FSP does not support VariableService
   @retval EFI_STATUS        Return FSP returned status
 
 **/
@@ -287,7 +304,8 @@ FspWrapperVariableRequestHandler (
   @param[in] FspHobListPtr        - Pointer to FSP HobList (valid after FSP-M completed)
   @param[in] ComponentIndex       - FSP Component which executing MultiPhase initialization.
 
-  @retval EFI_STATUS        Always return EFI_SUCCESS
+  @retval EFI_UNSUPPORTED   Specific MultiPhase action was not supported.
+  @retval EFI_SUCCESS       MultiPhase action were completed successfully.
 
 **/
 EFI_STATUS
-- 
2.35.0.windows.1


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

* Re: [PATCH] IntelFsp2WrapprPkg: Check header revision for MultiPhase support.
  2022-10-25 20:55 [PATCH] IntelFsp2WrapprPkg: Check header revision for MultiPhase support Chiu, Chasel
@ 2022-10-25 22:19 ` Nate DeSimone
  2022-10-26  0:45   ` Chiu, Chasel
  0 siblings, 1 reply; 3+ messages in thread
From: Nate DeSimone @ 2022-10-25 22:19 UTC (permalink / raw)
  To: Chiu, Chasel, devel@edk2.groups.io; +Cc: Zeng, Star

Hi Chasel,

There is a spelling error in your commit message. IntelFsp2WrapprPkg should be IntelFsp2WrapperPkg. Please fix this during the push. No need to send a 2nd review.

With that change...

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

> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Tuesday, October 25, 2022 1:56 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH] IntelFsp2WrapprPkg: Check header revision for MultiPhase
> support.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4119
> 
> Earlier version of FSP header may not have MultiPhase fields present in the
> FspInfoHeader so the handler should verify header revision before accessing
> the MultiPhase fields from the header.
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c b/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c
> index 1248accf87..0f524910d5 100644
> --- a/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c
> +++ b/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c
> @@ -73,15 +73,27 @@ CallFspMultiPhaseEntry (
>    //
>    // FSP_MULTI_PHASE_INIT and FSP_MULTI_PHASE_SI_INIT API functions having same prototype.
>    //
> -  UINTN       FspMultiPhaseApiEntry;
> -  UINTN       FspMultiPhaseApiOffset;
> -  EFI_STATUS  Status;
> -  BOOLEAN     InterruptState;
> +  UINTN                   FspMultiPhaseApiEntry;
> +  UINTN                   FspMultiPhaseApiOffset;
> +  EFI_STATUS              Status;
> +  BOOLEAN                 InterruptState;
> +  BOOLEAN                 IsVariableServiceRequest;
> +  FSP_MULTI_PHASE_PARAMS  *FspMultiPhaseParamsPtr;
> +
> +  FspMultiPhaseParamsPtr   = (FSP_MULTI_PHASE_PARAMS *)FspMultiPhaseParams;
> +  IsVariableServiceRequest = FALSE;
> +  if ((FspMultiPhaseParamsPtr->MultiPhaseAction == EnumMultiPhaseGetVariableRequestInfo) ||
> +      (FspMultiPhaseParamsPtr->MultiPhaseAction == EnumMultiPhaseCompleteVariableRequest))
> +  {
> +    IsVariableServiceRequest = TRUE;
> +  }
>  
>    if (ComponentIndex == FspMultiPhaseMemInitApiIndex) {
>      FspHeader = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32 (PcdFspmBaseAddress));
>      if (FspHeader == NULL) {
>        return EFI_DEVICE_ERROR;
> +    } else if (FspHeader->SpecVersion < 0x24) {
> +      return EFI_UNSUPPORTED;
>      }
>  
>      FspMultiPhaseApiOffset = FspHeader->FspMultiPhaseMemInitEntryOffset;
> @@ -89,6 +101,10 @@ CallFspMultiPhaseEntry (
>      FspHeader = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32 (PcdFspsBaseAddress));
>      if (FspHeader == NULL) {
>        return EFI_DEVICE_ERROR;
> +    } else if (FspHeader->SpecVersion < 0x22) {
> +      return EFI_UNSUPPORTED;
> +    } else if ((FspHeader->SpecVersion < 0x24) && (IsVariableServiceRequest == TRUE)) {
> +      return EFI_UNSUPPORTED;
>      }
>  
>      FspMultiPhaseApiOffset = FspHeader->FspMultiPhaseSiInitEntryOffset;
> @@ -120,7 +136,8 @@ CallFspMultiPhaseEntry (
>    @param[in] FspHobListPtr        - Pointer to FSP HobList (valid after FSP-M completed)
>    @param[in] ComponentIndex       - FSP Component which executing MultiPhase initialization.
>  
> -  @retval EFI_UNSUPPORTED   FSP Wrapper cannot support the specific variable request
> +  @retval EFI_UNSUPPORTED   FSP Wrapper cannot support the specific variable request,
> +                            or FSP does not support VariableService
>    @retval EFI_STATUS        Return FSP returned status
>  
>  **/
> @@ -287,7 +304,8 @@ FspWrapperVariableRequestHandler (
>    @param[in] FspHobListPtr        - Pointer to FSP HobList (valid after FSP-M completed)
>    @param[in] ComponentIndex       - FSP Component which executing MultiPhase initialization.
>  
> -  @retval EFI_STATUS        Always return EFI_SUCCESS
> +  @retval EFI_UNSUPPORTED   Specific MultiPhase action was not supported.
> +  @retval EFI_SUCCESS       MultiPhase action were completed successfully.
>  
>  **/
>  EFI_STATUS
> -- 
> 2.35.0.windows.1

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

* Re: [PATCH] IntelFsp2WrapprPkg: Check header revision for MultiPhase support.
  2022-10-25 22:19 ` Nate DeSimone
@ 2022-10-26  0:45   ` Chiu, Chasel
  0 siblings, 0 replies; 3+ messages in thread
From: Chiu, Chasel @ 2022-10-26  0:45 UTC (permalink / raw)
  To: Desimone, Nathaniel L, devel@edk2.groups.io; +Cc: Zeng, Star


Patch merged: https://github.com/tianocore/edk2/commit/26638d2aa399e436f47d629f877e581ed402020a

Thanks,
Chasel


> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Tuesday, October 25, 2022 3:19 PM
> To: Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH] IntelFsp2WrapprPkg: Check header revision for
> MultiPhase support.
> 
> Hi Chasel,
> 
> There is a spelling error in your commit message. IntelFsp2WrapprPkg
> should be IntelFsp2WrapperPkg. Please fix this during the push. No need to
> send a 2nd review.
> 
> With that change...
> 
> Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> 
> > -----Original Message-----
> > From: Chiu, Chasel <chasel.chiu@intel.com>
> > Sent: Tuesday, October 25, 2022 1:56 PM
> > To: devel@edk2.groups.io
> > Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: [PATCH] IntelFsp2WrapprPkg: Check header revision for
> > MultiPhase support.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4119
> >
> > Earlier version of FSP header may not have MultiPhase fields present
> > in the FspInfoHeader so the handler should verify header revision
> > before accessing the MultiPhase fields from the header.
> >
> > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > ---
> >
> >
> IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWr
> app
> > erMultiPhaseProcessLib.c | 30 ++++++++++++++++++++++++------
> >  1 file changed, 24 insertions(+), 6 deletions(-)
> >
> > diff --git
> >
> a/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFsp
> Wra
> > pperMultiPhaseProcessLib.c
> >
> b/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFsp
> Wra
> > pperMultiPhaseProcessLib.c
> > index 1248accf87..0f524910d5 100644
> > ---
> >
> a/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFsp
> Wra
> > pperMultiPhaseProcessLib.c
> > +++
> b/IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFs
> > +++ pWrapperMultiPhaseProcessLib.c
> > @@ -73,15 +73,27 @@ CallFspMultiPhaseEntry (
> >    //
> >    // FSP_MULTI_PHASE_INIT and FSP_MULTI_PHASE_SI_INIT API
> functions having same prototype.
> >    //
> > -  UINTN       FspMultiPhaseApiEntry;
> > -  UINTN       FspMultiPhaseApiOffset;
> > -  EFI_STATUS  Status;
> > -  BOOLEAN     InterruptState;
> > +  UINTN                   FspMultiPhaseApiEntry;
> > +  UINTN                   FspMultiPhaseApiOffset;
> > +  EFI_STATUS              Status;
> > +  BOOLEAN                 InterruptState;
> > +  BOOLEAN                 IsVariableServiceRequest;
> > +  FSP_MULTI_PHASE_PARAMS  *FspMultiPhaseParamsPtr;
> > +
> > +  FspMultiPhaseParamsPtr   = (FSP_MULTI_PHASE_PARAMS
> *)FspMultiPhaseParams;
> > +  IsVariableServiceRequest = FALSE;
> > +  if ((FspMultiPhaseParamsPtr->MultiPhaseAction ==
> EnumMultiPhaseGetVariableRequestInfo) ||
> > +      (FspMultiPhaseParamsPtr->MultiPhaseAction ==
> > + EnumMultiPhaseCompleteVariableRequest))
> > +  {
> > +    IsVariableServiceRequest = TRUE;
> > +  }
> >
> >    if (ComponentIndex == FspMultiPhaseMemInitApiIndex) {
> >      FspHeader = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32
> (PcdFspmBaseAddress));
> >      if (FspHeader == NULL) {
> >        return EFI_DEVICE_ERROR;
> > +    } else if (FspHeader->SpecVersion < 0x24) {
> > +      return EFI_UNSUPPORTED;
> >      }
> >
> >      FspMultiPhaseApiOffset =
> > FspHeader->FspMultiPhaseMemInitEntryOffset;
> > @@ -89,6 +101,10 @@ CallFspMultiPhaseEntry (
> >      FspHeader = (FSP_INFO_HEADER *)FspFindFspHeader (PcdGet32
> (PcdFspsBaseAddress));
> >      if (FspHeader == NULL) {
> >        return EFI_DEVICE_ERROR;
> > +    } else if (FspHeader->SpecVersion < 0x22) {
> > +      return EFI_UNSUPPORTED;
> > +    } else if ((FspHeader->SpecVersion < 0x24) &&
> (IsVariableServiceRequest == TRUE)) {
> > +      return EFI_UNSUPPORTED;
> >      }
> >
> >      FspMultiPhaseApiOffset =
> > FspHeader->FspMultiPhaseSiInitEntryOffset;
> > @@ -120,7 +136,8 @@ CallFspMultiPhaseEntry (
> >    @param[in] FspHobListPtr        - Pointer to FSP HobList (valid after FSP-
> M completed)
> >    @param[in] ComponentIndex       - FSP Component which executing
> MultiPhase initialization.
> >
> > -  @retval EFI_UNSUPPORTED   FSP Wrapper cannot support the specific
> variable request
> > +  @retval EFI_UNSUPPORTED   FSP Wrapper cannot support the specific
> variable request,
> > +                            or FSP does not support VariableService
> >    @retval EFI_STATUS        Return FSP returned status
> >
> >  **/
> > @@ -287,7 +304,8 @@ FspWrapperVariableRequestHandler (
> >    @param[in] FspHobListPtr        - Pointer to FSP HobList (valid after FSP-
> M completed)
> >    @param[in] ComponentIndex       - FSP Component which executing
> MultiPhase initialization.
> >
> > -  @retval EFI_STATUS        Always return EFI_SUCCESS
> > +  @retval EFI_UNSUPPORTED   Specific MultiPhase action was not
> supported.
> > +  @retval EFI_SUCCESS       MultiPhase action were completed
> successfully.
> >
> >  **/
> >  EFI_STATUS
> > --
> > 2.35.0.windows.1

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

end of thread, other threads:[~2022-10-26  0:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-25 20:55 [PATCH] IntelFsp2WrapprPkg: Check header revision for MultiPhase support Chiu, Chasel
2022-10-25 22:19 ` Nate DeSimone
2022-10-26  0:45   ` Chiu, Chasel

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