From patchwork Wed May 14 08:05:47 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 348646 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 23EE51400E2 for ; Wed, 14 May 2014 18:06:31 +1000 (EST) Received: from localhost ([::1]:49918 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkUCq-0005Xf-Vz for incoming@patchwork.ozlabs.org; Wed, 14 May 2014 04:06:28 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41056) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkUCS-0005D2-Cu for qemu-devel@nongnu.org; Wed, 14 May 2014 04:06:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkUCJ-0006n3-Cd for qemu-devel@nongnu.org; Wed, 14 May 2014 04:06:04 -0400 Received: from mail-ee0-x22b.google.com ([2a00:1450:4013:c00::22b]:36288) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkUCJ-0006mk-2J for qemu-devel@nongnu.org; Wed, 14 May 2014 04:05:55 -0400 Received: by mail-ee0-f43.google.com with SMTP id d17so1071566eek.30 for ; Wed, 14 May 2014 01:05:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=BkGxr8ylQC22f1qM+spWCekvWlwdiQ1Q6l5IR1RJNSg=; b=VYzBt/607paZ9kq7MI4lTz8zBgc2vIL1huEtRTvct12b44u0oe5bhaq1KsbjojhHRG KbB7+ntX4e73pnQzJpDu4Xf7LnRj/tU8UPHnG7ZWkoANn2oFP4Vj7SeB9vPTX2NcpDSt L/90hICxHLg7cTbrb7ABVO0OombPhY07pt9XbdEp14LgUQFOHMx8q3B7ibIrMzGYIUFs Kn5q+mM1I2NuFTChHyQsYR2w89P1h0ZWPPppEdMg6m5ayHzfFs9gPtTx8iklxx+CJnWb q1HgxxCMJk82piPLiqibljuRi8XDerGlJAUvDGQES4rF1GODgjyBHt5YZ4ksWyS/TuE5 Ngbw== X-Received: by 10.14.100.7 with SMTP id y7mr1505928eef.110.1400054753844; Wed, 14 May 2014 01:05:53 -0700 (PDT) Received: from yakj.usersys.redhat.com (net-37-117-141-58.cust.vodafonedsl.it. [37.117.141.58]) by mx.google.com with ESMTPSA id l3sm3190260eeo.43.2014.05.14.01.05.51 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 14 May 2014 01:05:52 -0700 (PDT) Message-ID: <537323DB.5070005@redhat.com> Date: Wed, 14 May 2014 10:05:47 +0200 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Kevin O'Connor References: <20140425171718.GA1591@morn.localdomain> <535B772D.1020602@redhat.com> <1398601366.21309.33.camel@localhost.localdomain> <535D1445.3040905@redhat.com> <20140427172524.GB28385@morn.localdomain> <5372636F.8080101@redhat.com> <20140513183920.GA23439@morn.localdomain> <20140513220705.GA7084@morn.localdomain> In-Reply-To: <20140513220705.GA7084@morn.localdomain> X-Enigmail-Version: 1.6 X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4013:c00::22b Cc: Richard Henderson , Gerd Hoffmann , qemu-devel@nongnu.org, marcel.a@redhat.com Subject: Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm. X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Il 14/05/2014 00:07, Kevin O'Connor ha scritto: > On Tue, May 13, 2014 at 02:39:20PM -0400, Kevin O'Connor wrote: >> On Tue, May 13, 2014 at 08:24:47PM +0200, Paolo Bonzini wrote: >>> Il 27/04/2014 19:25, Kevin O'Connor ha scritto: >>>> I was wondering about that as well. The Intel docs state that the CPL >>>> is bits 0-1 of the CS.selector register, and that protected mode >>>> starts immediately after setting the PE bit. The CS.selector field >>>> should be the value of %cs in real mode, which is the value added to >>>> eip (after shifting right by 4). >>>> >>>> I guess that means that the real mode code that enables the PE bit >>>> must run with a code segment aligned to a value of 4. (Which >>>> effectively means code alignment of 64 bytes because of the segment >>>> shift.) >>> >>> It turns out that this is not a requirement; which means that the >>> protected mode transition is exactly the only place where CPL is not >>> redundant. The CPL remains zero until you reload CS with a long jump. >> >> That doesn't sound right. > > FYI, I ran a couple of tests on a real machine where I set protected > mode while %cs=0xf003 and I can confirm that it doesn't cause faults. > So, you are correct - the CPL appears to be stored separately from > %cs[1:0] and it appears CPL isn't altered until CS is reloaded. CPL isn't even altered when CS is reloaded, because you cannot jump out of ring-0 except with an inter-privilege IRET, and that reloads SS too. An IRET or task switch is also the only way to set EFLAGS.VM, and it will hardcode SS.DPL=3, again matching CPL=3. Finally, to get out of real mode you need to have CPL=0, and whatever got you at CPL has also loaded SS with a ring-0 stack. This means that SS.DPL=0 right after clearing CR0.PE. Using SS.DPL as the CPL really sounds like the right approach. I tried it on my KVM testcase, and it works well. For QEMU, even the special case of SYSRET will be handled fine because QEMU does set SS.DPL = 3: cpu_x86_load_seg_cache(env, R_SS, selector + 8, 0, 0xffffffff, DESC_G_MASK | DESC_B_MASK | DESC_P_MASK | DESC_S_MASK | (3 << DESC_DPL_SHIFT) | DESC_W_MASK | DESC_A_MASK); SS.DPL=CPL=3, SS.RPL=selector & 3 is a mix of Intel behavior (which is SS.DPL=SS.RPL=CPL=3) and AMD behavior (because they set CPL=3 but SS.DPL=SS.RPL=selector & 3). We may want to match Intel behavior, but that's a different change. Can you check if this patch works for you, and if so reply with Tested-by/Reviewed-by? ------------- 8< ------------------- From: Paolo Bonzini Subject: [PATCH] target-i386: get CPL from SS.DPL CS.RPL is not equal to the CPL in the few instructions between setting CR0.PE and reloading CS. We get this right in the common case, because writes to CR0 do not modify the CPL, but it would not be enough if an SMI comes exactly during that brief period. Were this to happen, the RSM instruction would erroneously set CPL to the low two bits of the real-mode selector; and if they are not 00, the next instruction fetch cannot access the code segment and causes a triple fault. However, SS.DPL *is* always equal to the CPL. In real processors (AMD only) there is a weird case of SYSRET setting SS.DPL=SS.RPL from the STAR register while forcing CPL=3, but we do not emulate that. Signed-off-by: Paolo Bonzini Paolo diff --git a/target-i386/cpu.h b/target-i386/cpu.h index e9cbdab..65a44d9 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -986,7 +986,6 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env, /* update the hidden flags */ { if (seg_reg == R_CS) { - int cpl = selector & 3; #ifdef TARGET_X86_64 if ((env->hflags & HF_LMA_MASK) && (flags & DESC_L_MASK)) { /* long mode */ @@ -996,15 +995,14 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env, #endif { /* legacy / compatibility case */ - if (!(env->cr[0] & CR0_PE_MASK)) - cpl = 0; - else if (env->eflags & VM_MASK) - cpl = 3; new_hflags = (env->segs[R_CS].flags & DESC_B_MASK) >> (DESC_B_SHIFT - HF_CS32_SHIFT); env->hflags = (env->hflags & ~(HF_CS32_MASK | HF_CS64_MASK)) | new_hflags; } + } + if (seg_reg == R_SS) { + int cpl = (flags >> DESC_DPL_SHIFT) & 3; #if HF_CPL_MASK != 3 #error HF_CPL_MASK is hardcoded #endif