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

[Laszlo Ersek <lersek@redhat.com>]

On 11/10/17 16:56, Ard Biesheuvel wrote:
> On 10 November 2017 at 15:49, Laszlo Ersek <lersek@redhat.com> wrote:
>> This allows the PEI core to report the maximum temporary SEC/PEI stack
>> usage on the DEBUG_INFO level, in the PeiCheckAndSwitchStack() function
>> [MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c]:
>>
>> * Normal boot:
>>
>>> Temp Stack : BaseAddress=0x814000 Length=0x4000
>>> Temp Heap  : BaseAddress=0x810000 Length=0x4000
>>> Total temporary memory:    32768 bytes.
>>>   temporary memory stack ever used:       3664 bytes. <----
>>>   temporary memory heap used for HobList: 5904 bytes.
>>>   temporary memory heap occupied by memory pages: 0 bytes.
>>
>> * S3 resume (with PEI decompression / SMM):
>>
>>> Temp Stack : BaseAddress=0x814000 Length=0x4000
>>> Temp Heap  : BaseAddress=0x810000 Length=0x4000
>>> Total temporary memory:    32768 bytes.
>>>   temporary memory stack ever used:       3428 bytes. <----
>>>   temporary memory heap used for HobList: 4816 bytes.
>>>   temporary memory heap occupied by memory pages: 0 bytes.
>>
>> I unit-tested this change by transitorily adding an infinite loop right
>> after the "rep stosd", and dumping the guest's temp SEC/PEI RAM (32KB
>> currently) while the guest was stuck in the loop. The dump includes one
>> dword from before and after the temp SEC/PEI RAM:
>>
>>> $ virsh qemu-monitor-command GUEST_NAME --hmp 'xp /8194wx 0x80FFFC'
>>>
>>> 000000000080fffc: 0x00000000 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
>>> 000000000081000c: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
>>> ...
>>> 0000000000817fec: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
>>> 0000000000817ffc: 0x5aa55aa5 0x00000000
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=747
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  OvmfPkg/Sec/SecMain.inf        |  1 +
>>  OvmfPkg/Sec/Ia32/SecEntry.nasm | 13 +++++++++++++
>>  2 files changed, 14 insertions(+)
>>
>> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
>> index 711b59530907..6051cb3c6c4c 100644
>> --- a/OvmfPkg/Sec/SecMain.inf
>> +++ b/OvmfPkg/Sec/SecMain.inf
>> @@ -71,6 +71,7 @@ [Pcd]
>>    gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
>>    gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
>>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
>>
>>  [FeaturePcd]
>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>> 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
>
> What does this mean? Does it belong in this patch? (Knowing you, and
> noticing that the next patch adds it to the x86 version of this code
> as well, I am sure it probably does, but I just need you to explain it
> to me :-))

See the STOSD instruction in the Intel SDM, or just on the web:

http://x86.renejeschke.de/html/file_module_x86_id_306.html

> Stores a byte, word, or doubleword from the AL, AX, or EAX register,
> respectively, into the destination operand. The destination operand is
> a memory location, the address of which is read from either the ES:EDI
> or the ES:DI registers (depending on the address-size attribute of the
> instruction, 32 or 16, respectively). The ES segment cannot be
> overridden with a segment override prefix.

It means that whatever we put in EDI, it will be relative to the segment
"identified by" ES. (See the code comment near "mov edi": "base address,
relative to ES".)

Above I put "identified by" in quotes, because the definition of
"identified by" depends on the mode the processor is in.

(Instead of a botched attempt at writing up x86 segmentation (plus,
optionally, paging) generally :) , I'll just leave these references
here:

* Intel SDM: 2.2 MODES OF OPERATION; Figure 2-3. Transitions Among the
  Processor's Operating Modes

* https://en.wikipedia.org/wiki/Unreal_mode

* https://en.wikipedia.org/wiki/X86_memory_segmentation#80286_protected_mode

* https://en.wikipedia.org/wiki/X86_memory_segmentation#80386_protected_mode
)

So using STOSD and ES:EDI as an example, here's a hugely inaccurate list
of possible address calculations, dependent on processor mode:

* real mode:

  physical_address = ES * 0x10 + DI

  E.g., A000:1234 means physical address A1234.

