Re: [PATCH 10/10] OvmfPkg/Sec: Fill Temp Ram after TemporaryRamMigration

[Laszlo Ersek <lersek@redhat.com>]

On 02/18/19 05:11, Jordan Justen wrote:
> On some platforms, the TemporaryRamMigration PPI may cause Temporary
> RAM to become inaccessible after the RAM is migrated. To emulate this
> in OVMF, we should initialize memory to a bad state to make sure it
> isn't accidentally being used after TemporaryRamMigration is called.
> 
> I tested IA32 booting to shell and X64 booting to OS with this change.
> With X64 I also tested S3 suspend/resume.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien.grall@linaro.org>
> ---
>  OvmfPkg/Sec/SecMain.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> index 86c22a2ac9..72946e0eab 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -948,6 +948,14 @@ TemporaryRamMigration (
>      LongJump (&JumpBuffer, (UINTN)-1);
>    }
>  
> +  //
> +  // Initialize Temporary RAM to a bad value to make sure it will not
> +  // be used after migration.
> +  //
> +  SetMem32 (
> +    (VOID*)(UINTN)TemporaryMemoryBase, CopySize,
> +    PcdGet32 (PcdInitValueInTempStack));
> +
>    SaveAndSetDebugTimerInterrupt (OldStatus);
>  
>    return EFI_SUCCESS;
> 

Assuming the previous PEI Core patches in this series don't change the
order, it looks like the PeiCheckAndSwitchStack() function
[MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c] logs the "stack ever
used" stat first, and calls the TemporaryRamMigration() PPI member second.

Thus, by the time we overwrite the temp RAM in this patch, the
statistics will have been printed in PeiCheckAndSwitchStack(). OK.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo