[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save
From: |
Kevin O'Connor |
Subject: |
Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm. |
Date: |
Wed, 14 May 2014 21:32:42 -0400 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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