Hi Chasel,
I have a few comments for you.
First, we should have a platform provided LibraryClass for running code in
between multi-phase actions. Right now you just have this comment in
IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c:
//
// Platform handling can be added here to take care specific actions for
each phase
// before returning control back to FSP.
//
I would like a new LibraryClass added: FspWrapperPlatformMultiPhaseLib
This would implement a single function:
VOID
EFIAPI
FspWrapperPlatformMultiPhaseHandler (
IN UINT8 ComponentIndex,
IN UINT32 PhaseIndex
);
Add an implementation of this in
IntelFsp2WrapperPkg/Library/BaseFspWrapperPlatformMultiPhaseLibSample. That
.inf will provide an implementation of FspWrapperPlatformMultiPhaseHandler()
that doesn't do anything (just leave it empty).
Then, invoke FspWrapperPlatformMultiPhaseHandler() at the point where you have
that comment above in FspWrapperMultiPhaseHandler().
The *BoardPkg can provide an SOC specific implementation. Typically the real
*BoardPkg implementation will look something like this:
VOID
EFIAPI
FspWrapperPlatformMultiPhaseHandler (
IN UINT8 ComponentIndex,
IN UINT32 PhaseIndex
)
{
switch (ComponentIndex) {
case FspMultiPhaseMemInitApiIndex:
switch (PhaseIndex) {
case 1:
PeiServicesInstallPpi (mSomePlatformSpecificNotifyPpi1);
break;
}
break;
case FspMultiPhaseSiInitApiIndex:
switch (PhaseIndex) {
case 1:
PeiServicesInstallPpi (mSomePlatformSpecificNotifyPpi2);
break;
}
break;
}
}
The exact specifics would vary by SOC design and are out of scope for this
patch series. But you do need to provide the base case of "do nothing" in
IntelFsp2WrapperPkg.
Second, in
IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c:
case EnumFspVariableRequestSetVariable:
if (WriteVariableSupport) {
Status = VariablePpi->SetVariable (
VariablePpi,
FspVariableRequestParams->VariableName,
FspVariableRequestParams->VariableGuid,
*FspVariableRequestParams->Attributes,
(UINTN)*FspVariableRequestParams->DataSize,
FspVariableRequestParams->Data
);
} else {
return EFI_UNSUPPORTED;
}
Instead of return EFI_UNSUPPORTED; it should be Status = EFI_UNSUPPORTED;. Same
thing with EnumFspVariableRequestQueryVariableInfo.
Third, in FspWrapperVariableRequestHandler(), after you call
EnumMultiPhaseCompleteVariableRequest you need to check if one of the
FSP_STATUS_RESET_REQUIRED_* status codes is returned and if so invoke
CallFspWrapperResetSystem().
Fourth, there is a bug in IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c:
FspWrapperVariableRequestHandler (&FspHobListPtr,
FspMultiPhaseMemInitApiIndex);
Should be this:
FspWrapperVariableRequestHandler (&FspHobListPtr,
FspMultiPhaseSiInitApiIndex);
Fifth, I noticed some spelling errors in
IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c:
// Firstly querry variable request informaiton from FSP.
Should be:
// Get the variable request information from FSP.
And this:
// Firstly querry FSP for how many phases supported.
Should be:
// Query FSP for the number of phases supported.
Thanks,
Nate
-----Original Message-----
From: Chiu, Chasel <chasel.c...@intel.com>
Sent: Thursday, August 4, 2022 5:20 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desim...@intel.com>; Zeng, Star <star.z...@intel.com>
Subject: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 MultiPhase.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3916
Add FSP 2.4 MultiPhase interfaces and implementation.
Cc: Nate DeSimone
<nathaniel.l.desim...@intel.com<mailto:nathaniel.l.desim...@intel.com>>
Cc: Star Zeng <star.z...@intel.com<mailto:star.z...@intel.com>>
Signed-off-by: Chasel Chiu <chasel.c...@intel.com<mailto:chasel.c...@intel.com>>
Chasel Chiu (4):
IntelFsp2Pkg: Add FSP 2.4 MultiPhase interface.
IntelFsp2WrapperPkg: Add FSP 2.4 MultiPhase interface.
IntelFsp2Pkg: Adopt FSP 2.4 MultiPhase functions.
IntelFsp2WrapperPkg: Implement FSP 2.4 MultiPhase wrapper handlers.
IntelFsp2Pkg/FspSecCore/SecFsp.c
| 4 ++++
IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
| 9 +++++++++
IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/FspMultiPhaseLib.c
| 176
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
| 33 +++++++++++++++++++++++++--------
IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
| 27 +++++++++++++++++++++------
IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c
| 337
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
| 75
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf
| 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
| 304
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryS.nasm
| 101
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
| 3 +++
IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryM.nasm
| 303
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryS.nasm
| 108
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm
| 3 +++
IntelFsp2Pkg/Include/FspEas/FspApi.h
| 62
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
IntelFsp2Pkg/Include/FspGlobalData.h
| 5 ++++-
IntelFsp2Pkg/Include/Library/FspMultiPhaseLib.h
| 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/IntelFsp2Pkg.dec
| 12 ++++++++++--
IntelFsp2Pkg/IntelFsp2Pkg.dsc
| 4 ++++
IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/BaseFspMultiPhaseLib.inf
| 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/Tools/SplitFspBin.py
| 48 +++++++++++++++++++++++++-----------------------
IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
| 1 +
IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
| 3 ++-
IntelFsp2WrapperPkg/Include/Library/FspWrapperMultiPhaseProcessLib.h
| 38 ++++++++++++++++++++++++++++++++++++++
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
| 6 +++++-
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
| 4 +++-
IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/FspWrapperMultiPhaseProcessLib.inf
| 47 +++++++++++++++++++++++++++++++++++++++++++++++
27 files changed, 1831 insertions(+), 45 deletions(-) create mode 100644
IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/FspMultiPhaseLib.c
create mode 100644
IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c
create mode 100644 IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
create mode 100644 IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf
create mode 100644 IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
create mode 100644 IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryS.nasm
create mode 100644 IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryM.nasm
create mode 100644 IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryS.nasm
create mode 100644 IntelFsp2Pkg/Include/Library/FspMultiPhaseLib.h
create mode 100644
IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/BaseFspMultiPhaseLib.inf
create mode 100644
IntelFsp2WrapperPkg/Include/Library/FspWrapperMultiPhaseProcessLib.h
create mode 100644
IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/FspWrapperMultiPhaseProcessLib.inf
--
2.35.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92136): https://edk2.groups.io/g/devel/message/92136
Mute This Topic: https://groups.io/mt/92826131/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