* big real mode (segmentation enabled, paging disabled):

  physical_address = GDT[ES].base_addr + EDI;

  Basically, the value in ES is a "selector"; an offset into the GDT
  (Global Descriptor Table). The base address of the GDT has been loaded
  into the GDTR with the LGDT instruction. Then, at the offset
  pointed-to by ES into the GDT, we find a segment descriptor entry,
  which provides the base address of the segment (and some other
  characteristics). The physical address is relative to that base.

* 80386 protected mode (segmentation and paging):

  virtual_address = GDT[ES].base_addr + EDI;
  physical_address = paging(virtual_address);

  Where in paging(), the CPU can use a cached translation from the TLB,
  or perform a page table walk.

In the code at hand, we are in "big real mode" (also called unreal mode,
32-bit flat mode). Segmentation is enabled, paging is not. The segment
selector in ES is an "implicit" parameter for the code being added,
because the STOSD instruction relies on ES.

We move from real mode to big real mode earlier in

  UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm

> %define SEC_DEFAULT_CR0  0x40000023                               <---- bit 31, "Paging Enabled", is clear
>                                                                         bit  0, "Protection Enable", is set, it will enable segmentation
> %define SEC_DEFAULT_CR4  0x640
>
> BITS    16
>
> ;
> ; Modified:  EAX, EBX
> ;
> TransitionFromReal16To32BitFlat:
>
>     debugShowPostCode POSTCODE_16BIT_MODE
>
>     cli
>
>     mov     bx, 0xf000
>     mov     ds, bx
>
>     mov     bx, ADDR16_OF(gdtr)
>
> o32 lgdt    [cs:bx]                                               <---- loads the GDT's address into GDTR
>
>     mov     eax, SEC_DEFAULT_CR0
>     mov     cr0, eax
>
>     jmp     LINEAR_CODE_SEL:dword ADDR_OF(jumpTo32BitAndLandHere) <--- actual mode switch
> BITS    32
> jumpTo32BitAndLandHere:
>
>     mov     eax, SEC_DEFAULT_CR4
>     mov     cr4, eax
>
>     debugShowPostCode POSTCODE_32BIT_MODE
>
>     mov     ax, LINEAR_SEL
>     mov     ds, ax
>     mov     es, ax                                                <---- this is where ES gets the LINEAR_SEL offset into the GDT
>     mov     fs, ax
>     mov     gs, ax
>     mov     ss, ax
>
>     OneTimeCallRet TransitionFromReal16To32BitFlat

The LINEAR_SEL offset is defined lower down in the same file, where the
actual contents of that segment descriptor are defined as well:

> ;
> ; The Global Descriptor Table (GDT)
> ;
>
> GDT_BASE:
> ; null descriptor
> NULL_SEL            equ $-GDT_BASE
>     DW      0            ; limit 15:0
>     DW      0            ; base 15:0
>     DB      0            ; base 23:16
>     DB      0            ; sys flag, dpl, type
>     DB      0            ; limit 19:16, flags
>     DB      0            ; base 31:24
>
> ; linear data segment descriptor
> LINEAR_SEL          equ $-GDT_BASE
>     DW      0xffff       ; limit 15:0
>     DW      0            ; base 15:0
>     DB      0            ; base 23:16
>     DB      PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(DATA32_TYPE)
>     DB      GRANULARITY_FLAG(1)|DEFAULT_SIZE32(1)|CODE64_FLAG(0)|UPPER_LIMIT(0xf)
>     DB      0            ; base 31:24
>
> [...]

The point is that this descriptor defines a segment that is based at
address 0, and limited at address 4GB. (Because "limit", including
UPPER_LIMIT(0xf), gives 0xf_ffff, and GRANULARITY_FLAG(1) means the
limit is expressed in 4KB pages).

So, I added the "@param[in]  ES" comment to show that I was aware of:

- being in unreal mode (in fact the file being patched itself states
  "Processor is in flat protected mode"),

- STOSD using ES,

- ES having the selector value LINEAR_SEL,

- the segment descriptor pointed-to by LINEAR_SEL defining a 0-based
  32-bit segment, meant for "read/write data".

In other words, that I was allowed to use EDI as a plain, zero-based
physical address.

Thanks,
Laszlo


>>  ;
>>  ; @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
>> +    cld                                                   ; store from base up
>> +    rep stosd
>> +
>>      ;
>>      ; Setup parameters and call SecCoreStartupWithStack
>>      ;   [esp]   return address for call
>> --
>> 2.14.1.3.gb7cf6e02401b
>>
>>