From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E1A8220359A79 for ; Fri, 10 Nov 2017 10:07:00 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 31877C04B31A; Fri, 10 Nov 2017 18:11:03 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-145.rdu2.redhat.com [10.10.120.145]) by smtp.corp.redhat.com (Postfix) with ESMTP id E222A4DA68; Fri, 10 Nov 2017 18:11:01 +0000 (UTC) To: Ard Biesheuvel Cc: edk2-devel-01 , Jordan Justen , Ruiyu Ni References: <20171110154908.306-1-lersek@redhat.com> <20171110154908.306-3-lersek@redhat.com> From: Laszlo Ersek Message-ID: Date: Fri, 10 Nov 2017 19:11:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 10 Nov 2017 18:11:03 +0000 (UTC) Subject: Re: [PATCH 2/4] OvmfPkg/Sec/Ia32: seed the temporary RAM with PcdInitValueInTempStack X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Nov 2017 18:07:01 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/10/17 16:56, Ard Biesheuvel wrote: > On 10 November 2017 at 15:49, Laszlo Ersek 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 >> Cc: Jordan Justen >> Cc: Ruiyu Ni >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=747 >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Laszlo Ersek >> --- >> 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 >> >>