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 7385C203564DC for ; Tue, 30 Jan 2018 05:03:54 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AD96F5FB3A; Tue, 30 Jan 2018 13:09:28 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-185.rdu2.redhat.com [10.10.120.185]) by smtp.corp.redhat.com (Postfix) with ESMTP id 91BF87E3B0; Tue, 30 Jan 2018 13:09:08 +0000 (UTC) From: Laszlo Ersek To: "Wang, Jian J" , "edk2-devel@lists.01.org" Cc: "Ni, Ruiyu" , Paolo Bonzini , "Yao, Jiewen" , "Dong, Eric" , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= References: <20180115085433.25008-1-jian.j.wang@intel.com> <20180115085433.25008-5-jian.j.wang@intel.com> <2d1e4628-3367-2db7-ad4a-560988cf37d3@redhat.com> <4e5badfc-bfd1-45f2-58c2-4f5c1acce769@redhat.com> Message-ID: Date: Tue, 30 Jan 2018 14:09:07 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <4e5badfc-bfd1-45f2-58c2-4f5c1acce769@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 30 Jan 2018 13:09:28 +0000 (UTC) Subject: Re: [PATCH 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Enable NXE if it's supported X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Jan 2018 13:03:54 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/29/18 20:48, Laszlo Ersek wrote: > On 01/29/18 10:02, Wang, Jian J wrote: >> Hi Laszlo, >> >> I don't know the history of these code but I guess they're converted >> from .asm file. That may be why there's "DB 66h" prefix. I think >> you're right these tricks should be replaced with more formal ways. >> Please submit a bz tracker for it. > > I submitted . > >> As to the issue, I don't have clue right now. The code seems no >> problem. Since msr write didn't happen, code flow is correct. And >> those code has executed on real 32-bit platform without problem. I >> need more time to investigate it. > > I think I've found the issue; more exactly I narrowed it down a bit. I > remember that the same problem drove me mad a few years ago. :) > > The issue is that in the middle of such processor mode switches, no jump > instructions work *at all* on KVM. I don't know why, this is just my > experience. The KVM behavior could even be justified by the Intel SDM. > I'm unsure. > > Let's look at the patch with more context: > >> global ASM_PFX(SmmStartup) >> ASM_PFX(SmmStartup): >> + DB 0x66 >> + mov eax, 0x80000001 ; read capability >> + cpuid >> + DB 0x66 >> + mov ebx, edx ; rdmsr will change edx. keep it in ebx. >> DB 0x66, 0xb8 >> ASM_PFX(gSmmCr3): DD 0 >> mov cr3, eax >> DB 0x67, 0x66 >> lgdt [cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))] >> DB 0x66, 0xb8 >> ASM_PFX(gSmmCr4): DD 0 >> mov cr4, eax >> + DB 0x66 >> + mov ecx, 0xc0000080 ; IA32_EFER MSR >> + rdmsr >> + DB 0x66 >> + test ebx, BIT20 ; check NXE capability >> + jz .1 >> + or ah, BIT3 ; set NXE bit >> + wrmsr >> +.1: > > This code has exactly one jump, and in practice it is never taken > (because NX support is ubiquitous on physical platforms). > > In my testing I added some other conditional jumps -- I reimplemented > IsExecuteDisableBitAvailable() from > "MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c", which has more > conditions. All the conditional jumps that were *not* taken didn't > cause any issues. (This is why the same logic from the patch works for > me in the X64 version, because there the "jz" is never taken, since NX > is always available there.) However, the first jump that *was* taken in > my testing immediately hung or crashed the IA32 guest. > > Finally I replaced the entire NX management code with an unconditional > jump forward: > > jmp jump_here > jump_here: > > and even this hung / crashed the guest. > > For some reason this section of the code is unfit for jumping, under KVM > anyway. I found a work-around for this. While short jumps (= relative to EIP) do not work under KVM, in initial SMM, near jumps to absolute 32-bit addresses (specified indirectly via registers) do work [*]. Except, the address calculation has to take into account the trick that PiSmmCpuDxeSmm applies. Namely, for initial SMBASE relocation, - while the *short* gcSmmInitTemplate routine is copied to SMBASE+32KB (that is, to (0x3_0000 + 0x8000)), and executed from there, - the SmmStartup routine is actually executed from the *body* of PiSmmCpuDxeSmm -- that is, from SMRAM, to which place the SMM Core loaded and *relocated* PiSmmCpuDxeSmm, via normal SMM driver dispatch. This is why gcSmmInitTemplate jumps to SmmStartup as follows, in "UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm": > BITS 16 > ASM_PFX(gcSmmInitTemplate): > mov ebp, ASM_PFX(SmmStartup) > sub ebp, 0x30000 > jmp ebp > > ASM_PFX(gcSmmInitSize): DW $ - ASM_PFX(gcSmmInitTemplate) The "mov ebp, ASM_PFX(SmmStartup)" instruction is relocated at PiSmmCpuDxeSmm dispatch time, to the absolute address of SmmStartup in the body of PiSmmCpuDxeSmm. This absolute address is relative to zero. However, when the "jmp ebp" is executed in initial SMM mode, the CS register is not zero (i.e. EIP:=EBP will not be interpreted relative to zero). Instead, CS is initially set to 0x3000 in SMM, implying a code segment base that equals SMBASE (0x3_0000). Hence the "sub ebp, 0x30000" -- it compensates the original relocation of the "mov" for the suddenly increased CS base. The same applies to any near jump (with absolute indirect target) in the SmmStartup routine. All these jump instructions are relocated within the body of PiSmmCpuDxeSmm (at driver dispatch time) against a *zero* code segment base, but when they are actually executed in initial SMM, they are executed against a code segment base of 0x3_0000. I will send a patch (or a patch set, not sure yet). [*] segment limits are raised to 4GB, and near jumps can use such addresses with the "o32" (32-bit operand-size override) prefix. In fact, NASM inserts the prefix automatically upon seeing eg. "jmp ebx" in BITS 16. Thanks! Laszlo