From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::243; helo=mail-io0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EC3F220356863 for ; Sat, 11 Nov 2017 01:06:17 -0800 (PST) Received: by mail-io0-x243.google.com with SMTP id 71so322711ior.7 for ; Sat, 11 Nov 2017 01:10:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=OWW0FJUbQ8HnJDqK6qaCn73KVTLmPedevmrlPxtZ4gE=; b=gnNUHrDo998z70Hr8Hg+1j/2i262H8IoY7cRswsajngtLRsNV2xA0Pav3NtbmaUdi4 wPycOnULNGxwhPsxlMcRgRP9k6ZnpLBudKOvPD9DiM7CCDBPOAXFW/of6EFJcPOSMA79 RJVKS/NTaifl6MBKZ8XopfXxssjl/QHMdcb+8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=OWW0FJUbQ8HnJDqK6qaCn73KVTLmPedevmrlPxtZ4gE=; b=bbMTy9YXo8rxJfpQfBxOUzKepJ2g2XWuIz63xBo7e+AYPo6TG08fXg4WwBru6cmpD4 Ks+djTgAjwnfXWO4UuYDglVgU29DXgzvWNvWt+xtPNkDtD+w2BN+diGn9UbHRKu9ShoG GZbK/xsR94nVlvWz48f9fYtHNdSxtQwUBirSRXByKHyPzSrHxlbOLjvx3C4B2rnqWBnY S/RNle4LaNW9d+3zIYZSTJHo3C8gVEEcw6p5ZxtgsureE7w3yTvyf3aEoGXefwgFak3r zlUdrezWjLlpLk6maxZ1B5JYv54/PhFGHuLjiCY5aJhzlyCwr8WyzX7/4AjzblGUxRYO 5VkQ== X-Gm-Message-State: AJaThX5notSpHux3Bd3kylfGL07kMjQpGER+700uh9+KmIRE0PgnIxbH sTtD8rBtHwb8xSzeauHynzvoE/Ww4f8WhtDldXNDLA== X-Google-Smtp-Source: AGs4zMZBp33mYVDy8huxQinoJjLMpOcKLyIXvg7wtlSrsToiq8b4YK51XFJmCG35Aen6gGqSdKoHkJfEoGmYNnqwGVU= X-Received: by 10.107.174.206 with SMTP id n75mr3285071ioo.43.1510391420583; Sat, 11 Nov 2017 01:10:20 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.20 with HTTP; Sat, 11 Nov 2017 01:10:20 -0800 (PST) In-Reply-To: References: <20171110154908.306-1-lersek@redhat.com> <20171110154908.306-3-lersek@redhat.com> From: Ard Biesheuvel Date: Sat, 11 Nov 2017 09:10:20 +0000 Message-ID: To: Laszlo Ersek Cc: edk2-devel-01 , Jordan Justen , Ruiyu Ni 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: Sat, 11 Nov 2017 09:06:18 -0000 Content-Type: text/plain; charset="UTF-8" On 10 November 2017 at 18:11, Laszlo Ersek wrote: > 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. > Excellent clarification, thanks.