On Tue, Nov 20, 2018 at 04:44:33PM +0100, Ard Biesheuvel wrote: > Our poor man's implementation of EnterS3WithImmediateWake () currently > sets a high TPL level to disable interrupts, and simply calls the > PEI entrypoint again after disabling the MMU. > > Unfortunately, this is not sufficient: DMA capable devices such as > network controllers or USB controllers may still be enabled and > writing to memory, e.g., in response to incoming network packets. > > So instead, do the full ExitBootServices() dance: allocate space and > get the memory map, call ExitBootServices(), and in case it fails, get > the memory map again and call ExitBootServices() again. This ensures > that all cleanup related to DMA capable devices is performed before > doing the warm reset. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > v2: - add asm routines to ensure that we don't access memory after > disabling the MMU and caches
Fun times. Reviewed-by: Leif Lindholm <[email protected]> > - set MemMap to NULL > > ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S | 30 > +++++++++++ > ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm | 35 > ++++++++++++ > ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S | 29 > ++++++++++ > ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm | 34 > ++++++++++++ > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c | 57 > +++++++++++++++++--- > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 8 +++ > 6 files changed, 187 insertions(+), 6 deletions(-) > > diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S > b/ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S > new file mode 100644 > index 000000000000..1edca941cb8f > --- /dev/null > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.S > @@ -0,0 +1,30 @@ > +/** @file > + ResetSystemLib implementation using PSCI calls > + > + Copyright (c) 2018, Linaro Ltd. 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 > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > + > +**/ > + > +#include <AsmMacroIoLibV8.h> > + > +ASM_FUNC(DisableMmuAndReenterPei) > + stp x29, x30, [sp, #-16]! > + mov x29, sp > + > + bl ArmDisableMmu > + > + // no memory accesses after MMU and caches have been disabled > + > + MOV64 (x0, FixedPcdGet64 (PcdFvBaseAddress)) > + blr x0 > + > + // never returns > + nop > diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm > b/ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm > new file mode 100644 > index 000000000000..b3dca150c150 > --- /dev/null > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/AArch64/Reset.asm > @@ -0,0 +1,35 @@ > +/** @file > + ResetSystemLib implementation using PSCI calls > + > + Copyright (c) 2018, Linaro Ltd. 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 > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > + > +**/ > + > + AREA Reset, CODE, READONLY > + > + EXPORT DisableMmuAndReenterPei > + IMPORT ArmDisableMmu > + > +DisableMmuAndReenterPei > + stp x29, x30, [sp, #-16]! > + mov x29, sp > + > + bl ArmDisableMmu > + > + ; no memory accesses after MMU and caches have been disabled > + > + movl x0, FixedPcdGet64 (PcdFvBaseAddress) > + blr x0 > + > + ; never returns > + nop > + > + END > diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S > b/ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S > new file mode 100644 > index 000000000000..b6fe2bd82baa > --- /dev/null > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.S > @@ -0,0 +1,29 @@ > +/** @file > + ResetSystemLib implementation using PSCI calls > + > + Copyright (c) 2018, Linaro Ltd. 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 > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > + > +**/ > + > +#include <AsmMacroIoLib.h> > + > +ASM_FUNC(DisableMmuAndReenterPei) > + push {lr} > + > + bl ArmDisableMmu > + > + // no memory accesses after MMU and caches have been disabled > + > + MOV32 (r0, FixedPcdGet64 (PcdFvBaseAddress)) > + blx r0 > + > + // never returns > + nop > diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm > b/ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm > new file mode 100644 > index 000000000000..f098b352418b > --- /dev/null > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/Arm/Reset.asm > @@ -0,0 +1,34 @@ > +/** @file > + ResetSystemLib implementation using PSCI calls > + > + Copyright (c) 2018, Linaro Ltd. 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 > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > + > +**/ > + > + INCLUDE AsmMacroExport.inc > + PRESERVE8 > + > + IMPORT ArmDisableMmu > + > +RVCT_ASM_EXPORT DisableMmuAndReenterPei > + push {lr} > + > + bl ArmDisableMmu > + > + ; no memory accesses after MMU and caches have been disabled > + > + mov32 r0, FixedPcdGet64 (PcdFvBaseAddress) > + blx r0 > + > + ; never returns > + nop > + > + END > diff --git > a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c > b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c > index 10ceafd14d5d..3f7b8ae0b169 100644 > --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c > @@ -1,7 +1,7 @@ > /** @file > ResetSystemLib implementation using PSCI calls > > - Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR> > + Copyright (c) 2017 - 2018, Linaro Ltd. All rights reserved.<BR> > > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > @@ -81,6 +81,8 @@ ResetShutdown ( > ArmCallSmc (&ArmSmcArgs); > } > > +VOID DisableMmuAndReenterPei (VOID); > + > /** > This function causes the system to enter S3 and then wake up immediately. > > @@ -92,7 +94,12 @@ EnterS3WithImmediateWake ( > VOID > ) > { > - VOID (*Reset)(VOID); > + EFI_PHYSICAL_ADDRESS Alloc; > + EFI_MEMORY_DESCRIPTOR *MemMap; > + UINTN MemMapSize; > + UINTN MapKey, DescriptorSize; > + UINT32 DescriptorVersion; > + EFI_STATUS Status; > > if (FeaturePcdGet (PcdArmReenterPeiForCapsuleWarmReboot) && > !EfiAtRuntime ()) { > @@ -101,11 +108,49 @@ EnterS3WithImmediateWake ( > // immediate wake (which is used by capsule update) by disabling the MMU > // and interrupts, and jumping to the PEI entry point. > // > - Reset = (VOID (*)(VOID))(UINTN)FixedPcdGet64 (PcdFvBaseAddress); > > - gBS->RaiseTPL (TPL_HIGH_LEVEL); > - ArmDisableMmu (); > - Reset (); > + // > + // Obtain the size of the memory map > + // > + MemMapSize = 0; > + MemMap = NULL; > + Status = gBS->GetMemoryMap (&MemMapSize, MemMap, &MapKey, > &DescriptorSize, > + &DescriptorVersion); > + ASSERT (Status == EFI_BUFFER_TOO_SMALL); > + > + // > + // Add some slack to the allocation to cater for changes in the memory > + // map if ExitBootServices () fails the first time around. > + // > + MemMapSize += SIZE_4KB; > + Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData, > + EFI_SIZE_TO_PAGES (MemMapSize), &Alloc); > + ASSERT_EFI_ERROR (Status); > + > + MemMap = (EFI_MEMORY_DESCRIPTOR *)(UINTN)Alloc; > + > + Status = gBS->GetMemoryMap (&MemMapSize, MemMap, &MapKey, > &DescriptorSize, > + &DescriptorVersion); > + ASSERT_EFI_ERROR (Status); > + > + Status = gBS->ExitBootServices (gImageHandle, MapKey); > + if (EFI_ERROR (Status)) { > + // > + // ExitBootServices () may fail the first time around if an event fired > + // right after the call to GetMemoryMap() which allocated or freed > memory. > + // Since that first call to ExitBootServices () will disarm the timer, > + // this is guaranteed not to happen again, so one additional attempt > + // should suffice. > + // > + Status = gBS->GetMemoryMap (&MemMapSize, MemMap, &MapKey, > &DescriptorSize, > + &DescriptorVersion); > + ASSERT_EFI_ERROR (Status); > + > + Status = gBS->ExitBootServices (gImageHandle, MapKey); > + ASSERT_EFI_ERROR (Status); > + } > + > + DisableMmuAndReenterPei (); > } > } > > diff --git > a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf > b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf > index 19021cd1e8b6..3b8925c6f9cd 100644 > --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf > @@ -21,6 +21,14 @@ > VERSION_STRING = 1.0 > LIBRARY_CLASS = ResetSystemLib > > +[Sources.AARCH64] > + AArch64/Reset.S | GCC > + AArch64/Reset.asm | MSFT > + > +[Sources.ARM] > + Arm/Reset.S | GCC > + Arm/Reset.asm | RVCT > + > [Sources] > ArmSmcPsciResetSystemLib.c > > -- > 2.17.1 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
