Hi Eric, On 08/28/19 08:50, Eric Dong wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2136 > > SecPlatformMain is a platform hook function which let platform does > some update. Some platform may adjust SecCoreData->PeiTemporaryRamBase > which caused former saved AllSecPpiList variable invalid. > > This patch update the logic to get AllSecPpiList after SecPlatformMain. > > Cc: Ray Ni <ray...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Signed-off-by: Eric Dong <eric.d...@intel.com> > --- > UefiCpuPkg/SecCore/SecMain.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c > index f914446257..66c952b897 100644 > --- a/UefiCpuPkg/SecCore/SecMain.c > +++ b/UefiCpuPkg/SecCore/SecMain.c > @@ -228,7 +228,6 @@ SecStartupPhase2( > > PeiCoreEntryPoint = NULL; > SecCoreData = (EFI_SEC_PEI_HAND_OFF *) Context; > - AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) > SecCoreData->PeiTemporaryRamBase; > > // > // Perform platform specific initialization before entering PeiCore. > @@ -282,6 +281,8 @@ SecStartupPhase2( > } > > if (PpiList != NULL) { > + AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) > SecCoreData->PeiTemporaryRamBase; > + > // > // Remove the terminal flag from the terminal PPI > // >
Based on the SecPlatformMain() documentation in "UefiCpuPkg/Include/Library/PlatformSecLib.h", it seems valid for a platform to change "SecCoreData->PeiTemporaryRamBase". Therefore, in SecStartupPhase2(), it appears justified to assign "AllSecPpiList" only after calling SecPlatformMain(). This patch does something else too: it makes the assignment to "AllSecPpiList" conditional. I agree that is justified, as well. Namely: [*] If SecPlatformMain() returns no platform-specific PPI list, then there is nothing to merge, so we don't need "AllSecPpiList" at all. I think however that this change should be documented explicitly. When pushing, please add the sentence [*] to the commit message. With that: Reviewed-by: Laszlo Ersek <ler...@redhat.com> Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46567): https://edk2.groups.io/g/devel/message/46567 Mute This Topic: https://groups.io/mt/33054478/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
