From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35976) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkpH3-0000Rc-MH for qemu-devel@nongnu.org; Thu, 15 May 2014 02:36:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkpGv-00043H-6T for qemu-devel@nongnu.org; Thu, 15 May 2014 02:36:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32565) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkpGu-000435-Ue for qemu-devel@nongnu.org; Thu, 15 May 2014 02:36:05 -0400 Message-ID: <5374604C.40909@redhat.com> Date: Thu, 15 May 2014 08:35:56 +0200 From: Paolo Bonzini MIME-Version: 1.0 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> <537323DB.5070005@redhat.com> <20140515002059.GA28484@morn.localdomain> <20140515013242.GA608@morn.localdomain> In-Reply-To: <20140515013242.GA608@morn.localdomain> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin O'Connor Cc: Richard Henderson , Gerd Hoffmann , qemu-devel@nongnu.org, marcel.a@redhat.com Il 15/05/2014 03:32, Kevin O'Connor ha scritto: > On Wed, May 14, 2014 at 08:20:59PM -0400, Kevin O'Connor wrote: >> On Wed, May 14, 2014 at 10:05:47AM +0200, Paolo Bonzini wrote: >>> 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? >> >> Your patch causes Freedos to crash when emm386 is loaded, so I think >> it broke VM86 mode. Below are some logs I took from qemu at the point >> of the crash. > > FYI, with the patch below my quick test cases all look okay. > > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -974,7 +974,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 */ > @@ -984,16 +983,17 @@ 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 (env->eflags & VM_MASK) { > + cpl = 3; > + } > #if HF_CPL_MASK != 3 > #error HF_CPL_MASK is hardcoded > #endif > Looks like a bug entering VM86 mode. I'll take a look, thanks for confirming what works and what doesn't! Paolo