Re: [PATCH 2/4] OvmfPkg/Sec/Ia32: seed the temporary RAM with PcdInitValueInTempStack

[Jordan Justen <jordan.l.justen@intel.com>]

On 2017-11-10 07:49:06, Laszlo Ersek wrote:
> diff --git a/OvmfPkg/Sec/Ia32/SecEntry.nasm b/OvmfPkg/Sec/Ia32/SecEntry.nasm
> index 54d074e621f6..1d426fafa888 100644
> --- a/OvmfPkg/Sec/Ia32/SecEntry.nasm
> +++ b/OvmfPkg/Sec/Ia32/SecEntry.nasm
> @@ -29,6 +29,7 @@ extern ASM_PFX(SecCoreStartupWithStack)
>  ; @param[in]  EAX   Initial value of the EAX register (BIST: Built-in Self Test)
>  ; @param[in]  DI    'BP': boot-strap processor, or 'AP': application processor
>  ; @param[in]  EBP   Pointer to the start of the Boot Firmware Volume
> +; @param[in]  ES    Set to LINEAR_SEL in TransitionFromReal16To32BitFlat

Can you document all the segment registers, and also document them in
UefiCpuPkg/ResetVector/Vtf0/Main.asm?

>  ;
>  ; @return     None  This routine does not return
>  ;
> @@ -44,6 +45,18 @@ ASM_PFX(_ModuleEntryPoint):
>      mov     esp, ebx
>      nop
>  
> +    ;
> +    ; Fill the temporary RAM with the initial stack value.
> +    ; The loop below will seed the heap as well, but that's harmless.
> +    ;
> +    mov     eax, FixedPcdGet32 (PcdInitValueInTempStack)  ; dword to store
> +    mov     edi, FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) ; base address,
> +                                                          ;   relative to ES
> +    mov     ecx, FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) ; byte count
> +    shr     ecx, 2                                        ; dword count

I'm not sure, but I think NASM might let you do something like:

    mov     ecx, FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) / 4

> +    cld                                                   ; store from base up
> +    rep stosd

I think if you move this above the code in patch 1, then patch 1 is
not needed. I also think it would be reasonable to merge 2 & 3, but
separate is fine too.

-Jordan