On 02/18/19 05:11, Jordan Justen wrote: > Apparently something depends on the heap being above the stack after > TemporaryRamMigration. > > This is the opposite order that OVMF has always used for TempRam > before migration to permanent ram. In 42a83e80f37c (svn r10770), Mike > changed OVMF's TemporaryRamMigration to swap the locations of heap and > stack during the migration. > > Rather than doing the swap during TemporaryRamMigration, this change > causes OVMF to boot with TemporaryRam setup with heap being above the > stack. Then, during TemporaryRamMigration, we can directly copy all of > TemporaryRam. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: Anthony Perard <anthony.per...@citrix.com> > Cc: Julien Grall <julien.gr...@linaro.org> > --- > OvmfPkg/Sec/Ia32/SecEntry.nasm | 2 +- > OvmfPkg/Sec/SecMain.c | 39 +++++++++++++++++----------------- > OvmfPkg/Sec/X64/SecEntry.nasm | 2 +- > 3 files changed, 22 insertions(+), 21 deletions(-) > > diff --git a/OvmfPkg/Sec/Ia32/SecEntry.nasm b/OvmfPkg/Sec/Ia32/SecEntry.nasm > index 03501969eb..61917b9eef 100644 > --- a/OvmfPkg/Sec/Ia32/SecEntry.nasm > +++ b/OvmfPkg/Sec/Ia32/SecEntry.nasm > @@ -57,7 +57,7 @@ ASM_PFX(_ModuleEntryPoint): > ; Load temporary RAM stack based on PCDs > ; > %define SEC_TOP_OF_STACK (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) + \ > - FixedPcdGet32 (PcdOvmfSecPeiTempRamSize)) > + (FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) / 2)) > mov eax, SEC_TOP_OF_STACK > mov esp, eax > nop > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > index f7fec3d8c0..46ac739862 100644 > --- a/OvmfPkg/Sec/SecMain.c > +++ b/OvmfPkg/Sec/SecMain.c > @@ -1,7 +1,7 @@ > /** @file > Main SEC phase code. Transitions to PEI. > > - Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR> > (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> > > This program and the accompanying materials > @@ -779,15 +779,15 @@ SecCoreStartupWithStack ( > #endif > > // > - // |-------------| <-- TopOfCurrentStack > - // | Stack | 32k > // |-------------| > // | Heap | 32k > + // |-------------| <-- TopOfCurrentStack > + // | Stack | 32k > // |-------------| <-- SecCoreData.TemporaryRamBase > // > > ASSERT ((UINTN) (PcdGet32 (PcdOvmfSecPeiTempRamBase) + > - PcdGet32 (PcdOvmfSecPeiTempRamSize)) == > + (PcdGet32 (PcdOvmfSecPeiTempRamSize) / 2)) == > (UINTN) TopOfCurrentStack); > > // > @@ -795,14 +795,17 @@ SecCoreStartupWithStack ( > // > SecCoreData.DataSize = sizeof(EFI_SEC_PEI_HAND_OFF); > > - SecCoreData.TemporaryRamSize = (UINTN) PcdGet32 > (PcdOvmfSecPeiTempRamSize); > - SecCoreData.TemporaryRamBase = (VOID*)((UINT8 *)TopOfCurrentStack - > SecCoreData.TemporaryRamSize); > + SecCoreData.TemporaryRamBase = > + (VOID*)(UINTN) PcdGet32 (PcdOvmfSecPeiTempRamBase); > + SecCoreData.TemporaryRamSize = (UINTN) PcdGet32 (PcdOvmfSecPeiTempRamSize); > > - SecCoreData.PeiTemporaryRamBase = SecCoreData.TemporaryRamBase; > - SecCoreData.PeiTemporaryRamSize = SecCoreData.TemporaryRamSize >> 1; > + SecCoreData.PeiTemporaryRamBase = > + (UINT8*)(VOID*)(UINTN) PcdGet32 (PcdOvmfSecPeiTempRamBase) + > + ((UINTN) PcdGet32 (PcdOvmfSecPeiTempRamSize) / 2); > + SecCoreData.PeiTemporaryRamSize = PcdGet32 (PcdOvmfSecPeiTempRamSize) / 2; > > - SecCoreData.StackBase = (UINT8 *)SecCoreData.TemporaryRamBase > + SecCoreData.PeiTemporaryRamSize; > - SecCoreData.StackSize = SecCoreData.TemporaryRamSize >> 1; > + SecCoreData.StackBase = (VOID*)(UINTN) PcdGet32 (PcdOvmfSecPeiTempRamBase); > + SecCoreData.StackSize = PcdGet32 (PcdOvmfSecPeiTempRamSize) / 2; > > SecCoreData.BootFirmwareVolumeBase = BootFv; > SecCoreData.BootFirmwareVolumeSize = (UINTN) BootFv->FvLength; > @@ -895,10 +898,10 @@ TemporaryRamMigration ( > (UINT64)CopySize > )); > > - OldHeap = (VOID*)(UINTN)TemporaryMemoryBase; > + OldHeap = (VOID*)((UINTN)TemporaryMemoryBase + (CopySize >> 1)); > NewHeap = (VOID*)((UINTN)PermanentMemoryBase + (CopySize >> 1)); > > - OldStack = (VOID*)((UINTN)TemporaryMemoryBase + (CopySize >> 1)); > + OldStack = (VOID*)(UINTN)TemporaryMemoryBase; > NewStack = (VOID*)(UINTN)PermanentMemoryBase; > > DebugAgentContext.HeapMigrateOffset = (UINTN)NewHeap - (UINTN)OldHeap; > @@ -908,15 +911,13 @@ TemporaryRamMigration ( > InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, (VOID *) > &DebugAgentContext, NULL); > > // > - // Migrate Heap > + // Migrate the whole temporary memory to permenent memory.
s/permenent/permanent/ > // > - CopyMem (NewHeap, OldHeap, CopySize >> 1); > + CopyMem( > + (VOID*)(UINTN)PermanentMemoryBase, > + (VOID*)(UINTN)TemporaryMemoryBase, > + CopySize); I think the last paren should be on a separate line, in this style. With those updates: Reviewed-by: Laszlo Ersek <ler...@redhat.com> Thanks Laszlo > > - // > - // Migrate Stack > - // > - CopyMem (NewStack, OldStack, CopySize >> 1); > - > // > // Rebase IDT table in permanent memory > // > diff --git a/OvmfPkg/Sec/X64/SecEntry.nasm b/OvmfPkg/Sec/X64/SecEntry.nasm > index d76adcffd8..dd603d6eb0 100644 > --- a/OvmfPkg/Sec/X64/SecEntry.nasm > +++ b/OvmfPkg/Sec/X64/SecEntry.nasm > @@ -59,7 +59,7 @@ ASM_PFX(_ModuleEntryPoint): > ; Load temporary RAM stack based on PCDs > ; > %define SEC_TOP_OF_STACK (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) + \ > - FixedPcdGet32 (PcdOvmfSecPeiTempRamSize)) > + (FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) / 2)) > mov rsp, SEC_TOP_OF_STACK > nop > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
