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. -Kevin --- 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