| Message ID | 20171101131145.16459-3-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 3909c4a1934460d4c18f2a0c827d3d6d37781b7a |
| Headers | show |
| Series | PEI phase boot mode setting | expand |
On Wed, Nov 01, 2017 at 01:11:45PM +0000, Ard Biesheuvel wrote: > The current interdepencies between the PrePeiCore SEC module, the > platform PEIM and ArmPlatformLib is a bit awkward: due to the fact > that ArmPlatformLib is also used by SEC modules, we cannot use PEI > specific facilities in the implementation of ArmPlatformGetBootMode. > However, given that we call that library function /after/ invoking > PlatformPeiLib, there is no way for that library to set the boot mode > other than resorting to tricks like notification callbacks on arbitrary > unrelated events. > > ArmPlatformLib should probably be phased out anyway, given its quirky > nature, Yes, it should. > but for now, let's fix this particular issue by deferring the > call to PlatformPeim() to after the point where we set the boot mode > by calling ArmPlatformGetBootMode (). > > While we're at it, clean up the code slightly by using PeiServicesLib > instead of doing double pointer dereferencing. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmPlatformPkg/PlatformPei/PlatformPeim.c | 12 +++++++----- > ArmPlatformPkg/PlatformPei/PlatformPeim.inf | 1 + > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeim.c b/ArmPlatformPkg/PlatformPei/PlatformPeim.c > index e4535250c245..14f301e947a8 100644 > --- a/ArmPlatformPkg/PlatformPei/PlatformPeim.c > +++ b/ArmPlatformPkg/PlatformPei/PlatformPeim.c > @@ -83,21 +83,23 @@ InitializePlatformPeim ( > ) > { > EFI_STATUS Status; > - UINTN BootMode; > + EFI_BOOT_MODE BootMode; > > DEBUG ((EFI_D_LOAD | EFI_D_INFO, "Platform PEIM Loaded\n")); > > + Status = PeiServicesSetBootMode (ArmPlatformGetBootMode ()); > + ASSERT_EFI_ERROR (Status); > + > PlatformPeim (); > > - BootMode = ArmPlatformGetBootMode (); > - Status = (**PeiServices).SetBootMode (PeiServices, (UINT8) BootMode); > + Status = PeiServicesGetBootMode (&BootMode); > ASSERT_EFI_ERROR (Status); > > - Status = (**PeiServices).InstallPpi (PeiServices, &mPpiListBootMode); > + Status = PeiServicesInstallPpi (&mPpiListBootMode); > ASSERT_EFI_ERROR (Status); > > if (BootMode == BOOT_IN_RECOVERY_MODE) { > - Status = (**PeiServices).InstallPpi (PeiServices, &mPpiListRecoveryBootMode); > + Status = PeiServicesInstallPpi (&mPpiListRecoveryBootMode); > ASSERT_EFI_ERROR (Status); > } > > diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf > index f466c1412ad3..21701cdc0731 100644 > --- a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf > +++ b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf > @@ -43,6 +43,7 @@ [LibraryClasses] > HobLib > ArmPlatformLib > PlatformPeiLib > + PeiServicesLib If you move that one up one line: Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > > [Ppis] > gEfiPeiMasterBootModePpiGuid # PPI ALWAYS_PRODUCED > -- > 2.11.0 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeim.c b/ArmPlatformPkg/PlatformPei/PlatformPeim.c index e4535250c245..14f301e947a8 100644 --- a/ArmPlatformPkg/PlatformPei/PlatformPeim.c +++ b/ArmPlatformPkg/PlatformPei/PlatformPeim.c @@ -83,21 +83,23 @@ InitializePlatformPeim ( ) { EFI_STATUS Status; - UINTN BootMode; + EFI_BOOT_MODE BootMode; DEBUG ((EFI_D_LOAD | EFI_D_INFO, "Platform PEIM Loaded\n")); + Status = PeiServicesSetBootMode (ArmPlatformGetBootMode ()); + ASSERT_EFI_ERROR (Status); + PlatformPeim (); - BootMode = ArmPlatformGetBootMode (); - Status = (**PeiServices).SetBootMode (PeiServices, (UINT8) BootMode); + Status = PeiServicesGetBootMode (&BootMode); ASSERT_EFI_ERROR (Status); - Status = (**PeiServices).InstallPpi (PeiServices, &mPpiListBootMode); + Status = PeiServicesInstallPpi (&mPpiListBootMode); ASSERT_EFI_ERROR (Status); if (BootMode == BOOT_IN_RECOVERY_MODE) { - Status = (**PeiServices).InstallPpi (PeiServices, &mPpiListRecoveryBootMode); + Status = PeiServicesInstallPpi (&mPpiListRecoveryBootMode); ASSERT_EFI_ERROR (Status); } diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf index f466c1412ad3..21701cdc0731 100644 --- a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf +++ b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf @@ -43,6 +43,7 @@ [LibraryClasses] HobLib ArmPlatformLib PlatformPeiLib + PeiServicesLib [Ppis] gEfiPeiMasterBootModePpiGuid # PPI ALWAYS_PRODUCED
The current interdepencies between the PrePeiCore SEC module, the platform PEIM and ArmPlatformLib is a bit awkward: due to the fact that ArmPlatformLib is also used by SEC modules, we cannot use PEI specific facilities in the implementation of ArmPlatformGetBootMode. However, given that we call that library function /after/ invoking PlatformPeiLib, there is no way for that library to set the boot mode other than resorting to tricks like notification callbacks on arbitrary unrelated events. ArmPlatformLib should probably be phased out anyway, given its quirky nature, but for now, let's fix this particular issue by deferring the call to PlatformPeim() to after the point where we set the boot mode by calling ArmPlatformGetBootMode (). While we're at it, clean up the code slightly by using PeiServicesLib instead of doing double pointer dereferencing. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPlatformPkg/PlatformPei/PlatformPeim.c | 12 +++++++----- ArmPlatformPkg/PlatformPei/PlatformPeim.inf | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel