On Fri, Nov 10, 2017 at 02:21:20PM +0000, Ard Biesheuvel wrote: > In preparation of adding support for setting a DIP switch to clear the > EFI variable store, update the early capsule handling logic to take the > boot mode into account. > > This is necessary for two reasons: > - we override the boot mode when a capsule is detected, > - the capsule detection itself involves reading a EFI variable, which we > shouldn't be doing if the varstore may be in a bad state. > > So factor out the initial capsule check (to keep the code understandable) > and only perform it if we are not booting in 'clear NVRAM' mode. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Minor suggestion below, fold in if you like it. Regardless: Reviewed-by: Leif Lindholm <leif.lindh...@linaro.org> > --- > > Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c > | 68 +++++++++++++------- > 1 file changed, 45 insertions(+), 23 deletions(-) > > diff --git > a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c > > b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c > index b44c58d61062..63c441872da7 100644 > --- > a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c > +++ > b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c > @@ -170,6 +170,44 @@ DeclareDram ( > return EFI_SUCCESS; > } > > +STATIC > +BOOLEAN > +CheckCapsule ( > + IN EFI_PEI_SERVICES **PeiServices, > + IN PEI_CAPSULE_PPI *Capsule, > + IN EFI_PHYSICAL_ADDRESS UefiMemoryBase, > + OUT VOID **CapsuleBuffer, > + OUT UINTN *CapsuleBufferLength > + ) > +{ > + EFI_STATUS Status; > + > + Status = Capsule->CheckCapsuleUpdate (PeiServices); > + if (!EFI_ERROR (Status)) { > + > + // > + // Coalesce the capsule into unused memory. CreateState() below will copy > + // it to a properly allocated buffer. > + // > + *CapsuleBuffer = (VOID *)PcdGet64 (PcdSystemMemoryBase); > + *CapsuleBufferLength = UefiMemoryBase - PcdGet64 (PcdSystemMemoryBase); > + > + PeiServicesSetBootMode (BOOT_ON_FLASH_UPDATE); > + > + Status = Capsule->Coalesce (PeiServices, CapsuleBuffer, > + CapsuleBufferLength); > + if (!EFI_ERROR (Status)) { > + DEBUG ((DEBUG_INFO, "%a: Coalesced capsule @ %p (0x%lx)\n", > + __FUNCTION__, *CapsuleBuffer, *CapsuleBufferLength)); > + return TRUE; > + } else { > + DEBUG ((DEBUG_WARN, "%a: failed to coalesce() capsule (Status == > %r)\n", > + __FUNCTION__, Status)); > + } > + } > + return FALSE; > +} > + > EFI_STATUS > EFIAPI > MemoryPeim ( > @@ -184,6 +222,7 @@ MemoryPeim ( > VOID *CapsuleBuffer; > UINTN CapsuleBufferLength; > BOOLEAN HaveCapsule; > + EFI_BOOT_MODE BootMode; > > Status = DeclareDram (&VirtualMemoryTable); > ASSERT_EFI_ERROR (Status); > @@ -199,31 +238,14 @@ MemoryPeim ( > ASSERT_EFI_ERROR (Status); > > // > - // Check for persistent capsules > + // Check for persistent capsules, unless we are booting with default > + // settings. > // > HaveCapsule = FALSE; > - Status = Capsule->CheckCapsuleUpdate (PeiServices); > - if (!EFI_ERROR (Status)) { > - > - // > - // Coalesce the capsule into unused memory. CreateState() below will copy > - // it to a properly allocated buffer. > - // > - CapsuleBuffer = (VOID *)PcdGet64 (PcdSystemMemoryBase); > - CapsuleBufferLength = UefiMemoryBase - PcdGet64 (PcdSystemMemoryBase); > - > - PeiServicesSetBootMode (BOOT_ON_FLASH_UPDATE); > - > - Status = Capsule->Coalesce (PeiServices, &CapsuleBuffer, > - &CapsuleBufferLength); > - if (!EFI_ERROR (Status)) { > - DEBUG ((DEBUG_INFO, "%a: Coalesced capsule @ %p (0x%lx)\n", > - __FUNCTION__, CapsuleBuffer, CapsuleBufferLength)); > - HaveCapsule = TRUE; > - } else { > - DEBUG ((DEBUG_WARN, "%a: failed to coalesce() capsule (Status == > %r)\n", > - __FUNCTION__, Status)); > - } > + Status = PeiServicesGetBootMode (&BootMode); > + if (!EFI_ERROR (Status) && BootMode != BOOT_WITH_DEFAULT_SETTINGS) { > + HaveCapsule = CheckCapsule (PeiServices, Capsule, UefiMemoryBase, > + &CapsuleBuffer, &CapsuleBufferLength); You could do } else { HaveCapsule = FALSE; here instead of unconditionally before the test. / Leif > } > > Status = ArmConfigureMmu (VirtualMemoryTable, NULL, NULL); > -- > 2.11.0 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel