On 11/13/17 13:34, Laszlo Ersek wrote:
> I guess I could live with BASE_LIBRARY_JUMP_BUFFER.
Actually:
> More specific comments:
>
>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
>> index f7fec3d8c0..077f7d6563 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 - 2017, Intel Corporation. All rights reserved.<BR>
>> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>>
>> This program and the accompanying materials
>> @@ -731,6 +731,25 @@ SecCoreStartupWithStack (
>> UINT32 Index;
>> volatile UINT8 *Table;
>>
>> + //
>> + // Fill most of temporary RAM with PcdInitValueInTempStack. We stop
>> + // filling at the current stack pointer - 512 bytes.
>> + //
>> + DEBUG_CODE_BEGIN ();
>> + BASE_LIBRARY_JUMP_BUFFER JumpBuffer;
>> + UINTN StackUsed;
>> +
>> + SetJump (&JumpBuffer);
>> +#if defined (MDE_CPU_IA32)
>> + StackUsed = (UINTN)TopOfCurrentStack - JumpBuffer.Esp;
>> +#elif defined (MDE_CPU_X64)
>> + StackUsed = (UINTN)TopOfCurrentStack - JumpBuffer.Rsp;
>> +#endif
>> + SetMem32 ((VOID*)(UINTN)PcdGet32 (PcdOvmfSecPeiTempRamBase),
>> + PcdGet32 (PcdOvmfSecPeiTempRamSize) - StackUsed - 512,
>> + FixedPcdGet32 (PcdInitValueInTempStack));
>
> (1) SetMem32() is likely problematic in itself; please refer to the
> following comment -- partly visible in the context of Jordan's patch --,
> from commit 320b4f084a25 ("OvmfPkg: Sec: force reinit of
> BaseExtractGuidedSectionLib handler table", 2015-11-30):
>
> //
> // To ensure SMM can't be compromised on S3 resume, we must force re-init of
> // the BaseExtractGuidedSectionLib. Since this is before library contructors
> // are called, we must use a loop rather than SetMem.
> //
>
> Thus, we should use a loop and a pointer-to-volatile. (It would likely
> be slower than the REP STOSD / REP STOSQ.)
given that I'm opposed to calling any library functions before we reach
the ProcessLibraryConstructorList() call lower down in
SecCoreStartupWithStack(), I cannot agree to calling SetJump() either.
Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
