Good unit test result. Thank you Chasel. Here is my thought.
1) Can we change FspApiModeEnabled to FspMode? And we can define mode below: #define FSP_API_MODE 0 #define FSP_DISPATCH_MODE 1 2) Can we change gFspApiModePpiGuid to gFspApiModeEnablePpiGuid to make it clear? Other seems good. With above change, reviewed-by: Jiewen Yao <[email protected]> Thank you Yao Jiewen > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Chasel, Chiu > Sent: Thursday, October 18, 2018 11:16 AM > To: [email protected] > Cc: Yao, Jiewen <[email protected]> > Subject: [edk2] [PATCH v2] IntelFsp2Pkg: Support FSP Dispatch mode > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1241 > > Add support for both API (original mode) and DISPATCH mode: > 1. Add FspApiModeEnabled field from reserved byte of Global > Data Structure to tell which mode is selected by boot > loader. If boot loader invoking FSP-M API this field > will be initialized to 1, otherwise platform FSP should > set this field to 0 when initializing Global Data > Structure for DISPATCH mode. > 2. gFspApiModePpiGuid will be instaled when FSP running in API > mode and modules only for API mode should have this in depex. > 3. If it is DISPATCH mode, FSP will return to PEI dispatcher, > not directly return to boot loader. > 4. DISPATCH mode supports DXE NotifyPhase drivers so FSP > will not wait for PEI NotifyPhase callbacks, instead it > will install gFspReadyForNotifyPhasePpiGuid PPI for > platform to complete late initialization before transferring > to DXE. > > Test: Verified FSP API and DISPATCH modes on 2 internal > platforms and both boot successfully. > > Cc: Jiewen Yao <[email protected]> > Cc: Desimone Nathaniel L <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Chasel Chiu <[email protected]> > --- > IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c | 59 > +++++++++------- > IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf | 3 +- > IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf | 2 +- > IntelFsp2Pkg/FspSecCore/SecFsp.c | 6 +- > IntelFsp2Pkg/FspSecCore/SecMain.c | 5 ++ > IntelFsp2Pkg/Include/FspGlobalData.h | 5 +- > IntelFsp2Pkg/IntelFsp2Pkg.dec | 11 +++ > .../Library/BaseFspCommonLib/FspCommonLib.c | 22 +++--- > .../Library/BaseFspPlatformLib/FspPlatformNotify.c | 78 > +++++++++++++--------- > 9 files changed, 122 insertions(+), 69 deletions(-) > > diff --git a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c > b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c > index 52435fa0b2..0f8743ade2 100644 > --- a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c > +++ b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.c > @@ -48,6 +48,12 @@ CONST EFI_PEI_PPI_DESCRIPTOR gEndOfPeiSignalPpi > = { > NULL > }; > > +CONST EFI_PEI_PPI_DESCRIPTOR gFspReadyForNotifyPhasePpi = { > + (EFI_PEI_PPI_DESCRIPTOR_PPI | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), > + &gFspReadyForNotifyPhasePpiGuid, > + NULL > +}; > + > /** > > This function waits for FSP notify. > @@ -88,13 +94,15 @@ WaitForNotify ( > // > FspWaitForNotify (); > > - // > - // Should not come here > - // > - while (TRUE) { > - DEBUG ((DEBUG_ERROR, "No FSP API should be called after FSP is > DONE!\n")); > - SetFspApiReturnStatus (EFI_UNSUPPORTED); > - Pei2LoaderSwitchStack (); > + if (GetFspGlobalDataPointer ()->FspApiModeEnabled == TRUE) { > + // > + // Should not come here > + // > + while (TRUE) { > + DEBUG ((DEBUG_ERROR, "No FSP API should be called after FSP is > DONE!\n")); > + SetFspApiReturnStatus (EFI_UNSUPPORTED); > + Pei2LoaderSwitchStack (); > + } > } > > return EFI_SUCCESS; > @@ -121,22 +129,27 @@ FspNotifyPhasePeimEntryPoint ( > > DEBUG ((DEBUG_INFO | DEBUG_INIT, "The entry of > FspNotificationPeim\n")); > > - // > - // Locate old DXE IPL PPI > - // > - Status = PeiServicesLocatePpi ( > - &gEfiDxeIplPpiGuid, > - 0, > - &OldDescriptor, > - &OldDxeIplPpi > - ); > - ASSERT_EFI_ERROR (Status); > - > - // > - // Re-install the DXE IPL PPI to wait for notify > - // > - Status = PeiServicesReInstallPpi (OldDescriptor, &mInstallDxeIplPpi); > - ASSERT_EFI_ERROR (Status); > + if (GetFspGlobalDataPointer ()->FspApiModeEnabled == TRUE) { > + // > + // Locate old DXE IPL PPI > + // > + Status = PeiServicesLocatePpi ( > + &gEfiDxeIplPpiGuid, > + 0, > + &OldDescriptor, > + &OldDxeIplPpi > + ); > + ASSERT_EFI_ERROR (Status); > + > + // > + // Re-install the DXE IPL PPI to wait for notify > + // > + Status = PeiServicesReInstallPpi (OldDescriptor, &mInstallDxeIplPpi); > + ASSERT_EFI_ERROR (Status); > + } else { > + Status = PeiServicesInstallPpi (&gFspReadyForNotifyPhasePpi); > + ASSERT_EFI_ERROR (Status); > + } > > return EFI_SUCCESS; > } > diff --git a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf > b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf > index a0bc37580d..b0f6768cfa 100644 > --- a/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf > +++ b/IntelFsp2Pkg/FspNotifyPhase/FspNotifyPhasePeim.inf > @@ -1,7 +1,7 @@ > ## @file > # Component information file for the FSP notify phase PEI module. > # > -# Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved. > +# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved. > # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the > BSD License > # which accompanies this distribution. The full text of the license may > be found at > @@ -37,6 +37,7 @@ > [Ppis] > gEfiDxeIplPpiGuid ## PRODUCES > gEfiEndOfPeiSignalPpiGuid ## PRODUCES > + gFspReadyForNotifyPhasePpiGuid ## PRODUCES > > [Protocols] > gEfiPciEnumerationCompleteProtocolGuid ## PRODUCES > diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf > b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf > index c657862deb..3380c282df 100644 > --- a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf > +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf > @@ -65,4 +65,4 @@ > > [Ppis] > gEfiTemporaryRamSupportPpiGuid > ## PRODUCES > - > + gFspApiModePpiGuid > ## PRODUCES > diff --git a/IntelFsp2Pkg/FspSecCore/SecFsp.c > b/IntelFsp2Pkg/FspSecCore/SecFsp.c > index 3c9de354fd..70006c4f7e 100644 > --- a/IntelFsp2Pkg/FspSecCore/SecFsp.c > +++ b/IntelFsp2Pkg/FspSecCore/SecFsp.c > @@ -66,7 +66,6 @@ SecGetPlatformData ( > // The entries of platform information, together with the number of > them, > // reside in the bottom of stack, left untouched by normal stack > operation. > // > - > FspPlatformData->DataPtr = NULL; > FspPlatformData->MicrocodeRegionBase = 0; > FspPlatformData->MicrocodeRegionSize = 0; > @@ -140,7 +139,10 @@ FspGlobalDataInit ( > PeiFspData->CoreStack = BootLoaderStack; > PeiFspData->PerfIdx = 2; > PeiFspData->PerfSig = > FSP_PERFORMANCE_DATA_SIGNATURE; > - > + // > + // FspSecCore will be invoked in API mode only so here must be API > mode. > + // > + PeiFspData->FspApiModeEnabled = TRUE; > SetFspMeasurePoint (FSP_PERF_ID_API_FSP_MEMORY_INIT_ENTRY); > > // > diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c > b/IntelFsp2Pkg/FspSecCore/SecMain.c > index 5a43ba1967..beee784bd0 100644 > --- a/IntelFsp2Pkg/FspSecCore/SecMain.c > +++ b/IntelFsp2Pkg/FspSecCore/SecMain.c > @@ -20,6 +20,11 @@ EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI > gSecTemporaryRamSupportPpi = { > > EFI_PEI_PPI_DESCRIPTOR mPeiSecPlatformInformationPpi[] > = { > { > + EFI_PEI_PPI_DESCRIPTOR_PPI, > + &gFspApiModePpiGuid, > + NULL > + }, > + { > (EFI_PEI_PPI_DESCRIPTOR_PPI | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), > &gEfiTemporaryRamSupportPpiGuid, > &gSecTemporaryRamSupportPpi > diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h > b/IntelFsp2Pkg/Include/FspGlobalData.h > index 7de26606a7..ccc9ecd78a 100644 > --- a/IntelFsp2Pkg/Include/FspGlobalData.h > +++ b/IntelFsp2Pkg/Include/FspGlobalData.h > @@ -1,6 +1,6 @@ > /** @file > > - Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the > BSD License > which accompanies this distribution. The full text of the license may be > found at > @@ -54,7 +54,8 @@ typedef struct { > VOID *MemoryInitUpdPtr; > VOID *SiliconInitUpdPtr; > UINT8 ApiIdx; > - UINT8 Reserved3[31]; > + UINT8 FspApiModeEnabled; // 1: API mode; 0: > DISPATCH mode > + UINT8 Reserved3[30]; > UINT32 PerfSig; > UINT16 PerfLen; > UINT16 Reserved4; > diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dec b/IntelFsp2Pkg/IntelFsp2Pkg.dec > index b2ee4b6ef5..ed984886dd 100644 > --- a/IntelFsp2Pkg/IntelFsp2Pkg.dec > +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dec > @@ -43,6 +43,17 @@ > ## @libraryclass Provides FSP platform sec related actions. > FspSecPlatformLib|Include/Library/FspSecPlatformLib.h > > +[Ppis] > + # > + # PPI to indicate FSP is ready to enter notify phase > + # > + gFspReadyForNotifyPhasePpiGuid = { 0xcd167c1e, 0x6e0b, > 0x42b3, { 0x82, 0xf6, 0xe3, 0xe9, 0x6, 0x19, 0x98, 0x10}} > + > + # > + # PPI as dependency on some modules which only required for API mode > + # > + gFspApiModePpiGuid = { 0xa1eeab87, 0xc859, > 0x479d, {0x89, 0xb5, 0x14, 0x61, 0xf4, 0x06, 0x1a, 0x3e}} > + > [Guids] > # > # GUID defined in package > diff --git a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c > b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c > index d0128304e6..317531608d 100644 > --- a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c > +++ b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c > @@ -519,14 +519,16 @@ FspApiReturnStatusReset ( > > LoopUntilReset = TRUE; > DEBUG ((DEBUG_INFO, "FSP returning control to Bootloader with reset > required return status %x\n",FspResetType)); > - /// > - /// Below code is not an infinite loop.The control will go back to API > calling function in BootLoader each time BootLoader > - /// calls the FSP API without honoring the reset request by FSP > - /// > - do { > - SetFspApiReturnStatus ((EFI_STATUS)FspResetType); > - Pei2LoaderSwitchStack (); > - DEBUG ((DEBUG_ERROR, "!!!ERROR: FSP has requested BootLoader > for reset. But BootLoader has not honored the reset\n")); > - DEBUG ((DEBUG_ERROR, "!!!ERROR: Please add support in > BootLoader to honor the reset request from FSP\n")); > - } while (LoopUntilReset); > + if (GetFspGlobalDataPointer ()->FspApiModeEnabled == TRUE) { > + /// > + /// Below code is not an infinite loop.The control will go back to API > calling function in BootLoader each time BootLoader > + /// calls the FSP API without honoring the reset request by FSP > + /// > + do { > + SetFspApiReturnStatus ((EFI_STATUS)FspResetType); > + Pei2LoaderSwitchStack (); > + DEBUG ((DEBUG_ERROR, "!!!ERROR: FSP has requested BootLoader > for reset. But BootLoader has not honored the reset\n")); > + DEBUG ((DEBUG_ERROR, "!!!ERROR: Please add support in > BootLoader to honor the reset request from FSP\n")); > + } while (LoopUntilReset); > + } > } > diff --git a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c > b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c > index ac1fc1ac0b..8f695fc147 100644 > --- a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c > +++ b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformNotify.c > @@ -1,6 +1,6 @@ > /** @file > > - Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the > BSD License > which accompanies this distribution. The full text of the license may be > found at > @@ -117,6 +117,9 @@ FspSiliconInitDone2 ( > IN EFI_STATUS Status > ) > { > + volatile EFI_STATUS FspStatus; > + > + FspStatus = Status; > // > // Convert to FSP EAS defined API return codes > // > @@ -139,13 +142,15 @@ FspSiliconInitDone2 ( > DEBUG ((DEBUG_INFO | DEBUG_INIT, "FspSiliconInitApi() - [Status: > 0x%08X] - End\n", Status)); > PERF_END_EX (&gFspPerformanceDataGuid, "EventRec", NULL, 0, > FSP_STATUS_CODE_SILICON_INIT | FSP_STATUS_CODE_COMMON_CODE | > FSP_STATUS_CODE_API_EXIT); > REPORT_STATUS_CODE (EFI_PROGRESS_CODE, > FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT); > - do { > - SetFspApiReturnStatus (Status); > - Pei2LoaderSwitchStack (); > - if (Status != EFI_SUCCESS) { > - DEBUG ((DEBUG_ERROR, "!!!ERROR: FspSiliconInitApi() - [Status: > 0x%08X] - Error encountered during previous API and cannot proceed > further\n", Status)); > - } > - } while (Status != EFI_SUCCESS); > + if (GetFspGlobalDataPointer ()->FspApiModeEnabled == TRUE) { > + do { > + SetFspApiReturnStatus (Status); > + Pei2LoaderSwitchStack (); > + if (Status != EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "!!!ERROR: FspSiliconInitApi() - [Status: > 0x%08X] - Error encountered during previous API and cannot proceed > further\n", Status)); > + } > + } while (FspStatus != EFI_SUCCESS); > + } > } > > /** > @@ -162,6 +167,9 @@ FspMemoryInitDone2 ( > ) > { > FSP_GLOBAL_DATA *FspData; > + volatile EFI_STATUS FspStatus; > + > + FspStatus = Status; > // > // Calling use FspMemoryInit API > // Update HOB and return the control directly > @@ -199,13 +207,15 @@ FspMemoryInitDone2 ( > PERF_START_EX(&gFspPerformanceDataGuid, "EventRec", NULL, > (FspData->PerfData[2] & FSP_PERFORMANCE_DATA_TIMER_MASK), > FSP_STATUS_CODE_MEMORY_INIT | FSP_STATUS_CODE_COMMON_CODE | > FSP_STATUS_CODE_API_ENTRY); > PERF_END_EX(&gFspPerformanceDataGuid, "EventRec", NULL, 0, > FSP_STATUS_CODE_MEMORY_INIT | FSP_STATUS_CODE_COMMON_CODE | > FSP_STATUS_CODE_API_EXIT); > REPORT_STATUS_CODE (EFI_PROGRESS_CODE, > FSP_STATUS_CODE_MEMORY_INIT | FSP_STATUS_CODE_COMMON_CODE | > FSP_STATUS_CODE_API_EXIT); > - do { > - SetFspApiReturnStatus (Status); > - Pei2LoaderSwitchStack (); > - if (Status != EFI_SUCCESS) { > - DEBUG ((DEBUG_ERROR, "!!!ERROR: FspMemoryInitApi() - [Status: > 0x%08X] - Error encountered during previous API and cannot proceed > further\n", Status)); > - } > - } while (Status != EFI_SUCCESS); > + if (GetFspGlobalDataPointer ()->FspApiModeEnabled == TRUE) { > + do { > + SetFspApiReturnStatus (Status); > + Pei2LoaderSwitchStack (); > + if (Status != EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "!!!ERROR: FspMemoryInitApi() - > [Status: 0x%08X] - Error encountered during previous API and cannot > proceed further\n", Status)); > + } > + } while (FspStatus != EFI_SUCCESS); > + } > > // > // The TempRamExitApi is called > @@ -238,6 +248,9 @@ FspTempRamExitDone2 ( > ) > { > // > + volatile EFI_STATUS FspStatus; > + > + FspStatus = Status; > // Convert to FSP EAS defined API return codes > // > switch (Status) { > @@ -259,13 +272,15 @@ FspTempRamExitDone2 ( > SetFspMeasurePoint (FSP_PERF_ID_API_TEMP_RAM_EXIT_EXIT); > PERF_END_EX(&gFspPerformanceDataGuid, "EventRec", NULL, 0, > FSP_STATUS_CODE_TEMP_RAM_EXIT | > FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT); > REPORT_STATUS_CODE (EFI_PROGRESS_CODE, > FSP_STATUS_CODE_TEMP_RAM_EXIT | > FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT); > - do { > - SetFspApiReturnStatus (Status); > - Pei2LoaderSwitchStack (); > - if (Status != EFI_SUCCESS) { > - DEBUG ((DEBUG_ERROR, "!!!ERROR: TempRamExitApi() - [Status: > 0x%08X] - Error encountered during previous API and cannot proceed > further\n", Status)); > - } > - } while (Status != EFI_SUCCESS); > + if (GetFspGlobalDataPointer ()->FspApiModeEnabled == TRUE) { > + do { > + SetFspApiReturnStatus (Status); > + Pei2LoaderSwitchStack (); > + if (Status != EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "!!!ERROR: TempRamExitApi() - [Status: > 0x%08X] - Error encountered during previous API and cannot proceed > further\n", Status)); > + } > + } while (FspStatus != EFI_SUCCESS); > + } > SetPhaseStatusCode (FSP_STATUS_CODE_SILICON_INIT); > SetFspMeasurePoint (FSP_PERF_ID_API_FSP_SILICON_INIT_ENTRY); > PERF_START_EX(&gFspPerformanceDataGuid, "EventRec", NULL, 0, > FSP_STATUS_CODE_SILICON_INIT | FSP_STATUS_CODE_COMMON_CODE | > FSP_STATUS_CODE_API_ENTRY); > @@ -289,6 +304,7 @@ FspWaitForNotify ( > UINT32 NotificationValue; > UINT32 NotificationCount; > UINT8 Count; > + volatile EFI_STATUS FspStatus; > > NotificationCount = 0; > while (NotificationCount < sizeof(mFspNotifySequence) / sizeof(UINT32)) > { > @@ -341,15 +357,17 @@ FspWaitForNotify ( > PERF_END_EX(&gFspPerformanceDataGuid, "EventRec", NULL, 0, > FSP_STATUS_CODE_END_OF_FIRMWARE_NOTIFICATION | > FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT); > REPORT_STATUS_CODE (EFI_PROGRESS_CODE, > FSP_STATUS_CODE_END_OF_FIRMWARE_NOTIFICATION | > FSP_STATUS_CODE_COMMON_CODE | FSP_STATUS_CODE_API_EXIT); > } > - do { > - SetFspApiReturnStatus(Status); > - Pei2LoaderSwitchStack(); > - if (Status != EFI_SUCCESS) { > - DEBUG ((DEBUG_ERROR, "!!!ERROR: NotifyPhaseApi() > [Phase: %08X] - Failed - [Status: 0x%08X]\n", NotificationValue, Status)); > - } > - } while (Status != EFI_SUCCESS); > + if (GetFspGlobalDataPointer ()->FspApiModeEnabled == TRUE) { > + FspStatus = Status; > + do { > + SetFspApiReturnStatus(Status); > + Pei2LoaderSwitchStack(); > + if (Status != EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "!!!ERROR: NotifyPhaseApi() > [Phase: %08X] - Failed - [Status: 0x%08X]\n", NotificationValue, Status)); > + } > + } while (FspStatus != EFI_SUCCESS); > + } > } > - > // > // Control goes back to the PEI Core and it dispatches further PEIMs. > // DXEIPL is the final one to transfer control back to the boot loader. > -- > 2.13.3.windows.1 > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
